WT-3590 Keep data consistent if writes fail during a clean shutdown (#3666)

* Add a regular checkpoint during close.

* Add a test case showing trees out of sync when writes fail during close.

* Make sure the metadata is flushed during clean shutdown.  A database-wide checkpoint can be complex, flush the metadata again afterwards to make it easy to detect that recovery can be skipped.

* Do a "soft panic" if anything goes wrong during close.  Avoid aborting diagnostic builds, but make sure there are no further writes to disk (e.g., if the checkpoint fails).
(cherry picked from commit 3061e3e6e3a28b52eda60f010f8caa2374c988ce)

(cherry picked from commit 4ae5e2d71547d79f18aed301e71a7469471ffd50)

5 files changed, 153 insertions, 7 deletions

diff --git a/src/conn/conn_api.c b/src/conn/conn_api.c
index 68d45678965..33df2d2d3e7 100644
--- a/src/conn/conn_api.c
+++ b/src/conn/conn_api.c
@@ -1086,6 +1086,43 @@ err:	/*
 			WT_TRET(wt_session->close(wt_session, config));
 		}
 
+	/*
+	 * Perform a system-wide checkpoint so that all tables are consistent
+	 * with each other.  All transactions are resolved but ignore
+	 * timestamps to make sure all data gets to disk.  Do this before
+	 * shutting down all the subsystems.  We have shut down all user
+	 * sessions, but send in true for waiting for internal races.
+	 */
+	if (!F_ISSET(conn, WT_CONN_IN_MEMORY | WT_CONN_READONLY)) {
+		s = NULL;
+		WT_TRET(__wt_open_internal_session(
+		    conn, "close_ckpt", true, 0, &s));
+		if (s != NULL) {
+			const char *checkpoint_cfg[] = {
+			    WT_CONFIG_BASE(session, WT_SESSION_checkpoint),
+			    "use_timestamp=false",
+			    NULL
+			};
+			wt_session = &s->iface;
+			WT_TRET(__wt_txn_checkpoint(s, checkpoint_cfg, true));
+
+			/*
+			 * Mark the metadata dirty so we flush it on close,
+			 * allowing recovery to be skipped.
+			 */
+			WT_WITH_DHANDLE(s, WT_SESSION_META_DHANDLE(s),
+			    __wt_tree_modify_set(s));
+
+			WT_TRET(wt_session->close(wt_session, config));
+		}
+	}
+
+	if (ret != 0) {
+		__wt_err(session, ret,
+		    "failure during close, disabling further writes");
+		F_SET(conn, WT_CONN_PANIC);
+	}
+
 	WT_TRET(__wt_connection_close(conn));
 
 	/* We no longer have a session, don't try to update it. */
diff --git a/src/conn/conn_dhandle.c b/src/conn/conn_dhandle.c
index 1816e66b0b7..04af33058b8 100644
--- a/src/conn/conn_dhandle.c
+++ b/src/conn/conn_dhandle.c
@@ -199,7 +199,7 @@ __wt_conn_btree_sync_and_close(WT_SESSION_IMPL *session, bool final, bool force)
 			/* Reset the tree's eviction priority (if any). */
 			__wt_evict_priority_clear(session);
 		}
-		if (!marked_dead || final)
+		if (!marked_dead)
 			WT_ERR(__wt_checkpoint_close(session, final));
 	}
 
@@ -673,8 +673,8 @@ restart:
 			continue;
 
 		WT_WITH_DHANDLE(session, dhandle,
-		    WT_TRET(__wt_conn_dhandle_discard_single(
-		    session, true, F_ISSET(conn, WT_CONN_IN_MEMORY))));
+		    WT_TRET(__wt_conn_dhandle_discard_single(session, true,
+		    F_ISSET(conn, WT_CONN_IN_MEMORY | WT_CONN_PANIC))));
 		goto restart;
 	}
 
@@ -699,8 +699,8 @@ restart:
 	/* Close the metadata file handle. */
 	while ((dhandle = TAILQ_FIRST(&conn->dhqh)) != NULL)
 		WT_WITH_DHANDLE(session, dhandle,
-		    WT_TRET(__wt_conn_dhandle_discard_single(
-		    session, true, F_ISSET(conn, WT_CONN_IN_MEMORY))));
+		    WT_TRET(__wt_conn_dhandle_discard_single(session, true,
+		    F_ISSET(conn, WT_CONN_IN_MEMORY | WT_CONN_PANIC))));
 
 	return (ret);
 }
diff --git a/src/conn/conn_open.c b/src/conn/conn_open.c
index 5aa07a5e766..649bfa7c81f 100644
--- a/src/conn/conn_open.c
+++ b/src/conn/conn_open.c
@@ -144,7 +144,7 @@ __wt_connection_close(WT_CONNECTION_IMPL *conn)
 	 * conditional because we allocate the log path so that printlog can
 	 * run without running logging or recovery.
 	 */
-	if (FLD_ISSET(conn->log_flags, WT_CONN_LOG_ENABLED) &&
+	if (ret == 0 && FLD_ISSET(conn->log_flags, WT_CONN_LOG_ENABLED) &&
 	    FLD_ISSET(conn->log_flags, WT_CONN_LOG_RECOVER_DONE))
 		WT_TRET(__wt_txn_checkpoint_log(
 		    session, true, WT_TXN_LOG_CKPT_STOP, NULL));
diff --git a/src/support/err.c b/src/support/err.c
index 57efde72b23..f98b1943449 100644
--- a/src/support/err.c
+++ b/src/support/err.c
@@ -494,7 +494,18 @@ __wt_panic(WT_SESSION_IMPL *session)
     WT_GCC_FUNC_ATTRIBUTE((cold))
     WT_GCC_FUNC_ATTRIBUTE((visibility("default")))
 {
-	F_SET(S2C(session), WT_CONN_PANIC);
+	WT_CONNECTION_IMPL *conn;
+
+	conn = S2C(session);
+
+	/*
+	 * If the connection has already be marked for panic, just return the
+	 * error.
+	 */
+	if (F_ISSET(conn, WT_CONN_PANIC))
+		return (WT_PANIC);
+
+	F_SET(conn, WT_CONN_PANIC);
 	__wt_err(session, WT_PANIC, "the process must exit and restart");
 
 #if defined(HAVE_DIAGNOSTIC)
diff --git a/test/suite/test_bug018.py b/test/suite/test_bug018.py
new file mode 100644
index 00000000000..7d20ebcaacb
--- /dev/null
+++ b/test/suite/test_bug018.py
@@ -0,0 +1,98 @@
+#!/usr/bin/env python
+#
+# Public Domain 2014-2017 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.
+
+from helper import copy_wiredtiger_home
+import os
+import wiredtiger, wttest
+
+# test_bug018.py
+#   JIRA WT-3590: if writing table data fails during close then tables
+# that were updated within the same transaction could get out of sync with
+# each other.
+class test_bug018(wttest.WiredTigerTestCase):
+    '''Test closing/reopening/recovering tables when writes fail'''
+
+    conn_config = 'log=(enabled)'
+
+    def setUp(self):
+        # This test uses Linux-specific code so skip on any other system.
+        if os.name != 'posix' or os.uname()[0] != 'Linux':
+            self.skipTest('Linux-specific test skipped on ' + os.name)
+        super(test_bug018, self).setUp()
+
+    def create_table(self, uri):
+        self.session.create(uri, 'key_format=S,value_format=S')
+        return self.session.open_cursor(uri)
+
+    def test_bug018(self):
+        '''Test closing multiple tables'''
+        basename = 'bug018.'
+        baseuri = 'file:' + basename
+        c1 = self.create_table(baseuri + '01.wt')
+        c2 = self.create_table(baseuri + '02.wt')
+
+        self.session.begin_transaction()
+        c1['key'] = 'value'
+        c2['key'] = 'value'
+        self.session.commit_transaction()
+
+        # Simulate a write failure by closing the file descriptor for the second
+        # table out from underneath WiredTiger.  We do this right before
+        # closing the connection so that the write error happens during close
+        # when writing out the final data.  Allow table 1 to succeed and force
+        # an erorr writing out table 2.
+        #
+        # This is Linux-specific code to figure out the file descriptor.
+        for f in os.listdir('/proc/self/fd'):
+            try:
+                if os.readlink('/proc/self/fd/' + f).endswith(basename + '02.wt'):
+                    os.close(int(f))
+            except OSError:
+                pass
+
+        # Expect an error and messages, so turn off stderr checking.
+        with self.expectedStderrPattern(''):
+            try:
+                self.close_conn()
+            except wiredtiger.WiredTigerError:
+                self.conn = None
+
+        # Make a backup for forensics in case something goes wrong.
+        backup_dir = 'BACKUP'
+        copy_wiredtiger_home('.', backup_dir, True)
+
+        # After reopening and running recovery both tables should be in
+        # sync even though table 1 was successfully written and table 2
+        # had an error on close.
+        self.open_conn()
+        c1 = self.session.open_cursor(baseuri + '01.wt')
+        c2 = self.session.open_cursor(baseuri + '02.wt')
+        self.assertEqual(list(c1), list(c2))
+
+if __name__ == '__main__':
+    wttest.run()