Skip to content

Commit

Permalink
WT-7416 Backup cursor has a key, but incr backup cursor returns WT_NO…
Browse files Browse the repository at this point in the history
…TFOUND (#6524)


* Added a test for this bug

* Fixed issue potentially

* Cleaned up the branch, and renamed test

* Added more tests to backup22
  • Loading branch information
jiechenbo committed May 13, 2021
1 parent 7464d9c commit d59b4d6
Show file tree
Hide file tree
Showing 8 changed files with 166 additions and 53 deletions.
12 changes: 8 additions & 4 deletions src/btree/bt_import.c
Expand Up @@ -20,6 +20,7 @@ __wt_import_repair(WT_SESSION_IMPL *session, const char *uri, char **configp)
WT_CONFIG_ITEM v;
WT_DECL_ITEM(a);
WT_DECL_ITEM(b);
WT_DECL_ITEM(buf);
WT_DECL_ITEM(checkpoint);
WT_DECL_RET;
WT_KEYED_ENCRYPTOR *kencryptor;
Expand All @@ -33,6 +34,7 @@ __wt_import_repair(WT_SESSION_IMPL *session, const char *uri, char **configp)

WT_ERR(__wt_scr_alloc(session, 0, &a));
WT_ERR(__wt_scr_alloc(session, 0, &b));
WT_ERR(__wt_scr_alloc(session, 1024, &buf));
WT_ERR(__wt_scr_alloc(session, 0, &checkpoint));

WT_ASSERT(session, WT_PREFIX_MATCH(uri, "file:"));
Expand Down Expand Up @@ -92,13 +94,14 @@ __wt_import_repair(WT_SESSION_IMPL *session, const char *uri, char **configp)
* Build and flatten the metadata and the checkpoint list, then insert it into the metadata for
* this file.
*
* Strip out any incremental backup information, an imported file has not been part of a backup.
* Strip out the checkpoint LSN, an imported file isn't associated with any log files. Assign a
* unique file ID.
* Reconstruct the incremental backup information, to indicate copying the whole file as an
* imported file has not been part of backup. Strip out the checkpoint LSN, an imported file
* isn't associated with any log files. Assign a unique file ID.
*/
cfg[1] = a->data;
cfg[2] = checkpoint_list;
cfg[3] = "checkpoint_backup_info=";
WT_ERR(__wt_reset_blkmod(session, a->data, buf));
cfg[3] = buf->mem;
cfg[4] = "checkpoint_lsn=";
WT_WITH_SCHEMA_LOCK(session,
ret = __wt_snprintf(fileid, sizeof(fileid), "id=%" PRIu32, ++S2C(session)->next_file_id));
Expand Down Expand Up @@ -154,6 +157,7 @@ __wt_import_repair(WT_SESSION_IMPL *session, const char *uri, char **configp)

__wt_scr_free(session, &a);
__wt_scr_free(session, &b);
__wt_scr_free(session, &buf);
__wt_scr_free(session, &checkpoint);

return (ret);
Expand Down
27 changes: 16 additions & 11 deletions src/cursor/cur_backup_incr.c
Expand Up @@ -78,23 +78,28 @@ __curbackup_incr_blkmod(WT_SESSION_IMPL *session, WT_BTREE *btree, WT_CURSOR_BAC

/*
* The rename configuration string component was added later. So don't error if we don't
* find it in the string. If we don't have it, we're not doing a rename.
* find it in the string. If we don't have it, we're not doing a rename. Otherwise rename
* forces full copies, there is no need to traverse the blocks information.
*/
WT_ERR_NOTFOUND_OK(__wt_config_subgets(session, &v, "rename", &b), true);
if (ret == 0 && b.val)
if (ret == 0 && b.val) {
cb->nbits = 0;
cb->offset = 0;
cb->bit_offset = 0;
F_SET(cb, WT_CURBACKUP_RENAME);
else
} else {
F_CLR(cb, WT_CURBACKUP_RENAME);

/*
* We found a match. Load the block information into the cursor.
*/
if ((ret = __wt_config_subgets(session, &v, "blocks", &b)) == 0) {
WT_ERR(__wt_backup_load_incr(session, &b, &cb->bitstring, cb->nbits));
cb->bit_offset = 0;
F_SET(cb, WT_CURBACKUP_INCR_INIT);
/*
* We found a match. Load the block information into the cursor.
*/
if ((ret = __wt_config_subgets(session, &v, "blocks", &b)) == 0) {
WT_ERR(__wt_backup_load_incr(session, &b, &cb->bitstring, cb->nbits));
cb->bit_offset = 0;
F_SET(cb, WT_CURBACKUP_INCR_INIT);
}
WT_ERR_NOTFOUND_OK(ret, false);
}
WT_ERR_NOTFOUND_OK(ret, false);
break;
}
WT_ERR_NOTFOUND_OK(ret, false);
Expand Down
4 changes: 2 additions & 2 deletions src/include/extern.h
Expand Up @@ -1041,8 +1041,6 @@ extern int __wt_meta_apply_all(WT_SESSION_IMPL *session,
int (*file_func)(WT_SESSION_IMPL *, const char *[]),
int (*name_func)(WT_SESSION_IMPL *, const char *, bool *), const char *cfg[])
WT_GCC_FUNC_DECL_ATTRIBUTE((warn_unused_result));
extern int __wt_meta_blk_mods_load(WT_SESSION_IMPL *session, const char *config, WT_CKPT *base_ckpt,
WT_CKPT *ckpt, bool rename) WT_GCC_FUNC_DECL_ATTRIBUTE((warn_unused_result));
extern int __wt_meta_block_metadata(WT_SESSION_IMPL *session, const char *config, WT_CKPT *ckpt)
WT_GCC_FUNC_DECL_ATTRIBUTE((warn_unused_result));
extern int __wt_meta_checkpoint(WT_SESSION_IMPL *session, const char *fname, const char *checkpoint,
Expand Down Expand Up @@ -1239,6 +1237,8 @@ extern int __wt_reconcile(WT_SESSION_IMPL *session, WT_REF *ref, WT_SALVAGE_COOK
uint32_t flags) WT_GCC_FUNC_DECL_ATTRIBUTE((warn_unused_result));
extern int __wt_remove_if_exists(WT_SESSION_IMPL *session, const char *name, bool durable)
WT_GCC_FUNC_DECL_ATTRIBUTE((warn_unused_result));
extern int __wt_reset_blkmod(WT_SESSION_IMPL *session, const char *orig_config, WT_ITEM *buf)
WT_GCC_FUNC_DECL_ATTRIBUTE((warn_unused_result));
extern int __wt_rollback_to_stable(WT_SESSION_IMPL *session, const char *cfg[], bool no_ckpt)
WT_GCC_FUNC_DECL_ATTRIBUTE((warn_unused_result));
extern int __wt_row_ikey(WT_SESSION_IMPL *session, uint32_t cell_offset, const void *key,
Expand Down
35 changes: 30 additions & 5 deletions src/meta/meta_ckpt.c
Expand Up @@ -14,7 +14,7 @@ static int __ckpt_load(WT_SESSION_IMPL *, WT_CONFIG_ITEM *, WT_CONFIG_ITEM *, WT
static int __ckpt_named(WT_SESSION_IMPL *, const char *, const char *, WT_CKPT *);
static int __ckpt_set(WT_SESSION_IMPL *, const char *, const char *, bool);
static int __ckpt_version_chk(WT_SESSION_IMPL *, const char *, const char *);

static int meta_blk_mods_load(WT_SESSION_IMPL *, const char *, WT_CKPT *, WT_CKPT *, bool);
/*
* __ckpt_load_blk_mods --
* Load the block information from the config string.
Expand Down Expand Up @@ -495,12 +495,12 @@ __ckpt_copy_blk_mods(WT_SESSION_IMPL *session, WT_CKPT *src_ckpt, WT_CKPT *dst_c
}

/*
* __wt_meta_blk_mods_load --
* meta_blk_mods_load --
* Load the block mods for a given checkpoint and set up all the information to store. Load from
* either the metadata or from a base checkpoint.
*/
int
__wt_meta_blk_mods_load(
meta_blk_mods_load(
WT_SESSION_IMPL *session, const char *config, WT_CKPT *base_ckpt, WT_CKPT *ckpt, bool rename)
{
/*
Expand Down Expand Up @@ -674,8 +674,8 @@ __meta_ckptlist_allocate_new_ckpt(
}

/* Either load block mods from the config, or from the previous checkpoint. */
WT_RET(__wt_meta_blk_mods_load(
session, config, (slot == 0 ? NULL : &ckptbase[slot - 1]), ckpt, false));
WT_RET(
meta_blk_mods_load(session, config, (slot == 0 ? NULL : &ckptbase[slot - 1]), ckpt, false));
WT_ASSERT(session, ckpt->block_metadata != NULL);

return (0);
Expand Down Expand Up @@ -1316,3 +1316,28 @@ __ckpt_version_chk(WT_SESSION_IMPL *session, const char *fname, const char *conf
WT_BTREE_MAJOR_VERSION_MAX, WT_BTREE_MINOR_VERSION_MAX);
return (0);
}

/*
* __wt_reset_blkmod --
* Reset the incremental backup information, and recreate incremental backup information to
* indicate copying the entire file.
*/
int
__wt_reset_blkmod(WT_SESSION_IMPL *session, const char *orig_config, WT_ITEM *buf)
{
WT_CKPT ckpt;
WT_DECL_RET;

WT_CLEAR(ckpt);
/*
* Replace the old file entries with new file entries. We need to recreate the incremental
* backup information to indicate copying the entire file in its bitmap.
*/
/* First load any existing backup information into a temp checkpoint structure. */
WT_RET(meta_blk_mods_load(session, orig_config, NULL, &ckpt, true));

/* Take the checkpoint structure and generate the metadata string. */
ret = __wt_ckpt_blkmod_to_meta(session, buf, &ckpt);
__wt_meta_checkpoint_free(session, &ckpt);
return (ret);
}
18 changes: 14 additions & 4 deletions src/schema/schema_create.c
Expand Up @@ -129,10 +129,11 @@ __create_file(
WT_SESSION_IMPL *session, const char *uri, bool exclusive, bool import, const char *config)
{
WT_CONFIG_ITEM cval;
WT_DECL_ITEM(buf);
WT_DECL_ITEM(val);
WT_DECL_RET;
const char *filename, **p,
*filecfg[] = {WT_CONFIG_BASE(session, file_meta), config, NULL, NULL, NULL};
*filecfg[] = {WT_CONFIG_BASE(session, file_meta), config, NULL, NULL, NULL, NULL};
char *fileconf, *filemeta;
uint32_t allocsize;
bool exists, import_repair, is_metadata;
Expand All @@ -141,6 +142,7 @@ __create_file(

import_repair = false;
is_metadata = strcmp(uri, WT_METAFILE_URI) == 0;
WT_ERR(__wt_scr_alloc(session, 1024, &buf));

filename = uri;
WT_PREFIX_SKIP_REQUIRED(session, filename, "file:");
Expand Down Expand Up @@ -201,6 +203,12 @@ __create_file(
}
WT_ERR(__wt_strndup(session, cval.str, cval.len, &filemeta));
filecfg[2] = filemeta;
/*
* If there is a file metadata provided, reconstruct the incremental backup
* information as the imported file was not part of any backup.
*/
WT_ERR(__wt_reset_blkmod(session, config, buf));
filecfg[3] = buf->mem;
} else {
/*
* If there is no file metadata provided, the user should be specifying a "repair".
Expand All @@ -218,14 +226,15 @@ __create_file(
WT_ERR(__create_file_block_manager(session, uri, filename, allocsize));

/*
* If creating an ordinary file, update the file ID and current version numbers and strip the
* incremental backup information and checkpoint LSN from the extracted metadata.
* If creating an ordinary file, update the file ID and current version numbers and strip
* checkpoint LSN from the extracted metadata. If importing an existing file, incremental backup
* information is reconstructed inside import repair or when grabbing file metadata.
*/
if (!is_metadata) {
if (!import_repair) {
WT_ERR(__wt_scr_alloc(session, 0, &val));
WT_ERR(__wt_buf_fmt(session, val,
"id=%" PRIu32 ",version=(major=%d,minor=%d),checkpoint_backup_info=,checkpoint_lsn=",
"id=%" PRIu32 ",version=(major=%d,minor=%d),checkpoint_lsn=",
++S2C(session)->next_file_id, WT_BTREE_MAJOR_VERSION_MAX,
WT_BTREE_MINOR_VERSION_MAX));
for (p = filecfg; *p != NULL; ++p)
Expand Down Expand Up @@ -261,6 +270,7 @@ __create_file(
WT_ERR(__wt_session_release_dhandle(session));

err:
__wt_scr_free(session, &buf);
__wt_scr_free(session, &val);
__wt_free(session, fileconf);
__wt_free(session, filemeta);
Expand Down
26 changes: 1 addition & 25 deletions src/schema/schema_rename.c
Expand Up @@ -8,30 +8,6 @@

#include "wt_internal.h"

/*
* __rename_blkmod --
* Reset the incremental backup information for a rename.
*/
static int
__rename_blkmod(WT_SESSION_IMPL *session, const char *oldvalue, WT_ITEM *buf)
{
WT_CKPT ckpt;
WT_DECL_RET;

WT_CLEAR(ckpt);
/*
* Replace the old file entries with new file entries. We need to recreate the incremental
* backup information to indicate copying the entire file in its bitmap.
*/
/* First load any existing backup information into a temp checkpoint structure. */
WT_RET(__wt_meta_blk_mods_load(session, oldvalue, NULL, &ckpt, true));

/* Take the checkpoint structure and generate the metadata string. */
ret = __wt_ckpt_blkmod_to_meta(session, buf, &ckpt);
__wt_meta_checkpoint_free(session, &ckpt);
return (ret);
}

/*
* __rename_file --
* WT_SESSION::rename for a file.
Expand Down Expand Up @@ -89,7 +65,7 @@ __rename_file(WT_SESSION_IMPL *session, const char *uri, const char *newuri)
WT_ERR(__wt_metadata_remove(session, uri));
filecfg[0] = oldvalue;
if (F_ISSET(S2C(session), WT_CONN_INCR_BACKUP)) {
WT_ERR(__rename_blkmod(session, oldvalue, buf));
WT_ERR(__wt_reset_blkmod(session, oldvalue, buf));
filecfg[1] = buf->mem;
} else
filecfg[1] = NULL;
Expand Down
4 changes: 2 additions & 2 deletions test/format/config.h
Expand Up @@ -186,8 +186,8 @@ static CONFIG c[] = {

/*
* 0%
* FIXME-WT-7418 and FIXME-WT-7416: Temporarily disable import until WT_ROLLBACK error and
* interaction with backup thread is fixed. Should be 20%
* FIXME-WT-7418 and FIXME-WT-7510: Temporarily disable import until WT_ROLLBACK error and
* wt_copy_and_sync error is fixed. It should be (C_BOOL, 20, 0, 0).
*/
{"import", "import table from newly created database", C_BOOL, 0, 0, 0, &g.c_import, NULL},

Expand Down
93 changes: 93 additions & 0 deletions test/suite/test_backup22.py
@@ -0,0 +1,93 @@
#!/usr/bin/env python
#
# Public Domain 2014-present MongoDB, Inc.
# Public Domain 2008-2014 WiredTiger, Inc.
#
# This is free and unencumbered software released into the public domain.
#
# Anyone is free to copy, modify, publish, use, compile, sell, or
# distribute this software, either in source code form or as a compiled
# binary, for any purpose, commercial or non-commercial, and by any
# means.
#
# In jurisdictions that recognize copyright laws, the author or authors
# of this software dedicate any and all copyright interest in the
# software to the public domain. We make this dedication for the benefit
# of the public at large and to the detriment of our heirs and
# successors. We intend this dedication to be an overt act of
# relinquishment in perpetuity of all present and future rights to this
# software under copyright law.
#
# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
# EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
# MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
# IN NO EVENT SHALL THE AUTHORS BE LIABLE FOR ANY CLAIM, DAMAGES OR
# OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
# ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
# OTHER DEALINGS IN THE SOFTWARE.

import wiredtiger, os
from wtscenario import make_scenarios
from wtbackup import backup_base

# test_backup22.py
# Test interaction between import and incremental backup.
# Test the functionality of importing dropped tables in incremental backup.
#
class test_backup22(backup_base):
create_config = 'allocation_size=512,key_format=i,value_format=i'
# Backup directory name
dir='backup.dir'
incr_dir = 'incr_backup.dir'
uri = 'test_backup22'
scenarios = make_scenarios([
('import_with_metadata', dict(repair=False,checkpoint=False)),
('import_repair', dict(repair=True,checkpoint=False)),
('import_with_metadata_ckpt', dict(repair=False,checkpoint=True)),
('import_repair_ckpt', dict(repair=True,checkpoint=True)),
])

def test_import_with_open_backup_cursor(self):
os.mkdir(self.dir)
os.mkdir(self.incr_dir)

# Create and populate the table.
table_uri = 'table:' + self.uri
self.session.create(table_uri, self.create_config)
cursor = self.session.open_cursor(table_uri)
for i in range(1, 1000):
cursor[i] = i
cursor.close()
self.session.checkpoint()

# Export the metadata for the file.
file_uri = 'file:' + self.uri + '.wt'
c = self.session.open_cursor('metadata:', None, None)
original_db_table_config = c[table_uri]
original_db_file_config = c[file_uri]
c.close()

config = 'incremental=(enabled,granularity=4k,this_id="ID1")'
bkup_c = self.session.open_cursor('backup:', None, config)
self.take_full_backup(self.dir, bkup_c)
bkup_c.close()
self.session.drop(table_uri, 'remove_files=false')

# First construct the config string for the default or repair import scenario,
# then call create to import the table.
if self.repair:
import_config = 'import=(enabled,repair=true)'
else:
import_config = '{},import=(enabled,repair=false,file_metadata=({}))'.format(
original_db_table_config, original_db_file_config)
self.session.create(table_uri, import_config)

if self.checkpoint:
self.session.checkpoint()
# Perform incremental backup with id 2 on empty directory. We want empty directory
# because we expect all files to be copied over in it's entirety.
self.take_incr_backup(self.incr_dir, 2)
self.compare_backups(self.uri, self.dir, self.incr_dir)

if __name__ == '__main__':
wttest.run()

0 comments on commit d59b4d6

Please sign in to comment.