WT-3514 Fix checkpoint's use of stable_timestamp. (#3597)mongodb-3.5.12

* WT-3514 Fix checkpoint's use of stable_timestamp.

Prior to this change, there was a window in between checkpoint reading
the stable timestamp and using it to start a transaction.  In that
window, the application could update timestamps, making it invalid for
checkpoint's transaction to start.

Remove the undocumented "read_timestamp" config to
WT_SESSION::checkpoint, and instead copy the stable timestamp directly
into checkpoint's transaction while holding the global transaction
state locked.

* Fix non-timestamp build.

* Remove read_timestamp test case.

4 files changed, 28 insertions, 48 deletions

diff --git a/dist/api_data.py b/dist/api_data.py
index 526d0427559..37f9baedc70 100644
--- a/dist/api_data.py
+++ b/dist/api_data.py
@@ -1149,9 +1149,6 @@ methods = {
     Config('name', '', r'''
         if set, specify a name for the checkpoint (note that checkpoints
         including LSM trees may not be named)'''),
-    Config('read_timestamp', '', r'''
-        if set, create the checkpoint as of the specified timestamp''',
-        undoc=True),
     Config('target', '', r'''
         if non-empty, checkpoint the list of objects''', type='list'),
     Config('use_timestamp', 'true', r'''
diff --git a/src/config/config_def.c b/src/config/config_def.c
index 6511ee019cf..764006b024d 100644
--- a/src/config/config_def.c
+++ b/src/config/config_def.c
@@ -225,7 +225,6 @@ static const WT_CONFIG_CHECK confchk_WT_SESSION_checkpoint[] = {
 	{ "drop", "list", NULL, NULL, NULL, 0 },
 	{ "force", "boolean", NULL, NULL, NULL, 0 },
 	{ "name", "string", NULL, NULL, NULL, 0 },
-	{ "read_timestamp", "string", NULL, NULL, NULL, 0 },
 	{ "target", "list", NULL, NULL, NULL, 0 },
 	{ "use_timestamp", "boolean", NULL, NULL, NULL, 0 },
 	{ NULL, NULL, NULL, NULL, NULL, 0 }
@@ -1173,9 +1172,8 @@ static const WT_CONFIG_ENTRY config_entries[] = {
 	  confchk_WT_SESSION_begin_transaction, 6
 	},
 	{ "WT_SESSION.checkpoint",
-	  "drop=,force=false,name=,read_timestamp=,target=,"
-	  "use_timestamp=true",
-	  confchk_WT_SESSION_checkpoint, 6
+	  "drop=,force=false,name=,target=,use_timestamp=true",
+	  confchk_WT_SESSION_checkpoint, 5
 	},
 	{ "WT_SESSION.close",
 	  "",
diff --git a/src/txn/txn_ckpt.c b/src/txn/txn_ckpt.c
index dd47329a571..9065966fe8f 100644
--- a/src/txn/txn_ckpt.c
+++ b/src/txn/txn_ckpt.c
@@ -571,49 +571,17 @@ __checkpoint_prepare(WT_SESSION_IMPL *session, const char *cfg[])
 	WT_TXN *txn;
 	WT_TXN_GLOBAL *txn_global;
 	WT_TXN_STATE *txn_state;
-	char timestamp_buf[2 * WT_TIMESTAMP_SIZE + 1], timestamp_config[100];
-	const char *query_cfg[] = { WT_CONFIG_BASE(session,
-	    WT_CONNECTION_query_timestamp), "get=stable", NULL };
 	const char *txn_cfg[] = { WT_CONFIG_BASE(session,
 	    WT_SESSION_begin_transaction), "isolation=snapshot", NULL, NULL };
+	bool use_timestamp;
 
 	conn = S2C(session);
 	txn = &session->txn;
 	txn_global = &conn->txn_global;
 	txn_state = WT_SESSION_TXN_STATE(session);
 
-	/*
-	 * Someone giving us a specific timestamp overrides the general
-	 * use_timestamp.
-	 */
-	WT_RET(__wt_config_gets(session, cfg, "read_timestamp", &cval));
-	if (cval.len > 0) {
-		WT_RET(__wt_snprintf(timestamp_config, sizeof(timestamp_config),
-		    "read_timestamp=%.*s", (int)cval.len, cval.str));
-		txn_cfg[2] = timestamp_config;
-		__wt_verbose(session, WT_VERB_TIMESTAMP,
-		    "Timestamp %s : Checkpoint requested at specific timestamp",
-		    timestamp_config);
-	} else if (txn_global->has_stable_timestamp) {
-		WT_RET(__wt_config_gets(session, cfg, "use_timestamp", &cval));
-		/*
-		 * Get the stable timestamp currently set.  Then set that as
-		 * the read timestamp for the transaction.
-		 */
-		if (cval.val != 0) {
-			if ((ret = __wt_txn_global_query_timestamp(session,
-			    timestamp_buf, query_cfg)) != 0 &&
-			    ret != WT_NOTFOUND)
-				return (ret);
-			WT_RET(__wt_snprintf(timestamp_config,
-			    sizeof(timestamp_config),
-			    "read_timestamp=%s", timestamp_buf));
-			txn_cfg[2] = timestamp_config;
-			__wt_verbose(session, WT_VERB_TIMESTAMP,
-			    "Timestamp %s : Checkpoint requested at stable "
-			    "timestamp", timestamp_config);
-		}
-	}
+	WT_RET(__wt_config_gets(session, cfg, "use_timestamp", &cval));
+	use_timestamp = (cval.val != 0);
 
 	/*
 	 * Start a snapshot transaction for the checkpoint.
@@ -673,15 +641,33 @@ __checkpoint_prepare(WT_SESSION_IMPL *session, const char *cfg[])
 	 */
 	txn_state->id = txn_state->pinned_id =
 	    txn_state->metadata_pinned = WT_TXN_NONE;
-	__wt_writeunlock(session, &txn_global->rwlock);
 
 #ifdef HAVE_TIMESTAMPS
 	/*
-	 * Now that the checkpoint transaction is published, clear it from the
-	 * regular lists.
+	 * Set the checkpoint transaction's timestamp, if requested.
+	 *
+	 * We rely on having the global transaction data locked so the oldest
+	 * timestamp can't move past the stable timestamp.
 	 */
-	__wt_txn_clear_commit_timestamp(session);
-	__wt_txn_clear_read_timestamp(session);
+	WT_ASSERT(session, !F_ISSET(txn,
+	    WT_TXN_HAS_TS_COMMIT | WT_TXN_HAS_TS_READ |
+	    WT_TXN_PUBLIC_TS_COMMIT | WT_TXN_PUBLIC_TS_READ));
+
+	if (use_timestamp && txn_global->has_stable_timestamp) {
+		__wt_timestamp_set(
+		    &txn->read_timestamp, &txn_global->stable_timestamp);
+		F_SET(txn, WT_TXN_HAS_TS_READ);
+	}
+#else
+	WT_UNUSED(use_timestamp);
+#endif
+
+	__wt_writeunlock(session, &txn_global->rwlock);
+
+#ifdef HAVE_TIMESTAMPS
+	if (F_ISSET(txn, WT_TXN_HAS_TS_READ))
+		__wt_verbose_timestamp(session, &txn->read_timestamp,
+		    "Checkpoint requested at stable timestamp");
 #endif
 
 	/*
diff --git a/test/suite/test_timestamp03.py b/test/suite/test_timestamp03.py
index 734961e9e98..728200e528a 100644
--- a/test/suite/test_timestamp03.py
+++ b/test/suite/test_timestamp03.py
@@ -62,7 +62,6 @@ class test_timestamp03(wttest.WiredTigerTestCase, suite_subprocess):
         ('use_ts_def', dict(ckptcfg='', val='none')),
         ('use_ts_false', dict(ckptcfg='use_timestamp=false', val='all')),
         ('use_ts_true', dict(ckptcfg='use_timestamp=true', val='none')),
-        ('read_ts', dict(ckptcfg='read_timestamp', val='none')),
     ]
 
     conncfg = [