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.

9 files changed, 218 insertions, 60 deletions

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,37 +297,15 @@ 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
@@ -9,6 +9,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()