Skip to content

Commit b4ea5f7

Browse files
author
Jun
committed
[TDB-20] With checkpoint disabled, closing FT file still rotates header.
This is an attempt based on @BohuTANG's patch percona#331 in particular, the ctest is all his credits. Summary: 1) The issue is real. 2) @bohu's patch works for mediating the issue but somewhat introduces a ft leak that the ft with ref=0 may be left until next open/close or checkpoint. 3) If ft_close should be held off upon a backup, then it simply means a backup should hold a ref to the ft and responsible for closing ft up if it is the last one to hold it, like any other possible referencer. including the checkpoint, the txn and ft handlers. 4) mtr test contributed by @BohuTANG still passes
1 parent 871ee73 commit b4ea5f7

11 files changed

+253
-11
lines changed

ft/cachetable/cachetable-internal.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,9 @@ struct cachefile {
161161
void (*end_checkpoint_userdata)(CACHEFILE cf, int fd, void *userdata); // after checkpointing cachefiles call this function.
162162
void (*note_pin_by_checkpoint)(CACHEFILE cf, void *userdata); // add a reference to the userdata to prevent it from being removed from memory
163163
void (*note_unpin_by_checkpoint)(CACHEFILE cf, void *userdata); // add a reference to the userdata to prevent it from being removed from memory
164+
165+
void (*note_pin_by_backup)(CACHEFILE cf, void *userdata); // add a reference to the userdata to prevent it from being removed from memory
166+
void (*note_unpin_by_backup)(CACHEFILE cf, void *userdata); // add a reference to the userdata to prevent it from being removed from memory
164167
BACKGROUND_JOB_MANAGER bjm;
165168
};
166169

@@ -413,6 +416,8 @@ class checkpointer {
413416
void add_background_job();
414417
void remove_background_job();
415418
void end_checkpoint(void (*testcallback_f)(void*), void* testextra);
419+
void begin_backup();
420+
void end_backup();
416421
TOKULOGGER get_logger();
417422
// used during begin_checkpoint
418423
void increment_num_txns();
@@ -602,6 +607,7 @@ struct cachetable {
602607
KIBBUTZ client_kibbutz; // pool of worker threads and jobs to do asynchronously for the client.
603608
KIBBUTZ ct_kibbutz; // pool of worker threads and jobs to do asynchronously for the cachetable
604609
KIBBUTZ checkpointing_kibbutz; // small pool for checkpointing cloned pairs
610+
//bool in_backup; // we are in back up or NOT, default is false
605611

606612
char *env_dir;
607613
};

ft/cachetable/cachetable.cc

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2782,6 +2782,37 @@ void toku_cachetable_end_checkpoint(CHECKPOINTER cp, TOKULOGGER UU(logger),
27822782
cp->end_checkpoint(testcallback_f, testextra);
27832783
}
27842784

2785+
// in_backup begin
2786+
2787+
struct iterate_note_pin_backup {
2788+
static int fn(const CACHEFILE &cf, uint32_t UU(idx), void **UU(extra)) {
2789+
assert(cf->note_pin_by_backup);
2790+
cf->note_pin_by_backup(cf, cf->userdata);
2791+
return 0;
2792+
}
2793+
};
2794+
2795+
struct iterate_note_unpin_backup {
2796+
static int fn(const CACHEFILE &cf, uint32_t UU(idx), void **UU(extra)) {
2797+
assert(cf->note_unpin_by_backup);
2798+
cf->note_unpin_by_backup(cf, cf->userdata);
2799+
return 0;
2800+
}
2801+
};
2802+
void toku_cachetable_begin_backup(CACHETABLE ct)
2803+
{
2804+
ct->cf_list.read_lock();
2805+
ct->cf_list.m_active_fileid.iterate<void *, iterate_note_pin_backup::fn>(nullptr);
2806+
ct->cf_list.read_unlock();
2807+
}
2808+
2809+
void toku_cachetable_end_backup(CACHETABLE ct)
2810+
{
2811+
ct->cf_list.m_active_fileid.iterate<void *, iterate_note_unpin_backup::fn>(nullptr);
2812+
}
2813+
2814+
// in_backup end
2815+
27852816
TOKULOGGER toku_cachefile_logger (CACHEFILE cf) {
27862817
return cf->cachetable->cp.get_logger();
27872818
}
@@ -2898,7 +2929,9 @@ toku_cachefile_set_userdata (CACHEFILE cf,
28982929
void (*begin_checkpoint_userdata)(LSN, void*),
28992930
void (*end_checkpoint_userdata)(CACHEFILE, int, void*),
29002931
void (*note_pin_by_checkpoint)(CACHEFILE, void*),
2901-
void (*note_unpin_by_checkpoint)(CACHEFILE, void*)) {
2932+
void (*note_unpin_by_checkpoint)(CACHEFILE, void*),
2933+
void (*note_pin_by_backup)(CACHEFILE, void*),
2934+
void (*note_unpin_by_backup)(CACHEFILE, void*)) {
29022935
cf->userdata = userdata;
29032936
cf->log_fassociate_during_checkpoint = log_fassociate_during_checkpoint;
29042937
cf->close_userdata = close_userdata;
@@ -2908,6 +2941,8 @@ toku_cachefile_set_userdata (CACHEFILE cf,
29082941
cf->end_checkpoint_userdata = end_checkpoint_userdata;
29092942
cf->note_pin_by_checkpoint = note_pin_by_checkpoint;
29102943
cf->note_unpin_by_checkpoint = note_unpin_by_checkpoint;
2944+
cf->note_pin_by_backup = note_pin_by_backup;
2945+
cf->note_unpin_by_backup = note_unpin_by_backup;
29112946
}
29122947

29132948
void *toku_cachefile_get_userdata(CACHEFILE cf) {

ft/cachetable/cachetable.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,8 @@ void toku_cachetable_begin_checkpoint (CHECKPOINTER cp, struct tokulogger *logge
147147
void toku_cachetable_end_checkpoint(CHECKPOINTER cp, struct tokulogger *logger,
148148
void (*testcallback_f)(void*), void * testextra);
149149

150+
void toku_cachetable_begin_backup(CACHETABLE ct);
151+
void toku_cachetable_end_backup(CACHETABLE ct);
150152

151153
// Shuts down checkpoint thread
152154
// Requires no locks be held that are taken by the checkpoint function
@@ -285,7 +287,9 @@ void toku_cachefile_set_userdata(CACHEFILE cf, void *userdata,
285287
void (*begin_checkpoint_userdata)(LSN, void*),
286288
void (*end_checkpoint_userdata)(CACHEFILE, int, void*),
287289
void (*note_pin_by_checkpoint)(CACHEFILE, void*),
288-
void (*note_unpin_by_checkpoint)(CACHEFILE, void*));
290+
void (*note_unpin_by_checkpoint)(CACHEFILE, void*),
291+
void (*note_pin_by_backup)(CACHEFILE, void*),
292+
void (*note_unpin_by_backup)(CACHEFILE, void*));
289293
// Effect: Store some cachefile-specific user data. When the last reference to a cachefile is closed, we call close_userdata().
290294
// Before starting a checkpoint, we call checkpoint_prepare_userdata().
291295
// When the cachefile needs to be checkpointed, we call checkpoint_userdata().

ft/ft-internal.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,9 @@ struct ft {
197197
uint32_t num_txns;
198198
// A checkpoint is running. If true, then keep this header around for checkpoint, like a transaction
199199
bool pinned_by_checkpoint;
200-
200+
// Number of backups that are using this FT. If it is nonzero, keep this header around until backup
201+
// is completd.
202+
uint32_t num_backups;
201203
// is this ft a blackhole? if so, all messages are dropped.
202204
bool blackhole;
203205

ft/ft.cc

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,35 @@ static void ft_note_unpin_by_checkpoint (CACHEFILE UU(cachefile), void *header_v
313313
toku_ft_remove_reference(ft, false, ZERO_LSN, unpin_by_checkpoint_callback, NULL);
314314
}
315315

316+
317+
// maps to cf->note_pin_by_backup
318+
//Must be protected by ydb lock.
319+
//Is only called by backup begin, which holds it
320+
static void ft_note_pin_by_backup (CACHEFILE UU(cachefile), void *header_v) {
321+
// Note: open_close lock is held by checkpoint begin
322+
FT ft = (FT) header_v;
323+
toku_ft_grab_reflock(ft);
324+
assert(toku_ft_needed_unlocked(ft));
325+
ft->num_backups ++;
326+
toku_ft_release_reflock(ft);
327+
}
328+
329+
// Requires: the reflock is held.
330+
static void unpin_by_backup_callback(FT ft, void *extra) {
331+
invariant(extra == NULL);
332+
invariant(ft->num_backups>0);
333+
ft->num_backups --;
334+
}
335+
336+
// maps to cf->note_unpin_by_backup
337+
//Must be protected by ydb lock.
338+
//Called by end_backup, which grabs ydb lock around note_unpin
339+
static void ft_note_unpin_by_backup (CACHEFILE UU(cachefile), void *header_v) {
340+
FT ft = (FT) header_v;
341+
toku_ft_remove_reference(ft, false, ZERO_LSN, unpin_by_backup_callback, NULL);
342+
}
343+
344+
316345
//
317346
// End of Functions that are callbacks to the cachefile
318347
/////////////////////////////////////////////////////////////////////////
@@ -358,7 +387,9 @@ static void ft_init(FT ft, FT_OPTIONS options, CACHEFILE cf) {
358387
ft_begin_checkpoint,
359388
ft_end_checkpoint,
360389
ft_note_pin_by_checkpoint,
361-
ft_note_unpin_by_checkpoint);
390+
ft_note_unpin_by_checkpoint,
391+
ft_note_pin_by_backup,
392+
ft_note_unpin_by_backup);
362393

363394
ft->blocktable.verify_no_free_blocknums();
364395
}
@@ -455,7 +486,9 @@ int toku_read_ft_and_store_in_cachefile (FT_HANDLE ft_handle, CACHEFILE cf, LSN
455486
ft_begin_checkpoint,
456487
ft_end_checkpoint,
457488
ft_note_pin_by_checkpoint,
458-
ft_note_unpin_by_checkpoint);
489+
ft_note_unpin_by_checkpoint,
490+
ft_note_pin_by_backup,
491+
ft_note_unpin_by_backup);
459492
*header = ft;
460493
return 0;
461494
}
@@ -475,7 +508,7 @@ static int
475508
ft_get_reference_count(FT ft) {
476509
uint32_t pinned_by_checkpoint = ft->pinned_by_checkpoint ? 1 : 0;
477510
int num_handles = toku_list_num_elements_est(&ft->live_ft_handles);
478-
return pinned_by_checkpoint + ft->num_txns + num_handles;
511+
return pinned_by_checkpoint + ft->num_txns + ft-> num_backups + num_handles;
479512
}
480513

481514
// a ft is needed in memory iff its reference count is non-zero

ft/tests/cachetable-pin-checkpoint.cc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,9 @@ cachetable_test (void) {
376376
&test_begin_checkpoint,
377377
&dummy_end,
378378
&dummy_note_pin,
379-
&dummy_note_unpin
379+
&dummy_note_unpin,
380+
&dummy_note_pin,
381+
&dummy_note_unpin
380382
);
381383

382384
toku_pthread_t time_tid;

ft/tests/cachetable-put-checkpoint.cc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -508,7 +508,9 @@ cachetable_test (void) {
508508
test_begin_checkpoint, // called in begin_checkpoint
509509
&dummy_end,
510510
&dummy_note_pin,
511-
&dummy_note_unpin
511+
&dummy_note_unpin,
512+
&dummy_note_pin,
513+
&dummy_note_unpin
512514
);
513515

514516
toku_pthread_t time_tid;

ft/tests/cachetable-simple-close.cc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,9 @@ static void set_cf_userdata(CACHEFILE f1) {
6060
&dummy_begin,
6161
&dummy_end,
6262
&dummy_note_pin,
63-
&dummy_note_unpin
63+
&dummy_note_unpin,
64+
&dummy_note_pin,
65+
&dummy_note_unpin
6466
);
6567
}
6668

ft/tests/cachetable-test.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,5 +68,7 @@ create_dummy_functions(CACHEFILE cf)
6868
&dummy_begin,
6969
&dummy_end,
7070
&dummy_note_pin,
71+
&dummy_note_unpin,
72+
&dummy_note_pin,
7173
&dummy_note_unpin);
7274
};

ft/tests/ft-in-backup-test.cc

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
/* -*- mode: C++; c-basic-offset: 4; indent-tabs-mode: nil -*- */
2+
// vim: ft=cpp:expandtab:ts=8:sw=4:softtabstop=4:
3+
#ident "$Id$"
4+
/*======
5+
This file is part of PerconaFT.
6+
7+
8+
Copyright (c) 2006, 2015, Percona and/or its affiliates. All rights reserved.
9+
10+
PerconaFT is free software: you can redistribute it and/or modify
11+
it under the terms of the GNU General Public License, version 2,
12+
as published by the Free Software Foundation.
13+
14+
PerconaFT is distributed in the hope that it will be useful,
15+
but WITHOUT ANY WARRANTY; without even the implied warranty of
16+
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
17+
GNU General Public License for more details.
18+
19+
You should have received a copy of the GNU General Public License
20+
along with PerconaFT. If not, see <http://www.gnu.org/licenses/>.
21+
22+
----------------------------------------
23+
24+
PerconaFT is free software: you can redistribute it and/or modify
25+
it under the terms of the GNU Affero General Public License, version 3,
26+
as published by the Free Software Foundation.
27+
28+
PerconaFT is distributed in the hope that it will be useful,
29+
but WITHOUT ANY WARRANTY; without even the implied warranty of
30+
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
31+
GNU Affero General Public License for more details.
32+
33+
You should have received a copy of the GNU Affero General Public License
34+
along with PerconaFT. If not, see <http://www.gnu.org/licenses/>.
35+
======= */
36+
37+
#ident "Copyright (c) 2006, 2015, Percona and/or its affiliates. All rights reserved."
38+
39+
#include "test.h"
40+
#include "cachetable/checkpoint.h"
41+
42+
static TOKUTXN const null_txn = 0;
43+
static const char *fname = TOKU_TEST_FILENAME;
44+
45+
/* test for_backup in ft_close */
46+
static void test_in_backup() {
47+
int r;
48+
CACHETABLE ct;
49+
FT_HANDLE ft;
50+
unlink(fname);
51+
52+
toku_cachetable_create(&ct, 0, ZERO_LSN, nullptr);
53+
// TEST1 : for normal
54+
r = toku_open_ft_handle(fname, 1, &ft, 1<<12, 1<<9, TOKU_DEFAULT_COMPRESSION_METHOD, ct, null_txn, toku_builtin_compare_fun);
55+
assert_zero(r);
56+
r = toku_close_ft_handle_nolsn(ft, 0);
57+
assert_zero(r);
58+
59+
r = toku_open_ft_handle(fname, 0, &ft, 1<<12, 1<<9, TOKU_DEFAULT_COMPRESSION_METHOD, ct, null_txn, toku_builtin_compare_fun);
60+
assert_zero(r);
61+
{
62+
DBT k,v;
63+
toku_ft_insert(ft, toku_fill_dbt(&k, "hello", 6), toku_fill_dbt(&v, "there", 6), null_txn);
64+
}
65+
r = toku_close_ft_handle_nolsn(ft, 0);
66+
assert_zero(r);
67+
68+
r = toku_open_ft_handle(fname, 0, &ft, 1<<12, 1<<9, TOKU_DEFAULT_COMPRESSION_METHOD, ct, null_txn, toku_builtin_compare_fun);
69+
assert_zero(r);
70+
ft_lookup_and_check_nodup(ft, "hello", "there");
71+
r = toku_close_ft_handle_nolsn(ft, 0);
72+
assert_zero(r);
73+
toku_cachetable_close(&ct);
74+
75+
// TEST2: in fly without checkpoint test
76+
toku_cachetable_create(&ct, 0, ZERO_LSN, nullptr);
77+
r = toku_open_ft_handle(fname, 0, &ft, 1<<12, 1<<9, TOKU_DEFAULT_COMPRESSION_METHOD, ct, null_txn, toku_builtin_compare_fun);
78+
assert_zero(r);
79+
80+
toku_cachetable_begin_backup(ct);
81+
// this key/value just in fly since we are in backing up
82+
{
83+
DBT k,v;
84+
toku_ft_insert(ft, toku_fill_dbt(&k, "halou", 6), toku_fill_dbt(&v, "not there", 10), null_txn);
85+
}
86+
r = toku_close_ft_handle_nolsn(ft, 0);
87+
assert_zero(r);
88+
89+
r = toku_open_ft_handle(fname, 0, &ft, 1<<12, 1<<9, TOKU_DEFAULT_COMPRESSION_METHOD, ct, null_txn, toku_builtin_compare_fun);
90+
assert_zero(r);
91+
ft_lookup_and_check_nodup(ft, "halou", "not there");
92+
93+
// because we are in backup, so the FT header is stale after cachefile&cachetable closed
94+
// here has a leak for this ft evicts from memroy, but that makes sense
95+
r = toku_close_ft_handle_nolsn(ft, 0);
96+
assert_zero(r);
97+
toku_cachetable_close(&ct);
98+
99+
// check the in fly key/value, it shouldn't exist
100+
toku_cachetable_create(&ct, 0, ZERO_LSN, nullptr);
101+
r = toku_open_ft_handle(fname, 0, &ft, 1<<12, 1<<9, TOKU_DEFAULT_COMPRESSION_METHOD, ct, null_txn, toku_builtin_compare_fun);
102+
assert_zero(r);
103+
ft_lookup_and_fail_nodup(ft, (char*)"halou");
104+
r = toku_close_ft_handle_nolsn(ft, 0);
105+
assert_zero(r);
106+
toku_cachetable_end_backup(ct);
107+
toku_cachetable_close(&ct);
108+
109+
// TEST3: in fly with checkpoint test
110+
toku_cachetable_create(&ct, 0, ZERO_LSN, nullptr);
111+
r = toku_open_ft_handle(fname, 0, &ft, 1<<12, 1<<9, TOKU_DEFAULT_COMPRESSION_METHOD, ct, null_txn, toku_builtin_compare_fun);
112+
assert_zero(r);
113+
114+
toku_cachetable_begin_backup(ct);
115+
// this key/value just in fly since we are in backup
116+
{
117+
DBT k,v;
118+
toku_ft_insert(ft, toku_fill_dbt(&k, "halou1", 7), toku_fill_dbt(&v, "not there", 10), null_txn);
119+
}
120+
r = toku_close_ft_handle_nolsn(ft, 0);
121+
assert_zero(r);
122+
123+
r = toku_open_ft_handle(fname, 0, &ft, 1<<12, 1<<9, TOKU_DEFAULT_COMPRESSION_METHOD, ct, null_txn, toku_builtin_compare_fun);
124+
assert_zero(r);
125+
ft_lookup_and_check_nodup(ft, "halou1", "not there");
126+
127+
// because we are in backup, so the FT header is stale after cachefile&cachetable closed
128+
r = toku_close_ft_handle_nolsn(ft, 0);
129+
assert_zero(r);
130+
toku_cachetable_end_backup(ct);
131+
toku_cachetable_close(&ct);
132+
133+
toku_cachetable_create(&ct, 0, ZERO_LSN, nullptr);
134+
r = toku_open_ft_handle(fname, 0, &ft, 1<<12, 1<<9, TOKU_DEFAULT_COMPRESSION_METHOD, ct, null_txn, toku_builtin_compare_fun);
135+
assert_zero(r);
136+
ft_lookup_and_check_nodup(ft, "halou1", "not there");
137+
r = toku_close_ft_handle_nolsn(ft, 0);
138+
assert_zero(r);
139+
toku_cachetable_close(&ct);
140+
}
141+
142+
int
143+
test_main (int argc , const char *argv[]) {
144+
default_parse_args(argc, argv);
145+
test_in_backup();
146+
if (verbose) printf("test ok\n");
147+
return 0;
148+
}

0 commit comments

Comments
 (0)