5s - fuzz write knob in simulation

fdbclient/ClientKnobCollection.cpp

@@ -21,16 +21,16 @@
 #include "fdbclient/ClientKnobCollection.h"
 
 ClientKnobCollection::ClientKnobCollection(Randomize randomize, IsSimulated isSimulated)
-  : flowKnobs(randomize, isSimulated), clientKnobs(randomize) {}
+  : flowKnobs(randomize, isSimulated), clientKnobs(randomize, isSimulated) {}
 
 void ClientKnobCollection::initialize(Randomize randomize, IsSimulated isSimulated) {
 	flowKnobs.initialize(randomize, isSimulated);
-	clientKnobs.initialize(randomize);
+	clientKnobs.initialize(randomize, isSimulated);
 }
 
 void ClientKnobCollection::reset(Randomize randomize, IsSimulated isSimulated) {
 	flowKnobs.reset(randomize, isSimulated);
-	clientKnobs.reset(randomize);
+	clientKnobs.reset(randomize, isSimulated);
 }
 
 Optional<KnobValue> ClientKnobCollection::tryParseKnobValue(std::string const& knobName,

fdbclient/ClientKnobs.cpp

@@ -23,16 +23,27 @@
 #include "fdbclient/SystemData.h"
 #include "fdbclient/Tenant.h"
 #include "flow/IRandom.h"
+#include "flow/Knobs.h"
 #include "flow/UnitTest.h"
 #include "flow/flow.h"
 
+// Returns a deterministically random transaction timeout value for simulation testing.
+// More weight is given to the famous 5s timeout, but [1, 10] range is returned with lower weight.
+int randomTxnTimeoutSeconds() {
+	if (deterministicRandom()->truePercent(90)) {
+		return 5;
+	} else {
+		return deterministicRandom()->randomInt(1, 11); // [1, 10]
+	}
+}
+
 #define init(...) KNOB_FN(__VA_ARGS__, INIT_ATOMIC_KNOB, INIT_KNOB)(__VA_ARGS__)
 
-ClientKnobs::ClientKnobs(Randomize randomize) {
-	initialize(randomize);
+ClientKnobs::ClientKnobs(Randomize randomize, IsSimulated isSimulated) {
+	initialize(randomize, isSimulated);
 }
 
-void ClientKnobs::initialize(Randomize randomize) {
+void ClientKnobs::initialize(Randomize randomize, IsSimulated isSimulated) {
 	// clang-format off
 
 	init( TOO_MANY,                            1000000 );
@@ -126,7 +137,7 @@ void ClientKnobs::initialize(Randomize randomize) {
 
 	// Versions -- knobs that control 5s timeout
 	init( VERSIONS_PER_SECOND,                     1e6 ); // Must be the same as SERVER_KNOBS->VERSIONS_PER_SECOND
-	init( MAX_WRITE_TRANSACTION_LIFE_VERSIONS,     5 * VERSIONS_PER_SECOND);  // Must be the same as SERVER_KNOBS->MAX_WRITE_TRANSACTION_LIFE_VERSIONS
+	init( MAX_WRITE_TRANSACTION_LIFE_VERSIONS,     5 * VERSIONS_PER_SECOND);  if (isSimulated) MAX_WRITE_TRANSACTION_LIFE_VERSIONS = randomTxnTimeoutSeconds() * VERSIONS_PER_SECOND;
 
 	// Core
 	init( CORE_VERSIONSPERSECOND,		           1e6 );
@@ -354,13 +365,13 @@ void ClientKnobs::initialize(Randomize randomize) {
 
 TEST_CASE("/fdbclient/knobs/initialize") {
 	// This test depends on TASKBUCKET_TIMEOUT_VERSIONS being defined as a constant multiple of CORE_VERSIONSPERSECOND
-	ClientKnobs clientKnobs(Randomize::False);
+	ClientKnobs clientKnobs(Randomize::False, IsSimulated::False);
 	int64_t initialCoreVersionsPerSecond = clientKnobs.CORE_VERSIONSPERSECOND;
 	int initialTaskBucketTimeoutVersions = clientKnobs.TASKBUCKET_TIMEOUT_VERSIONS;
 	clientKnobs.setKnob("core_versionspersecond", initialCoreVersionsPerSecond * 2);
 	ASSERT_EQ(clientKnobs.CORE_VERSIONSPERSECOND, initialCoreVersionsPerSecond * 2);
 	ASSERT_EQ(clientKnobs.TASKBUCKET_TIMEOUT_VERSIONS, initialTaskBucketTimeoutVersions);
-	clientKnobs.initialize(Randomize::False);
+	clientKnobs.initialize(Randomize::False, IsSimulated::False);
 	ASSERT_EQ(clientKnobs.CORE_VERSIONSPERSECOND, initialCoreVersionsPerSecond * 2);
 	ASSERT_EQ(clientKnobs.TASKBUCKET_TIMEOUT_VERSIONS, initialTaskBucketTimeoutVersions * 2);
 	return Void();

fdbclient/ServerKnobCollection.cpp

@@ -48,7 +48,16 @@ Optional<KnobValue> ServerKnobCollection::tryParseKnobValue(std::string const& k
 }
 
 bool ServerKnobCollection::trySetKnob(std::string const& knobName, KnobValueRef const& knobValue) {
-	return clientKnobCollection.trySetKnob(knobName, knobValue) || knobValue.visitSetKnob(knobName, serverKnobs);
+	// Do not short circuit by directly returning:
+	//     clientKnobCollection.trySetKnob(knobName, knobValue) || knobValue.visitSetKnob(knobName, serverKnobs)
+	// This is because some knobs have the same name in client and server e.g. MAX_WRITE_TRANSACTION_LIFE_VERSIONS
+	// When setting such knobs, we want both client and server knob to have its value updated
+	// Short circuiting would mean that server knob FOO won't be updated if client knob FOO was updated
+	// Instead, we attempt setting client and server knobs in separate statements, and return true
+	// if at least one of the set attempts was succesful.
+	const bool setClientKnob = clientKnobCollection.trySetKnob(knobName, knobValue);
+	const bool setServerKnob = knobValue.visitSetKnob(knobName, serverKnobs);
+	return setClientKnob || setServerKnob;
 }
 
 bool ServerKnobCollection::isAtomic(std::string const& knobName) const {

fdbclient/ServerKnobs.cpp

@@ -19,34 +19,27 @@
  */
 
 #include "fdbclient/ServerKnobs.h"
+#include "fdbclient/IKnobCollection.h"
 #include "flow/CompressionUtils.h"
 #include "flow/IRandom.h"
 #include "flow/flow.h"
 
+extern int randomTxnTimeoutSeconds();
+
 #define init(...) KNOB_FN(__VA_ARGS__, INIT_ATOMIC_KNOB, INIT_KNOB)(__VA_ARGS__)
 
 ServerKnobs::ServerKnobs(Randomize randomize, ClientKnobs* clientKnobs, IsSimulated isSimulated) {
 	initialize(randomize, clientKnobs, isSimulated);
 }
 
-// Returns a deterministically random transaction timeout value for simulation testing.
-// More weight is given to the famous 5s timeout, but [1, 10] range is returned with lower weight.
-int randomTxnTimeoutSeconds() {
-	if (deterministicRandom()->truePercent(90)) {
-		return 5;
-	} else {
-		return deterministicRandom()->randomInt(1, 11); // [1, 10]
-	}
-}
-
 void ServerKnobs::initialize(Randomize randomize, ClientKnobs* clientKnobs, IsSimulated isSimulated) {
 	// clang-format off
 	init( ALLOW_DANGEROUS_KNOBS,                               isSimulated );
 
 	// Versions -- knobs that control 5s timeout
 	init( VERSIONS_PER_SECOND,                                   1e6 );
 	init( MAX_READ_TRANSACTION_LIFE_VERSIONS,      5 * VERSIONS_PER_SECOND ); if (isSimulated) MAX_READ_TRANSACTION_LIFE_VERSIONS = randomTxnTimeoutSeconds() * VERSIONS_PER_SECOND;
-	init( MAX_WRITE_TRANSACTION_LIFE_VERSIONS,     5 * VERSIONS_PER_SECOND ); if (randomize && BUGGIFY) MAX_WRITE_TRANSACTION_LIFE_VERSIONS=std::max<int>(1, 1 * VERSIONS_PER_SECOND);
+	init( MAX_WRITE_TRANSACTION_LIFE_VERSIONS,     5 * VERSIONS_PER_SECOND ); if (isSimulated) MAX_WRITE_TRANSACTION_LIFE_VERSIONS = clientKnobs->MAX_WRITE_TRANSACTION_LIFE_VERSIONS;
 
 	// Versions -- other
 	init( MAX_VERSIONS_IN_FLIGHT,                100 * VERSIONS_PER_SECOND );
@@ -56,7 +49,6 @@ void ServerKnobs::initialize(Randomize randomize, ClientKnobs* clientKnobs, IsSi
 	init( ENABLE_VERSION_VECTOR_TLOG_UNICAST,                  false );
 	init( ENABLE_VERSION_VECTOR_HA_OPTIMIZATION,               false );
 	init( ENABLE_VERSION_VECTOR_REPLY_RECOVERY,                false );
-
 	init( MAX_COMMIT_BATCH_INTERVAL,                             2.0 ); if( randomize && BUGGIFY ) MAX_COMMIT_BATCH_INTERVAL = 0.5; // Each commit proxy generates a CommitTransactionBatchRequest at least this often, so that versions always advance smoothly
 	MAX_COMMIT_BATCH_INTERVAL = std::min(MAX_COMMIT_BATCH_INTERVAL, MAX_READ_TRANSACTION_LIFE_VERSIONS/double(2*VERSIONS_PER_SECOND)); // Ensure that the proxy commits 2 times every MAX_READ_TRANSACTION_LIFE_VERSIONS, otherwise the master will not give out versions fast enough
 	MAX_COMMIT_BATCH_INTERVAL = std::min(MAX_COMMIT_BATCH_INTERVAL, MAX_WRITE_TRANSACTION_LIFE_VERSIONS/double(2*VERSIONS_PER_SECOND)); // Ensure that the proxy commits 2 times every MAX_WRITE_TRANSACTION_LIFE_VERSIONS, otherwise the master will not give out versions fast enough

fdbclient/include/fdbclient/ClientKnobs.h

@@ -365,8 +365,8 @@ class SWIFT_CXX_IMMORTAL_SINGLETON_TYPE ClientKnobs : public KnobsImpl<ClientKno
 	// Enable to logging verbose trace events related to the accumulative checksum
 	bool ENABLE_ACCUMULATIVE_CHECKSUM_LOGGING;
 
-	ClientKnobs(Randomize randomize);
-	void initialize(Randomize randomize);
+	ClientKnobs(Randomize randomize, IsSimulated isSimulated);
+	void initialize(Randomize randomize, IsSimulated isSimulated);
 };
 
 #endif

tests/fast/AutomaticIdempotency.toml

@@ -1,3 +1,7 @@
+[[knobs]]
+# versions_per_second = 1M, max_write_transaction_life_versions = 5 * 1M = 5M
+max_write_transaction_life_versions = 5000000
+
 [[test]]
 testTitle = 'AutomaticIdempotency'