From 61a727d70151ff02a1440dcd27695fe9b8a287ad Mon Sep 17 00:00:00 2001 From: Andrew Noyes Date: Mon, 3 Feb 2020 13:55:40 -0800 Subject: [PATCH 1/7] Allow new databases to be configured as locked --- fdbcli/fdbcli.actor.cpp | 4 ++++ fdbclient/ManagementAPI.actor.cpp | 24 +++++++++++++++++++++++- fdbclient/ManagementAPI.actor.h | 3 ++- 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/fdbcli/fdbcli.actor.cpp b/fdbcli/fdbcli.actor.cpp index 6ae53ca79d..192a27db4c 100644 --- a/fdbcli/fdbcli.actor.cpp +++ b/fdbcli/fdbcli.actor.cpp @@ -1774,6 +1774,10 @@ ACTOR Future configure( Database db, std::vector tokens, Refere printf("Configuration changed\n"); ret=false; break; + case ConfigurationResult::LOCKED_NOT_NEW: + printf("ERROR: `only new databases can be configured as locked`\n"); + ret = true; + break; default: ASSERT(false); ret=true; diff --git a/fdbclient/ManagementAPI.actor.cpp b/fdbclient/ManagementAPI.actor.cpp index 14cd25abe5..4cdf1efc73 100644 --- a/fdbclient/ManagementAPI.actor.cpp +++ b/fdbclient/ManagementAPI.actor.cpp @@ -20,6 +20,7 @@ #include +#include "IRandom.h" #include "fdbclient/ManagementAPI.actor.h" #include "fdbclient/SystemData.h" @@ -52,6 +53,11 @@ std::map configForToken( std::string const& mode ) { return out; } + if (mode == "locked") { + out[databaseLockedKey.toString()] = deterministicRandom()->randomUniqueID().toString(); + return out; + } + size_t pos; // key:=value is unvalidated and unchecked @@ -297,6 +303,17 @@ ACTOR Future changeConfig( Database cx, std::map locked; + { + auto iter = m.find(databaseLockedKey.toString()); + if (iter != m.end()) { + if (!creating) { + return ConfigurationResult::LOCKED_NOT_NEW; + } + locked = UID::fromString(iter->second); + m.erase(iter); + } + } if (creating) { m[initIdKey.toString()] = deterministicRandom()->randomUniqueID().toString(); if (!isCompleteConfiguration(m)) { @@ -484,7 +501,12 @@ ACTOR Future changeConfig( Database cx, std::mapfirst) ); } - for(auto i=m.begin(); i!=m.end(); ++i) + if (locked.present()) { + ASSERT(creating); + wait(lockDatabase(&tr, locked.get())); + } + + for (auto i = m.begin(); i != m.end(); ++i) tr.set( StringRef(i->first), StringRef(i->second) ); tr.addReadConflictRange( singleKeyRange(moveKeysLockOwnerKey) ); diff --git a/fdbclient/ManagementAPI.actor.h b/fdbclient/ManagementAPI.actor.h index 0fe942e019..265b8b180e 100644 --- a/fdbclient/ManagementAPI.actor.h +++ b/fdbclient/ManagementAPI.actor.h @@ -61,7 +61,8 @@ public: NOT_ENOUGH_WORKERS, REGION_REPLICATION_MISMATCH, DCID_MISSING, - SUCCESS + SUCCESS, + LOCKED_NOT_NEW, }; }; From a043646d1d0ff0069f7962e774a5262c730c7706 Mon Sep 17 00:00:00 2001 From: Andrew Noyes Date: Mon, 3 Feb 2020 14:35:35 -0800 Subject: [PATCH 2/7] Don't read in init database transaction --- fdbclient/ManagementAPI.actor.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/fdbclient/ManagementAPI.actor.cpp b/fdbclient/ManagementAPI.actor.cpp index 4cdf1efc73..0f3d6a4c8a 100644 --- a/fdbclient/ManagementAPI.actor.cpp +++ b/fdbclient/ManagementAPI.actor.cpp @@ -503,7 +503,11 @@ ACTOR Future changeConfig( Database cx, std::map Date: Wed, 5 Feb 2020 09:57:18 -0800 Subject: [PATCH 3/7] Don't include header --- fdbclient/ManagementAPI.actor.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/fdbclient/ManagementAPI.actor.cpp b/fdbclient/ManagementAPI.actor.cpp index 0f3d6a4c8a..c0689ff84c 100644 --- a/fdbclient/ManagementAPI.actor.cpp +++ b/fdbclient/ManagementAPI.actor.cpp @@ -20,7 +20,6 @@ #include -#include "IRandom.h" #include "fdbclient/ManagementAPI.actor.h" #include "fdbclient/SystemData.h" From 7b5de42d43afa0160d2f2c8132112a04d65dd57e Mon Sep 17 00:00:00 2001 From: Andrew Noyes Date: Tue, 11 Feb 2020 10:40:09 -0800 Subject: [PATCH 4/7] Address review comments --- fdbclient/ManagementAPI.actor.cpp | 2 ++ fdbclient/ManagementAPI.actor.h | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/fdbclient/ManagementAPI.actor.cpp b/fdbclient/ManagementAPI.actor.cpp index c0689ff84c..dafad6a15e 100644 --- a/fdbclient/ManagementAPI.actor.cpp +++ b/fdbclient/ManagementAPI.actor.cpp @@ -53,6 +53,8 @@ std::map configForToken( std::string const& mode ) { } if (mode == "locked") { + // Setting this key is interpreted as an instruction to use the normal version-stamp-based mechanism for locking + // the database. out[databaseLockedKey.toString()] = deterministicRandom()->randomUniqueID().toString(); return out; } diff --git a/fdbclient/ManagementAPI.actor.h b/fdbclient/ManagementAPI.actor.h index 265b8b180e..e85b005bf4 100644 --- a/fdbclient/ManagementAPI.actor.h +++ b/fdbclient/ManagementAPI.actor.h @@ -61,8 +61,8 @@ public: NOT_ENOUGH_WORKERS, REGION_REPLICATION_MISMATCH, DCID_MISSING, - SUCCESS, LOCKED_NOT_NEW, + SUCCESS, }; }; From 1e1e75123fbb208d6441ffc2391ed23cd1460688 Mon Sep 17 00:00:00 2001 From: Andrew Noyes Date: Tue, 11 Feb 2020 11:10:09 -0800 Subject: [PATCH 5/7] Add simulation testing --- fdbserver/SimulatedCluster.actor.cpp | 17 +++++++++++++---- fdbserver/workloads/LockDatabase.actor.cpp | 7 ++++--- tests/CMakeLists.txt | 1 + tests/fast/ConfigureLocked.txt | 4 ++++ 4 files changed, 22 insertions(+), 7 deletions(-) create mode 100644 tests/fast/ConfigureLocked.txt diff --git a/fdbserver/SimulatedCluster.actor.cpp b/fdbserver/SimulatedCluster.actor.cpp index 65ede31bfd..64311b703a 100644 --- a/fdbserver/SimulatedCluster.actor.cpp +++ b/fdbserver/SimulatedCluster.actor.cpp @@ -1088,11 +1088,14 @@ void SimulationConfig::generateNormalConfig(int minimumReplication, int minimumR void setupSimulatedSystem(vector>* systemActors, std::string baseFolder, int* pTesterCount, Optional* pConnString, Standalone* pStartingConfiguration, int extraDB, int minimumReplication, int minimumRegions, Reference tlsOptions, - std::string whitelistBinPaths) { + std::string whitelistBinPaths, bool configureLocked) { // SOMEDAY: this does not test multi-interface configurations SimulationConfig simconfig(extraDB, minimumReplication, minimumRegions); StatusObject startingConfigJSON = simconfig.db.toJSON(true); std::string startingConfigString = "new"; + if (configureLocked) { + startingConfigString += " locked"; + } for( auto kv : startingConfigJSON) { startingConfigString += " "; if( kv.second.type() == json_spirit::int_type ) { @@ -1351,7 +1354,8 @@ void setupSimulatedSystem(vector>* systemActors, std::string baseFo .detail("StartingConfiguration", pStartingConfiguration->toString()); } -void checkExtraDB(const char *testFile, int &extraDB, int &minimumReplication, int &minimumRegions) { +void checkTestConf(const char* testFile, int& extraDB, int& minimumReplication, int& minimumRegions, + int& configureLocked) { std::ifstream ifs; ifs.open(testFile, std::ifstream::in); if (!ifs.good()) @@ -1383,6 +1387,10 @@ void checkExtraDB(const char *testFile, int &extraDB, int &minimumReplication, i if (attrib == "minimumRegions") { sscanf( value.c_str(), "%d", &minimumRegions ); } + + if (attrib == "configureLocked") { + sscanf(value.c_str(), "%d", &configureLocked); + } } ifs.close(); @@ -1396,7 +1404,8 @@ ACTOR void setupAndRun(std::string dataFolder, const char *testFile, bool reboot state int extraDB = 0; state int minimumReplication = 0; state int minimumRegions = 0; - checkExtraDB(testFile, extraDB, minimumReplication, minimumRegions); + state int configureLocked = 0; + checkTestConf(testFile, extraDB, minimumReplication, minimumRegions, configureLocked); // TODO (IPv6) Use IPv6? wait(g_simulator.onProcess( @@ -1427,7 +1436,7 @@ ACTOR void setupAndRun(std::string dataFolder, const char *testFile, bool reboot else { g_expect_full_pointermap = 1; setupSimulatedSystem(&systemActors, dataFolder, &testerCount, &connFile, &startingConfiguration, extraDB, - minimumReplication, minimumRegions, tlsOptions, whitelistBinPaths); + minimumReplication, minimumRegions, tlsOptions, whitelistBinPaths, configureLocked); wait( delay(1.0) ); // FIXME: WHY!!! //wait for machines to boot } std::string clusterFileDir = joinPath( dataFolder, deterministicRandom()->randomUniqueID().toString() ); diff --git a/fdbserver/workloads/LockDatabase.actor.cpp b/fdbserver/workloads/LockDatabase.actor.cpp index 097e422873..d8c1894a65 100644 --- a/fdbserver/workloads/LockDatabase.actor.cpp +++ b/fdbserver/workloads/LockDatabase.actor.cpp @@ -27,12 +27,14 @@ struct LockDatabaseWorkload : TestWorkload { double lockAfter, unlockAfter; bool ok; + bool onlyCheckLocked; LockDatabaseWorkload(WorkloadContext const& wcx) : TestWorkload(wcx), ok(true) { lockAfter = getOption( options, LiteralStringRef("lockAfter"), 0.0 ); unlockAfter = getOption( options, LiteralStringRef("unlockAfter"), 10.0 ); + onlyCheckLocked = getOption(options, LiteralStringRef("onlyCheckLocked"), false); ASSERT(unlockAfter > lockAfter); } @@ -42,9 +44,8 @@ struct LockDatabaseWorkload : TestWorkload { return Void(); } - virtual Future start( Database const& cx ) { - if( clientId == 0 ) - return lockWorker( cx, this ); + virtual Future start(Database const& cx) { + if (clientId == 0) return onlyCheckLocked ? checkLocked(cx, this) : lockWorker(cx, this); return Void(); } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index e5b40e7238..b7284529a9 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -100,6 +100,7 @@ add_fdb_test(TEST_FILES fast/BackupCorrectnessClean.txt) add_fdb_test(TEST_FILES fast/BackupToDBCorrectness.txt) add_fdb_test(TEST_FILES fast/BackupToDBCorrectnessClean.txt) add_fdb_test(TEST_FILES fast/CloggedSideband.txt) +add_fdb_test(TEST_FILES fast/ConfigureLocked.txt) add_fdb_test(TEST_FILES fast/ConstrainedRandomSelector.txt) add_fdb_test(TEST_FILES fast/CycleAndLock.txt) add_fdb_test(TEST_FILES fast/CycleTest.txt) diff --git a/tests/fast/ConfigureLocked.txt b/tests/fast/ConfigureLocked.txt new file mode 100644 index 0000000000..c4324e65d4 --- /dev/null +++ b/tests/fast/ConfigureLocked.txt @@ -0,0 +1,4 @@ +testTitle=ConfigureLocked +testName=LockDatabase +configureLocked=1 +onlyCheckLocked=1 From 17660fb18dea02199918b8954118803f37caea6c Mon Sep 17 00:00:00 2001 From: Andrew Noyes Date: Tue, 11 Feb 2020 13:49:19 -0800 Subject: [PATCH 6/7] Fix simulation test --- fdbserver/tester.actor.cpp | 2 ++ fdbserver/workloads/LockDatabase.actor.cpp | 3 ++- tests/fast/ConfigureLocked.txt | 4 +++- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/fdbserver/tester.actor.cpp b/fdbserver/tester.actor.cpp index 9c7639aa46..235c9ef090 100644 --- a/fdbserver/tester.actor.cpp +++ b/fdbserver/tester.actor.cpp @@ -970,6 +970,8 @@ vector readTests( ifstream& ifs ) { TraceEvent("TestParserTest").detail("ParsedSimDrAgents", spec.simDrAgents); } else if( attrib == "extraDB" ) { TraceEvent("TestParserTest").detail("ParsedExtraDB", ""); + } else if ( attrib == "configureLocked" ) { + TraceEvent("TestParserTest").detail("ParsedConfigureLocked", ""); } else if( attrib == "minimumReplication" ) { TraceEvent("TestParserTest").detail("ParsedMinimumReplication", ""); } else if( attrib == "minimumRegions" ) { diff --git a/fdbserver/workloads/LockDatabase.actor.cpp b/fdbserver/workloads/LockDatabase.actor.cpp index d8c1894a65..9c9d51a6e9 100644 --- a/fdbserver/workloads/LockDatabase.actor.cpp +++ b/fdbserver/workloads/LockDatabase.actor.cpp @@ -45,7 +45,7 @@ struct LockDatabaseWorkload : TestWorkload { } virtual Future start(Database const& cx) { - if (clientId == 0) return onlyCheckLocked ? checkLocked(cx, this) : lockWorker(cx, this); + if (clientId == 0) return onlyCheckLocked ? timeout(checkLocked(cx, this), 60, Void()) : lockWorker(cx, this); return Void(); } @@ -111,6 +111,7 @@ struct LockDatabaseWorkload : TestWorkload { self->ok = false; return Void(); } catch( Error &e ) { + TEST(e.code() == error_code_database_locked); // Database confirmed locked wait( tr.onError(e) ); } } diff --git a/tests/fast/ConfigureLocked.txt b/tests/fast/ConfigureLocked.txt index c4324e65d4..90be170716 100644 --- a/tests/fast/ConfigureLocked.txt +++ b/tests/fast/ConfigureLocked.txt @@ -1,4 +1,6 @@ testTitle=ConfigureLocked testName=LockDatabase configureLocked=1 -onlyCheckLocked=1 +onlyCheckLocked=true +runConsistencyCheck=false +clearAfterTest=false From 8fb9722277d36cf512fddb6380eaede6b42d3cda Mon Sep 17 00:00:00 2001 From: Andrew Noyes Date: Tue, 11 Feb 2020 13:52:56 -0800 Subject: [PATCH 7/7] Fix newline at end of file --- tests/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index faf73630ed..4890a96f95 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -221,4 +221,4 @@ if(WITH_PYTHON) verify_testing() else() message(WARNING "Python not found, won't configure ctest") -endif() \ No newline at end of file +endif()