API Tests: Fix atomic operations workload for tests with restarts on timeouts (#8749)

* run_c_api_tests an option to disable log dump

* ApiTester: Utility for self-conflicting transactions

* ApiTester: A reusable code for making write transactions self-conflicting; Make atomic operation transactions self-conflicting

bindings/c/test/apitester/TesterApiWorkload.cpp

@@ -165,8 +165,8 @@ void ApiWorkload::populateDataTx(TTaskFct cont, std::optional<int> tenantId) {
 	}
 	execTransaction(
 	    [kvPairs](auto ctx) {
+		    ctx->makeSelfConflicting();
 		    for (const fdb::KeyValue& kv : *kvPairs) {
-			    ctx->tx().addReadConflictRange(kv.key, kv.key + fdb::Key(1, '\x00'));
 			    ctx->tx().set(kv.key, kv.value);
 		    }
 		    ctx->commit();
@@ -202,7 +202,7 @@ void ApiWorkload::clearData(TTaskFct cont) {
 		    // Make this self-conflicting, so that if we're retrying on timeouts
 		    // once we get a successful commit all previous attempts are no
 		    // longer in-flight.
-		    ctx->tx().addReadConflictRange(keyPrefix, keyPrefix + fdb::Key(1, '\xff'));
+		    ctx->makeSelfConflicting();
 		    ctx->tx().clearRange(keyPrefix, keyPrefix + fdb::Key(1, '\xff'));
 		    ctx->commit();
 	    },
@@ -257,8 +257,8 @@ void ApiWorkload::randomInsertOp(TTaskFct cont, std::optional<int> tenantId) {
 	}
 	execTransaction(
 	    [kvPairs](auto ctx) {
+		    ctx->makeSelfConflicting();
 		    for (const fdb::KeyValue& kv : *kvPairs) {
-			    ctx->tx().addReadConflictRange(kv.key, kv.key + fdb::Key(1, '\x00'));
 			    ctx->tx().set(kv.key, kv.value);
 		    }
 		    ctx->commit();
@@ -281,7 +281,7 @@ void ApiWorkload::randomClearOp(TTaskFct cont, std::optional<int> tenantId) {
 	execTransaction(
 	    [keys](auto ctx) {
 		    for (const auto& key : *keys) {
-			    ctx->tx().addReadConflictRange(key, key + fdb::Key(1, '\x00'));
+			    ctx->makeSelfConflicting();
 			    ctx->tx().clear(key);
 		    }
 		    ctx->commit();
@@ -303,7 +303,7 @@ void ApiWorkload::randomClearRangeOp(TTaskFct cont, std::optional<int> tenantId)
 	}
 	execTransaction(
 	    [begin, end](auto ctx) {
-		    ctx->tx().addReadConflictRange(begin, end);
+		    ctx->makeSelfConflicting();
 		    ctx->tx().clearRange(begin, end);
 		    ctx->commit();
 	    },

bindings/c/test/apitester/TesterAtomicOpsCorrectnessWorkload.cpp

@@ -160,7 +160,7 @@ private:
 		execTransaction(
 		    // 1. Set the key to val1
 		    [key, val1](auto ctx) {
-			    ctx->tx().addReadConflictRange(key, key + fdb::Key(1, '\x00'));
+			    ctx->makeSelfConflicting();
 			    ctx->tx().set(key, val1);
 			    ctx->commit();
 		    },
@@ -170,6 +170,7 @@ private:
 			        // retries of commit_unknown_result would cause the operation to be applied multiple times, see
 			        // https://github.com/apple/foundationdb/issues/1321.
 			        [key, opType, val1, val2](auto ctx) {
+				        ctx->makeSelfConflicting();
 				        auto f = ctx->tx().get(key, false);
 				        ctx->continueAfter(f, [ctx, f, opType, key, val1, val2]() {
 					        auto outputVal = f.get();
@@ -223,6 +224,7 @@ private:
 		execTransaction(
 		    // 1. Perform SetVersionstampedKey operation.
 		    [key, val, versionstamp_f](auto ctx) {
+			    ctx->makeSelfConflicting();
 			    ctx->tx().atomicOp(key, val, FDBMutationType::FDB_MUTATION_TYPE_SET_VERSIONSTAMPED_KEY);
 			    *versionstamp_f = ctx->tx().getVersionstamp();
 			    ctx->commit();
@@ -259,6 +261,7 @@ private:
 		execTransaction(
 		    // 1. Perform SetVersionstampedValue operation.
 		    [key, val, versionstamp_f](auto ctx) {
+			    ctx->makeSelfConflicting();
 			    ctx->tx().atomicOp(key, val, FDBMutationType::FDB_MUTATION_TYPE_SET_VERSIONSTAMPED_VALUE);
 			    *versionstamp_f = ctx->tx().getVersionstamp();
 			    ctx->commit();
@@ -296,14 +299,15 @@ private:
 		execTransaction(
 		    // 1. Set the key to initial value
 		    [key, val](auto ctx) {
+			    ctx->makeSelfConflicting();
 			    ctx->tx().set(key, val);
-			    ctx->tx().addReadConflictRange(key, key + fdb::Key(1, '\x00'));
 			    ctx->commit();
 		    },
 		    [this, key, val, cont]() {
 			    execTransaction(
 			        // 2. Perform CompareAndClear operation.
 			        [key, val](auto ctx) {
+				        ctx->makeSelfConflicting();
 				        ctx->tx().atomicOp(key, val, FDBMutationType::FDB_MUTATION_TYPE_COMPARE_AND_CLEAR);
 				        ctx->commit();
 			        },

bindings/c/test/apitester/TesterTransactionExecutor.cpp

@@ -81,7 +81,8 @@ public:
 	                       bool restartOnTimeout)
 	  : executor(executor), startFct(startFct), contAfterDone(cont), scheduler(scheduler), retryLimit(retryLimit),
 	    txState(TxState::IN_PROGRESS), commitCalled(false), bgBasePath(bgBasePath), tenantName(tenantName),
-	    transactional(transactional), restartOnTimeout(restartOnTimeout) {
+	    transactional(transactional), restartOnTimeout(restartOnTimeout),
+	    selfConflictingKey(Random::get().randomByteStringLowerCase(8, 8)) {
 		databaseCreateErrorInjected = executor->getOptions().injectDatabaseCreateErrors &&
 		                              Random::get().randomBool(executor->getOptions().databaseCreateErrorRatio);
 		if (databaseCreateErrorInjected) {
@@ -158,6 +159,15 @@ public:
 		contAfterDone(fdb::Error::success());
 	}
 
+	void makeSelfConflicting() override {
+		ASSERT(transactional);
+		if (restartOnTimeout) {
+			auto transaction = tx();
+			transaction.addReadConflictRange(selfConflictingKey, selfConflictingKey + fdb::Key(1, '\x00'));
+			transaction.addWriteConflictRange(selfConflictingKey, selfConflictingKey + fdb::Key(1, '\x00'));
+		}
+	}
+
 	std::string getBGBasePath() override { return bgBasePath; }
 
 	virtual void onError(fdb::Error err) override {
@@ -373,6 +383,9 @@ protected:
 
 	// Specifies whether the operation is transactional
 	const bool transactional;
+
+	// A randomly generated key for making transaction self-conflicting
+	const fdb::Key selfConflictingKey;
 };
 
 /**

bindings/c/test/apitester/TesterTransactionExecutor.h

@@ -58,6 +58,10 @@ public:
 	// Mark the transaction as completed without committing it (for read transactions)
 	virtual void done() = 0;
 
+	// Make the transaction self-conflicting if it needs to be restarted on timeouts.
+	// Must be called for all write transactions that may need to be restarted on timeouts
+	virtual void makeSelfConflicting() = 0;
+
 	// Plumbing for blob granule base path
 	virtual std::string getBGBasePath() = 0;
 

bindings/c/test/apitester/run_c_api_tests.py

@@ -142,7 +142,7 @@ def run_tester(args, cluster, test_file):
         else:
             reason = "exit code: %d" % ret_code
         get_logger().error("\n'%s' did not complete succesfully (%s)" % (cmd[0], reason))
-        if log_dir is not None:
+        if log_dir is not None and not args.disable_log_dump:
             dump_client_logs(log_dir)
 
     get_logger().info("")
@@ -258,6 +258,11 @@ def parse_args(argv):
         dest="knobs",
         help="[lowercase-knob-name]=[knob-value] (there may be multiple --knob options)",
     )
+    parser.add_argument(
+        "--disable-log-dump",
+        help="Do not dump logs on error",
+        action="store_true",
+    )
 
     return parser.parse_args(argv)
 

bindings/c/test/fdb_api.hpp

@@ -663,12 +663,18 @@ public:
 		native::fdb_transaction_clear_range(tr.get(), begin.data(), intSize(begin), end.data(), intSize(end));
 	}
 
-	void addReadConflictRange(KeyRef begin, KeyRef end) {
+	void addConflictRange(KeyRef begin, KeyRef end, FDBConflictRangeType rangeType) {
 		if (auto err = Error(native::fdb_transaction_add_conflict_range(
-		        tr.get(), begin.data(), intSize(begin), end.data(), intSize(end), FDB_CONFLICT_RANGE_TYPE_READ))) {
+		        tr.get(), begin.data(), intSize(begin), end.data(), intSize(end), rangeType))) {
 			throwError("fdb_transaction_add_conflict_range returned error: ", err);
 		}
 	}
+
+	void addReadConflictRange(KeyRef begin, KeyRef end) { addConflictRange(begin, end, FDB_CONFLICT_RANGE_TYPE_READ); }
+
+	void addWriteConflictRange(KeyRef begin, KeyRef end) {
+		addConflictRange(begin, end, FDB_CONFLICT_RANGE_TYPE_WRITE);
+	}
 };
 
 class Tenant final {