Do not partially reset a transaction when it is committed or fails to commit with an error.

bindings/c/test/unit/unit_tests.cpp

@@ -2177,6 +2177,81 @@ TEST_CASE("monitor_network_busyness") {
 	CHECK(containsGreaterZero);
 }
 
+// Commit a transaction and confirm it has not been reset
+TEST_CASE("commit_does_not_reset") {
+	fdb::Transaction tr(db);
+	fdb::Transaction tr2(db);
+
+	// Commit two transactions, one that will fail with conflict and the other
+	// that will succeed. Ensure both transactions are not reset at the end.
+	while (1) {
+		fdb::Int64Future tr1GrvFuture = tr.get_read_version();
+		fdb_error_t err = wait_future(tr1GrvFuture);
+		if (err) {
+			fdb::EmptyFuture tr1OnErrorFuture = tr.on_error(err);
+			fdb_check(wait_future(tr1OnErrorFuture));
+			continue;
+		}
+
+		int64_t tr1StartVersion;
+		CHECK(!tr1GrvFuture.get(&tr1StartVersion));
+
+		fdb::Int64Future tr2GrvFuture = tr2.get_read_version();
+		err = wait_future(tr2GrvFuture);
+
+		if (err) {
+			fdb::EmptyFuture tr2OnErrorFuture = tr2.on_error(err);
+			fdb_check(wait_future(tr2OnErrorFuture));
+			continue;
+		}
+
+		int64_t tr2StartVersion;
+		CHECK(!tr2GrvFuture.get(&tr2StartVersion));
+
+		tr.set(key("foo"), "bar");
+		fdb::EmptyFuture tr1CommitFuture = tr.commit();
+		err = wait_future(tr1CommitFuture);
+		if (err) {
+			fdb::EmptyFuture tr1OnErrorFuture = tr.on_error(err);
+			fdb_check(wait_future(tr1OnErrorFuture));
+			continue;
+		}
+
+		fdb_check(tr2.add_conflict_range(key("foo"), strinc(key("foo")), FDB_CONFLICT_RANGE_TYPE_READ));
+		tr2.set(key("foo"), "bar");
+		fdb::EmptyFuture tr2CommitFuture = tr2.commit();
+		err = wait_future(tr2CommitFuture);
+		CHECK(err == 1020); // not_committed
+
+		fdb::Int64Future tr1GrvFuture2 = tr.get_read_version();
+		err = wait_future(tr1GrvFuture2);
+		if (err) {
+			fdb::EmptyFuture tr1OnErrorFuture = tr.on_error(err);
+			fdb_check(wait_future(tr1OnErrorFuture));
+			continue;
+		}
+
+		int64_t tr1EndVersion;
+		CHECK(!tr1GrvFuture2.get(&tr1EndVersion));
+
+		fdb::Int64Future tr2GrvFuture2 = tr2.get_read_version();
+		err = wait_future(tr2GrvFuture2);
+		if (err) {
+			fdb::EmptyFuture tr2OnErrorFuture = tr2.on_error(err);
+			fdb_check(wait_future(tr2OnErrorFuture));
+			continue;
+		}
+
+		int64_t tr2EndVersion;
+		CHECK(!tr2GrvFuture2.get(&tr2EndVersion));
+
+		// If we reset the transaction, then the read version will change
+		CHECK(tr1StartVersion == tr1EndVersion);
+		CHECK(tr2StartVersion == tr2EndVersion);
+		break;
+	}
+}
+
 int main(int argc, char** argv) {
 	if (argc < 3) {
 		std::cout << "Unit tests for the FoundationDB C API.\n"

documentation/sphinx/source/api-version-upgrade-guide.rst

@@ -25,6 +25,8 @@ API version 700
 General
 -------
 
+* Committing a transaction will no longer partially reset it. In particular, getting the read version from a transaction that has committed or failed to commit with an error will return the original read version.
+
 Python bindings
 ---------------
 

documentation/sphinx/source/release-notes/release-notes-700.rst

@@ -91,6 +91,7 @@ Other Changes
 * The ``foundationdb`` service installed by the RPM packages will now automatically restart ``fdbmonitor`` after 60 seconds when it fails. `(PR #3841) <https://github.com/apple/foundationdb/pull/3841>`_
 * Capture output of forked snapshot processes in trace events. `(PR #4254) <https://github.com/apple/foundationdb/pull/4254/files>`_
 * Add ErrorKind field to Severity 40 trace events. `(PR #4741) <https://github.com/apple/foundationdb/pull/4741/files>`_
+* Committing a transaction will no longer partially reset it as of API version 700. `(PR #) <https://github.com/apple/foundationdb/pull//files>`_
 
 Earlier release notes
 ---------------------

fdbclient/NativeAPI.actor.cpp

@@ -5244,7 +5244,10 @@ ACTOR Future<Void> commitAndWatch(Transaction* self) {
 			self->setupWatches();
 		}
 
-		self->reset();
+		if (!self->apiVersionAtLeast(700)) {
+			self->reset();
+		}
+
 		return Void();
 	} catch (Error& e) {
 		if (e.code() != error_code_actor_cancelled) {
@@ -5253,7 +5256,10 @@ ACTOR Future<Void> commitAndWatch(Transaction* self) {
 			}
 
 			self->versionstampPromise.sendError(transaction_invalid_version());
-			self->reset();
+
+			if (!self->apiVersionAtLeast(700)) {
+				self->reset();
+			}
 		}
 
 		throw;