From 4a61b73ae13d53848c7773075a9e7d5e1386389b Mon Sep 17 00:00:00 2001 From: sueloverso Date: Wed, 26 Oct 2016 10:14:30 -0400 Subject: WT-2968 Don't open file handles on backup. (#3101) * WT-2968 Don't open file handles on backup. * whitespace, KNF * Add timing to test and populate with thousands of tables. * Remove macro and meta function. Add schema backup check function. * Refactor new function per review. * Remove cursor backup entry data structure. Simplify test a bit. * Schema ops walk file list in memory instead of on-disk file. * Set connection list entry to NULL before freeing memory. * Remove call to backup_stop on error. Errors are handled above. * Add comment * Fix test description. * Fix new test to use new DataSet code. --- src/cursor/cur_backup.c | 73 ++++++--------------- src/include/connection.h | 3 +- src/include/cursor.h | 6 +- src/include/extern.h | 1 + src/include/wt_internal.h | 2 - src/schema/schema_drop.c | 1 + src/schema/schema_rename.c | 2 + src/schema/schema_util.c | 34 ++++++++++ test/suite/test_backup06.py | 156 ++++++++++++++++++++++++++++++++++++++++++++ 9 files changed, 218 insertions(+), 60 deletions(-) create mode 100644 test/suite/test_backup06.py diff --git a/src/cursor/cur_backup.c b/src/cursor/cur_backup.c index 3a3ff7de92b..e846728dc87 100644 --- a/src/cursor/cur_backup.c +++ b/src/cursor/cur_backup.c @@ -31,13 +31,13 @@ __curbackup_next(WT_CURSOR *cursor) cb = (WT_CURSOR_BACKUP *)cursor; CURSOR_API_CALL(cursor, session, next, NULL); - if (cb->list == NULL || cb->list[cb->next].name == NULL) { + if (cb->list == NULL || cb->list[cb->next] == NULL) { F_CLR(cursor, WT_CURSTD_KEY_SET); WT_ERR(WT_NOTFOUND); } - cb->iface.key.data = cb->list[cb->next].name; - cb->iface.key.size = strlen(cb->list[cb->next].name) + 1; + cb->iface.key.data = cb->list[cb->next]; + cb->iface.key.size = strlen(cb->list[cb->next]) + 1; ++cb->next; F_SET(cursor, WT_CURSTD_KEY_INT); @@ -226,9 +226,13 @@ __backup_start( * holds the lock until it's finished the checkpoint, otherwise we * could start a hot backup that would race with an already-started * checkpoint. + * + * We are holding the checkpoint and schema locks so schema operations + * will not see the backup file list until it is complete and valid. */ __wt_writelock(session, conn->hot_backup_lock); conn->hot_backup = true; + conn->hot_backup_list = NULL; __wt_writeunlock(session, conn->hot_backup_lock); /* We're the lock holder, we own cleanup. */ @@ -293,36 +297,14 @@ err: /* Close the hot backup file. */ if (ret == 0) { WT_ASSERT(session, dest != NULL); WT_TRET(__wt_fs_rename(session, WT_BACKUP_TMP, dest, false)); + __wt_writelock(session, conn->hot_backup_lock); + conn->hot_backup_list = cb->list; + __wt_writeunlock(session, conn->hot_backup_lock); } return (ret); } -/* - * __backup_cleanup_handles -- - * Release and free all btree handles held by the backup. - */ -static int -__backup_cleanup_handles(WT_SESSION_IMPL *session, WT_CURSOR_BACKUP *cb) -{ - WT_CURSOR_BACKUP_ENTRY *p; - WT_DECL_RET; - - if (cb->list == NULL) - return (0); - - /* Release the handles, free the file names, free the list itself. */ - for (p = cb->list; p->name != NULL; ++p) { - if (p->handle != NULL) - WT_WITH_DHANDLE(session, p->handle, - WT_TRET(__wt_session_release_btree(session))); - __wt_free(session, p->name); - } - - __wt_free(session, cb->list); - return (ret); -} - /* * __backup_stop -- * Stop a backup. @@ -335,8 +317,12 @@ __backup_stop(WT_SESSION_IMPL *session, WT_CURSOR_BACKUP *cb) conn = S2C(session); - /* Release all btree handles held by the backup. */ - WT_TRET(__backup_cleanup_handles(session, cb)); + /* Release all btree names held by the backup. */ + __wt_writelock(session, conn->hot_backup_lock); + conn->hot_backup_list = NULL; + __wt_writeunlock(session, conn->hot_backup_lock); + if (cb->list != NULL) + __wt_free(session, cb->list); /* Remove any backup specific file. */ WT_TRET(__wt_backup_file_remove(session)); @@ -513,40 +499,23 @@ static int __backup_list_append( WT_SESSION_IMPL *session, WT_CURSOR_BACKUP *cb, const char *uri) { - WT_CURSOR_BACKUP_ENTRY *p; - WT_DATA_HANDLE *old_dhandle; - WT_DECL_RET; + char **p; const char *name; /* Leave a NULL at the end to mark the end of the list. */ WT_RET(__wt_realloc_def(session, &cb->list_allocated, cb->list_next + 2, &cb->list)); p = &cb->list[cb->list_next]; - p[0].name = p[1].name = NULL; - p[0].handle = p[1].handle = NULL; + p[0] = p[1] = NULL; name = uri; /* - * If it's a file in the database, get a handle for the underlying - * object (this handle blocks schema level operations, for example - * WT_SESSION.drop or an LSM file discard after level merging). - * - * If the handle is busy (e.g., it is being bulk-loaded), silently skip - * it. We have a special fake checkpoint in the metadata, and recovery - * will recreate an empty file. + * If it's a file in the database we need to remove the prefix. */ - if (WT_PREFIX_MATCH(uri, "file:")) { + if (WT_PREFIX_MATCH(uri, "file:")) name += strlen("file:"); - old_dhandle = session->dhandle; - ret = __wt_session_get_btree(session, uri, NULL, NULL, 0); - p->handle = session->dhandle; - session->dhandle = old_dhandle; - if (ret != 0) - return (ret == EBUSY ? 0 : ret); - } - /* * !!! * Assumes metadata file entries map one-to-one to physical files. @@ -556,7 +525,7 @@ __backup_list_append( * that for now, that block manager might not even support physical * copying of files by applications. */ - WT_RET(__wt_strdup(session, name, &p->name)); + WT_RET(__wt_strdup(session, name, p)); ++cb->list_next; return (0); diff --git a/src/include/connection.h b/src/include/connection.h index d7c3bf69686..88c672ea49b 100644 --- a/src/include/connection.h +++ b/src/include/connection.h @@ -269,7 +269,8 @@ struct __wt_connection_impl { WT_TXN_GLOBAL txn_global; /* Global transaction state */ WT_RWLOCK *hot_backup_lock; /* Hot backup serialization */ - bool hot_backup; + bool hot_backup; /* Hot backup in progress */ + char **hot_backup_list; /* Hot backup file list */ WT_SESSION_IMPL *ckpt_session; /* Checkpoint thread session */ wt_thread_t ckpt_tid; /* Checkpoint thread */ diff --git a/src/include/cursor.h b/src/include/cursor.h index 524e65de12f..d522abc2a56 100644 --- a/src/include/cursor.h +++ b/src/include/cursor.h @@ -59,10 +59,6 @@ 0 /* uint32_t flags */ \ } -struct __wt_cursor_backup_entry { - char *name; /* File name */ - WT_DATA_HANDLE *handle; /* Handle */ -}; struct __wt_cursor_backup { WT_CURSOR iface; @@ -70,7 +66,7 @@ struct __wt_cursor_backup { WT_FSTREAM *bfs; /* Backup file stream */ uint32_t maxid; /* Maximum log file ID seen */ - WT_CURSOR_BACKUP_ENTRY *list; /* List of files to be copied. */ + char **list; /* List of files to be copied. */ size_t list_allocated; size_t list_next; diff --git a/src/include/extern.h b/src/include/extern.h index 89b83ce6969..84cd1b6aa8c 100644 --- a/src/include/extern.h +++ b/src/include/extern.h @@ -583,6 +583,7 @@ extern int __wt_curstat_table_init(WT_SESSION_IMPL *session, const char *uri, co extern int __wt_schema_truncate( WT_SESSION_IMPL *session, const char *uri, const char *cfg[]) WT_GCC_FUNC_DECL_ATTRIBUTE((warn_unused_result)); extern int __wt_range_truncate(WT_CURSOR *start, WT_CURSOR *stop) WT_GCC_FUNC_DECL_ATTRIBUTE((warn_unused_result)); extern int __wt_schema_range_truncate( WT_SESSION_IMPL *session, WT_CURSOR *start, WT_CURSOR *stop) WT_GCC_FUNC_DECL_ATTRIBUTE((warn_unused_result)); +extern int __wt_schema_backup_check(WT_SESSION_IMPL *session, const char *name) WT_GCC_FUNC_DECL_ATTRIBUTE((warn_unused_result)); extern WT_DATA_SOURCE *__wt_schema_get_source(WT_SESSION_IMPL *session, const char *name); extern int __wt_str_name_check(WT_SESSION_IMPL *session, const char *str) WT_GCC_FUNC_DECL_ATTRIBUTE((warn_unused_result)); extern int __wt_name_check(WT_SESSION_IMPL *session, const char *str, size_t len) WT_GCC_FUNC_DECL_ATTRIBUTE((warn_unused_result)); diff --git a/src/include/wt_internal.h b/src/include/wt_internal.h index d354757c592..dc3e3a0b6c0 100644 --- a/src/include/wt_internal.h +++ b/src/include/wt_internal.h @@ -126,8 +126,6 @@ struct __wt_connection_stats; typedef struct __wt_connection_stats WT_CONNECTION_STATS; struct __wt_cursor_backup; typedef struct __wt_cursor_backup WT_CURSOR_BACKUP; -struct __wt_cursor_backup_entry; - typedef struct __wt_cursor_backup_entry WT_CURSOR_BACKUP_ENTRY; struct __wt_cursor_btree; typedef struct __wt_cursor_btree WT_CURSOR_BTREE; struct __wt_cursor_bulk; diff --git a/src/schema/schema_drop.c b/src/schema/schema_drop.c index ead8cc45c62..96bd3452e1f 100644 --- a/src/schema/schema_drop.c +++ b/src/schema/schema_drop.c @@ -28,6 +28,7 @@ __drop_file( if (!WT_PREFIX_SKIP(filename, "file:")) return (EINVAL); + WT_RET(__wt_schema_backup_check(session, filename)); /* Close all btree handles associated with this file. */ WT_WITH_HANDLE_LIST_LOCK(session, ret = __wt_conn_dhandle_close_all(session, uri, force)); diff --git a/src/schema/schema_rename.c b/src/schema/schema_rename.c index bc92c882117..8b3660c61e6 100644 --- a/src/schema/schema_rename.c +++ b/src/schema/schema_rename.c @@ -29,6 +29,8 @@ __rename_file( !WT_PREFIX_SKIP(newfile, "file:")) return (EINVAL); + WT_RET(__wt_schema_backup_check(session, filename)); + WT_RET(__wt_schema_backup_check(session, newfile)); /* Close any btree handles in the file. */ WT_WITH_HANDLE_LIST_LOCK(session, ret = __wt_conn_dhandle_close_all(session, uri, false)); diff --git a/src/schema/schema_util.c b/src/schema/schema_util.c index d1c84dc8d85..808a7fc36cf 100644 --- a/src/schema/schema_util.c +++ b/src/schema/schema_util.c @@ -8,6 +8,40 @@ #include "wt_internal.h" +/* + * __wt_schema_backup_check -- + * Check if a backup cursor is open and give an error if the schema + * operation will conflict. This is called after the schema operations + * have taken the schema lock so no hot backup cursor can be created until + * this is done. + */ +int +__wt_schema_backup_check(WT_SESSION_IMPL *session, const char *name) +{ + WT_CONNECTION_IMPL *conn; + WT_DECL_RET; + int i; + char **backup_list; + + conn = S2C(session); + if (!conn->hot_backup) + return (0); + __wt_readlock(session, conn->hot_backup_lock); + if (!conn->hot_backup) { + __wt_readunlock(session, conn->hot_backup_lock); + return (0); + } + for (i = 0, backup_list = conn->hot_backup_list; + backup_list[i] != NULL; ++i) { + if (strcmp(backup_list[i], name) == 0) { + ret = EBUSY; + break; + } + } + __wt_readunlock(session, conn->hot_backup_lock); + return (ret); +} + /* * __wt_schema_get_source -- * Find a matching data source or report an error. diff --git a/test/suite/test_backup06.py b/test/suite/test_backup06.py new file mode 100644 index 00000000000..a9895f34a9c --- /dev/null +++ b/test/suite/test_backup06.py @@ -0,0 +1,156 @@ +#!/usr/bin/env python +# +# Public Domain 2014-2016 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 glob +import os +import shutil +import string +from suite_subprocess import suite_subprocess +import wiredtiger, wttest +from wiredtiger import stat +from wtdataset import SimpleDataSet, ComplexDataSet, ComplexLSMDataSet + +# test_backup06.py +# Test that opening a backup cursor does not open file handles. +class test_backup06(wttest.WiredTigerTestCase, suite_subprocess): + conn_config = 'statistics=(fast)' + # This will create several hundred tables. + num_table_sets = 20 + pfx='test_backup' + + # We try to do some schema operations. Have some well + # known names. + schema_uri = 'file:schema_test' + rename_uri = 'file:new_test' + trename_uri = 'table:new_test' + + fobjs = [ + ( 'file:' + pfx + '.1', SimpleDataSet), + ( 'file:' + pfx + '.2', SimpleDataSet), + ] + tobjs = [ + ('table:' + pfx + '.3', SimpleDataSet), + ('table:' + pfx + '.4', SimpleDataSet), + ('table:' + pfx + '.5', ComplexDataSet), + ('table:' + pfx + '.6', ComplexDataSet), + ('table:' + pfx + '.7', ComplexLSMDataSet), + ('table:' + pfx + '.8', ComplexLSMDataSet), + ] + + # Populate a set of objects. + def populate_many(self): + for t in range(0, self.num_table_sets): + for i in self.fobjs: + uri = i[0] + "." + str(t) + i[1](self, uri, 10).populate() + for i in self.tobjs: + uri = i[0] + "." + str(t) + i[1](self, uri, 10).populate() + + def populate(self): + for i in self.fobjs: + i[1](self, i[0], 100).populate() + for i in self.tobjs: + i[1](self, i[0], 100).populate() + + # Test that the open handle count does not change. + def test_cursor_open_handles(self): + self.populate_many() + # Close and reopen the connection so the populate dhandles are + # not in the list. + self.reopen_conn() + + # Confirm that opening a backup cursor does not open + # file handles. + stat_cursor = self.session.open_cursor('statistics:', None, None) + dh_before = stat_cursor[stat.conn.dh_conn_handle_count][2] + stat_cursor.close() + cursor = self.session.open_cursor('backup:', None, None) + stat_cursor = self.session.open_cursor('statistics:', None, None) + dh_after = stat_cursor[stat.conn.dh_conn_handle_count][2] + stat_cursor.close() + if (dh_before != dh_after): + print "Dhandles open before backup open: " + str(dh_before) + print "Dhandles open after backup open: " + str(dh_after) + self.assertEqual(dh_after == dh_before, True) + cursor.close() + + def test_cursor_schema_protect(self): + schema_uri = 'file:schema_test' + rename_uri = 'file:new_test' + trename_uri = 'table:new_test' + + # + # Set up a number of tables. Close and reopen the connection so that + # we do not have open dhandles. Then we want to open a backup cursor + # testing both with and without the configuration setting. + # We want to confirm that we open data handles when using schema + # protection and we do not open the data handles when set to false. + # We also want to make sure we detect and get an error when set to + # false. When set to true the open handles protect against schema + # operations. + self.populate() + cursor = self.session.open_cursor('backup:', None, None) + # Check that we can create. + self.session.create(schema_uri, None) + for i in self.fobjs: + self.assertRaises(wiredtiger.WiredTigerError, + lambda: self.session.drop(i[0], None)) + self.assertRaises(wiredtiger.WiredTigerError, + lambda: self.session.rename(i[0], rename_uri)) + for i in self.tobjs: + self.assertRaises(wiredtiger.WiredTigerError, + lambda: self.session.drop(i[0], None)) + self.assertRaises(wiredtiger.WiredTigerError, + lambda: self.session.rename(i[0], trename_uri)) + cursor.close() + + # Test cursor reset runs through the list twice. + def test_cursor_reset(self): + self.populate() + cursor = self.session.open_cursor('backup:', None, None) + i = 0 + while True: + ret = cursor.next() + if ret != 0: + break + i += 1 + self.assertEqual(ret, wiredtiger.WT_NOTFOUND) + total = i * 2 + cursor.reset() + while True: + ret = cursor.next() + if ret != 0: + break + i += 1 + self.assertEqual(ret, wiredtiger.WT_NOTFOUND) + self.assertEqual(i, total) + cursor.close() + +if __name__ == '__main__': + wttest.run() -- cgit v1.2.1