Merge remote-tracking branch 'origin/master' into bugfixes/macos-literal-string

.gitignore

@@ -84,6 +84,7 @@ ipch/
 compile_commands.json
 flow/actorcompiler/obj
 flow/coveragetool/obj
+*.code-workspace
 
 # IDE indexing (commonly used tools)
 /compile_commands.json

bindings/python/tests/fdbcli_tests.py

@@ -416,6 +416,6 @@ if __name__ == '__main__':
     else:
         assert process_number > 1, "Process number should be positive"
         coordinators()
-        exclude()
+        # exclude()
 
     

fdbclient/BackupContainerAzureBlobStore.actor.cpp

@@ -172,9 +172,11 @@ public:
 		}
 		Reference<IAsyncFile> f =
 		    makeReference<ReadFile>(self->asyncTaskThread, self->containerName, fileName, self->client.get());
+#if ENCRYPTION_ENABLED
 		if (self->usesEncryption()) {
 			f = makeReference<AsyncFileEncrypted>(f, false);
 		}
+#endif
 		return f;
 	}
 
@@ -185,9 +187,11 @@ public:
 			    return Void();
 		    }));
 		auto f = makeReference<WriteFile>(self->asyncTaskThread, self->containerName, fileName, self->client.get());
+#if ENCRYPTION_ENABLED
 		if (self->usesEncryption()) {
 			f = makeReference<AsyncFileEncrypted>(f, true);
 		}
+#endif
 		return makeReference<BackupFile>(fileName, f);
 	}
 

fdbclient/BackupContainerFileSystem.actor.cpp

@@ -1129,8 +1129,7 @@ public:
 		return false;
 	}
 
-#if (!defined(TLS_DISABLED) && !defined(_WIN32))
-
+#if ENCRYPTION_ENABLED
 	ACTOR static Future<Void> createTestEncryptionKeyFile(std::string filename) {
 		state Reference<IAsyncFile> keyFile = wait(IAsyncFileSystem::filesystem()->open(
 		    filename,
@@ -1167,7 +1166,8 @@ public:
 		StreamCipher::Key::initializeKey(std::move(key));
 		return Void();
 	}
-#endif
+#endif // ENCRYPTION_ENABLED
+
 }; // class BackupContainerFileSystemImpl
 
 Future<Reference<IBackupFile>> BackupContainerFileSystem::writeLogFile(Version beginVersion,
@@ -1484,11 +1484,19 @@ Future<Void> BackupContainerFileSystem::encryptionSetupComplete() const {
 #if (!defined(TLS_DISABLED) && !defined(_WIN32))
 void BackupContainerFileSystem::setEncryptionKey(Optional<std::string> const& encryptionKeyFileName) {
 	if (encryptionKeyFileName.present()) {
+#if ENCRYPTION_ENABLED
 		encryptionSetupFuture = BackupContainerFileSystemImpl::readEncryptionKey(encryptionKeyFileName.get());
+#else
+		encryptionSetupFuture = Void();
+#endif
 	}
 }
 Future<Void> BackupContainerFileSystem::createTestEncryptionKeyFile(std::string const &filename) {
+#if ENCRYPTION_ENABLED
 	return BackupContainerFileSystemImpl::createTestEncryptionKeyFile(filename);
+#else
+	return Void();
+#endif
 }
 #else
 Future<Void> BackupContainerFileSystem::createTestEncryptionKeyFile(std::string const& filename) {

fdbclient/BackupContainerS3BlobStore.actor.cpp

@@ -175,7 +175,8 @@ std::string BackupContainerS3BlobStore::getURLFormat() {
 
 Future<Reference<IAsyncFile>> BackupContainerS3BlobStore::readFile(const std::string& path) {
 	Reference<IAsyncFile> f = makeReference<AsyncFileS3BlobStoreRead>(m_bstore, m_bucket, dataPath(path));
-#if (!defined(TLS_DISABLED) && !defined(_WIN32))
+
+#if ENCRYPTION_ENABLED
 	if (usesEncryption()) {
 		f = makeReference<AsyncFileEncrypted>(f, AsyncFileEncrypted::Mode::READ_ONLY);
 	}
@@ -195,7 +196,7 @@ Future<std::vector<std::string>> BackupContainerS3BlobStore::listURLs(Reference<
 
 Future<Reference<IBackupFile>> BackupContainerS3BlobStore::writeFile(const std::string& path) {
 	Reference<IAsyncFile> f = makeReference<AsyncFileS3BlobStoreWrite>(m_bstore, m_bucket, dataPath(path));
-#if (!defined(TLS_DISABLED) && !defined(_WIN32))
+#if ENCRYPTION_ENABLED
 	if (usesEncryption()) {
 		f = makeReference<AsyncFileEncrypted>(f, AsyncFileEncrypted::Mode::APPEND_ONLY);
 	}

fdbclient/ServerKnobs.cpp

@@ -335,7 +335,9 @@ void ServerKnobs::initialize(Randomize randomize, ClientKnobs* clientKnobs, IsSi
 	// KeyValueStoreRocksDB
 	init( ROCKSDB_BACKGROUND_PARALLELISM,                          0 );
 	init( ROCKSDB_READ_PARALLELISM,                                4 );
-	init( ROCKSDB_MEMTABLE_BYTES,                  512 * 1024 * 1024 );
+	// Use a smaller memtable in simulation to avoid OOMs.
+	int64_t memtableBytes = isSimulated ? 32 * 1024 : 512 * 1024 * 1024;
+	init( ROCKSDB_MEMTABLE_BYTES,                      memtableBytes );
 	init( ROCKSDB_UNSAFE_AUTO_FSYNC,                           false );
 	init( ROCKSDB_PERIODIC_COMPACTION_SECONDS,                     0 );
 	init( ROCKSDB_PREFIX_LEN,                                      0 );

fdbrpc/AsyncFileEncrypted.h

@@ -26,6 +26,8 @@
 #include "flow/IRandom.h"
 #include "flow/StreamCipher.h"
 
+#if ENCRYPTION_ENABLED
+
 #include <array>
 
 /*
@@ -79,3 +81,5 @@ public:
 	void releaseZeroCopy(void* data, int length, int64_t offset) override;
 	int64_t debugFD() const override;
 };
+
+#endif // ENCRYPTION_ENABLED

fdbrpc/CMakeLists.txt

@@ -1,6 +1,7 @@
 set(FDBRPC_SRCS
   AsyncFileCached.actor.h
   AsyncFileEIO.actor.h
+  AsyncFileEncrypted.h
   AsyncFileKAIO.actor.h
   AsyncFileNonDurable.actor.h
   AsyncFileReadAhead.actor.h
@@ -36,7 +37,6 @@ set(FDBRPC_SRCS
 if(WITH_TLS AND NOT WIN32)
   set(FDBRPC_SRCS
     ${FDBRPC_SRCS}
-    AsyncFileEncrypted.h
     AsyncFileEncrypted.actor.cpp)
 endif()
 

fdbrpc/Net2FileSystem.cpp

@@ -32,9 +32,7 @@
 
 #include "fdbrpc/AsyncFileCached.actor.h"
 #include "fdbrpc/AsyncFileEIO.actor.h"
-#if (!defined(TLS_DISABLED) && !defined(_WIN32))
 #include "fdbrpc/AsyncFileEncrypted.h"
-#endif
 #include "fdbrpc/AsyncFileWinASIO.actor.h"
 #include "fdbrpc/AsyncFileKAIO.actor.h"
 #include "flow/AsioReactor.h"
@@ -79,14 +77,14 @@ Future<Reference<class IAsyncFile>> Net2FileSystem::open(const std::string& file
 		    static_cast<boost::asio::io_service*>((void*)g_network->global(INetwork::enASIOService)));
 	if (FLOW_KNOBS->PAGE_WRITE_CHECKSUM_HISTORY > 0)
 		f = map(f, [=](Reference<IAsyncFile> r) { return Reference<IAsyncFile>(new AsyncFileWriteChecker(r)); });
-#if (!defined(TLS_DISABLED) && !defined(_WIN32))
+#if ENCRYPTION_ENABLED
 	if (flags & IAsyncFile::OPEN_ENCRYPTED)
 		f = map(f, [flags](Reference<IAsyncFile> r) {
 			auto mode = flags & IAsyncFile::OPEN_READWRITE ? AsyncFileEncrypted::Mode::APPEND_ONLY
 			                                               : AsyncFileEncrypted::Mode::READ_ONLY;
 			return Reference<IAsyncFile>(new AsyncFileEncrypted(r, mode));
 		});
-#endif
+#endif // ENCRYPTION_ENABLED
 	return f;
 }
 

fdbrpc/sim2.actor.cpp

@@ -33,9 +33,7 @@
 #include "flow/Util.h"
 #include "fdbrpc/IAsyncFile.h"
 #include "fdbrpc/AsyncFileCached.actor.h"
-#if (!defined(TLS_DISABLED) && !defined(_WIN32))
 #include "fdbrpc/AsyncFileEncrypted.h"
-#endif
 #include "fdbrpc/AsyncFileNonDurable.actor.h"
 #include "flow/crc32c.h"
 #include "fdbrpc/TraceFileIO.h"
@@ -2477,14 +2475,14 @@ Future<Reference<class IAsyncFile>> Sim2FileSystem::open(const std::string& file
 		f = AsyncFileDetachable::open(f);
 		if (FLOW_KNOBS->PAGE_WRITE_CHECKSUM_HISTORY > 0)
 			f = map(f, [=](Reference<IAsyncFile> r) { return Reference<IAsyncFile>(new AsyncFileWriteChecker(r)); });
-#if (!defined(TLS_DISABLED) && !defined(_WIN32))
+#if ENCRYPTION_ENABLED
 		if (flags & IAsyncFile::OPEN_ENCRYPTED)
 			f = map(f, [flags](Reference<IAsyncFile> r) {
 				auto mode = flags & IAsyncFile::OPEN_READWRITE ? AsyncFileEncrypted::Mode::APPEND_ONLY
 				                                               : AsyncFileEncrypted::Mode::READ_ONLY;
 				return Reference<IAsyncFile>(new AsyncFileEncrypted(r, mode));
 			});
-#endif
+#endif // ENCRYPTION_ENABLED
 		return f;
 	} else
 		return AsyncFileCached::open(filename, flags, mode);

fdbserver/KeyValueStoreRocksDB.actor.cpp

@@ -7,6 +7,7 @@
 #include <rocksdb/slice_transform.h>
 #include <rocksdb/table.h>
 #include <rocksdb/utilities/table_properties_collectors.h>
+#include "fdbserver/CoroFlow.h"
 #include "flow/flow.h"
 #include "flow/IThreadPool.h"
 
@@ -283,7 +284,9 @@ struct RocksDBKeyValueStore : IKeyValueStore {
 				                              std::min(value.size(), size_t(a.maxLength)))));
 			} else {
 				if (!s.IsNotFound()) {
-					TraceEvent(SevError, "RocksDBError").detail("Error", s.ToString()).detail("Method", "ReadValuePrefix");
+					TraceEvent(SevError, "RocksDBError")
+					    .detail("Error", s.ToString())
+					    .detail("Method", "ReadValuePrefix");
 				}
 				a.result.send(Optional<Value>());
 			}
@@ -367,8 +370,23 @@ struct RocksDBKeyValueStore : IKeyValueStore {
 	std::unique_ptr<rocksdb::WriteBatch> writeBatch;
 
 	explicit RocksDBKeyValueStore(const std::string& path, UID id) : path(path), id(id) {
-		writeThread = createGenericThreadPool();
-		readThreads = createGenericThreadPool();
+		// In simluation, run the reader/writer threads as Coro threads (i.e. in the network thread. The storage engine
+		// is still multi-threaded as background compaction threads are still present. Reads/writes to disk will also
+		// block the network thread in a way that would be unacceptable in production but is a necessary evil here. When
+		// performing the reads in background threads in simulation, the event loop thinks there is no work to do and
+		// advances time faster than 1 sec/sec. By the time the blocking read actually finishes, simulation has advanced
+		// time by more than 5 seconds, so every read fails with a transaction_too_old error. Doing blocking IO on the
+		// main thread solves this issue. There are almost certainly better fixes, but my goal was to get a less
+		// invasive change merged first and work on a more realistic version if/when we think that would provide
+		// substantially more confidence in the correctness.
+		// TODO: Adapt the simulation framework to not advance time quickly when background reads/writes are occurring.
+		if (g_network->isSimulated()) {
+			writeThread = CoroThreadPool::createThreadPool();
+			readThreads = CoroThreadPool::createThreadPool();
+		} else {
+			writeThread = createGenericThreadPool();
+			readThreads = createGenericThreadPool();
+		}
 		writeThread->addThread(new Writer(db, id), "fdb-rocksdb-wr");
 		for (unsigned i = 0; i < SERVER_KNOBS->ROCKSDB_READ_PARALLELISM; ++i) {
 			readThreads->addThread(new Reader(db), "fdb-rocksdb-re");

fdbserver/SimulatedCluster.actor.cpp

@@ -22,6 +22,7 @@
 #include <fstream>
 #include <ostream>
 #include <sstream>
+#include <string_view>
 #include <toml.hpp>
 #include "fdbrpc/Locality.h"
 #include "fdbrpc/simulator.h"
@@ -50,6 +51,19 @@ extern const char* getSourceVersion();
 
 using namespace std::literals;
 
+// TODO: Defining these here is just asking for ODR violations.
+template <>
+std::string describe(bool const& val) {
+	return val ? "true" : "false";
+}
+
+template <>
+std::string describe(int const& val) {
+	return format("%d", val);
+}
+
+namespace {
+
 const int MACHINE_REBOOT_TIME = 10;
 
 bool destructed = false;
@@ -232,6 +246,7 @@ public:
 	//	1 = "memory"
 	//	2 = "memory-radixtree-beta"
 	//	3 = "ssd-redwood-experimental"
+	//	4 = "ssd-rocksdb-experimental"
 	// Requires a comma-separated list of numbers WITHOUT whitespaces
 	std::vector<int> storageEngineExcludeTypes;
 	// Set the maximum TLog version that can be selected for a test
@@ -629,16 +644,6 @@ ACTOR Future<ISimulator::KillType> simulatedFDBDRebooter(Reference<ClusterConnec
 	}
 }
 
-template <>
-std::string describe(bool const& val) {
-	return val ? "true" : "false";
-}
-
-template <>
-std::string describe(int const& val) {
-	return format("%d", val);
-}
-
 // Since a datacenter kill is considered to be the same as killing a machine, files cannot be swapped across datacenters
 std::map<Optional<Standalone<StringRef>>, std::vector<std::vector<std::string>>> availableFolders;
 // process count is no longer needed because it is now the length of the vector of ip's, because it was one ip per
@@ -1252,7 +1257,7 @@ void SimulationConfig::setDatacenters(const TestConfig& testConfig) {
 
 // Sets storage engine based on testConfig details
 void SimulationConfig::setStorageEngine(const TestConfig& testConfig) {
-	int storage_engine_type = deterministicRandom()->randomInt(0, 4);
+	int storage_engine_type = deterministicRandom()->randomInt(0, 5);
 	if (testConfig.storageEngineType.present()) {
 		storage_engine_type = testConfig.storageEngineType.get();
 	} else {
@@ -1260,7 +1265,7 @@ void SimulationConfig::setStorageEngine(const TestConfig& testConfig) {
 		while (std::find(testConfig.storageEngineExcludeTypes.begin(),
 		                 testConfig.storageEngineExcludeTypes.end(),
 		                 storage_engine_type) != testConfig.storageEngineExcludeTypes.end()) {
-			storage_engine_type = deterministicRandom()->randomInt(0, 4);
+			storage_engine_type = deterministicRandom()->randomInt(0, 5);
 		}
 	}
 
@@ -1285,6 +1290,16 @@ void SimulationConfig::setStorageEngine(const TestConfig& testConfig) {
 		set_config("ssd-redwood-experimental");
 		break;
 	}
+	case 4: {
+		TEST(true); // Simulated cluster using RocksDB storage engine
+		set_config("ssd-rocksdb-experimental");
+		// Tests using the RocksDB engine are necessarily non-deterministic because of RocksDB
+		// background threads.
+		TraceEvent(SevWarn, "RocksDBNonDeterminism")
+		    .detail("Explanation", "The RocksDB storage engine is threaded and non-deterministic");
+		noUnseed = true;
+		break;
+	}
 	default:
 		ASSERT(false); // Programmer forgot to adjust cases.
 	}
@@ -2081,9 +2096,17 @@ void setupSimulatedSystem(vector<Future<Void>>* systemActors,
 
 using namespace std::literals;
 
+#ifdef SSD_ROCKSDB_EXPERIMENTAL
+bool rocksDBEnabled = true;
+#else
+bool rocksDBEnabled = false;
+#endif
+
 // Populates the TestConfig fields according to what is found in the test file.
 void checkTestConf(const char* testFile, TestConfig* testConfig) {}
 
+} // namespace
+
 ACTOR void setupAndRun(std::string dataFolder,
                        const char* testFile,
                        bool rebooting,
@@ -2098,6 +2121,19 @@ ACTOR void setupAndRun(std::string dataFolder,
 	g_simulator.hasDiffProtocolProcess = testConfig.startIncompatibleProcess;
 	g_simulator.setDiffProtocol = false;
 
+	// The RocksDB storage engine does not support the restarting tests because you cannot consistently get a clean
+	// snapshot of the storage engine without a snapshotting file system.
+	// https://github.com/apple/foundationdb/issues/5155
+	if (std::string_view(testFile).find("restarting") != std::string_view::npos) {
+		testConfig.storageEngineExcludeTypes.push_back(4);
+	}
+
+	// The RocksDB engine is not always built with the rest of fdbserver. Don't try to use it if it is not included
+	// in the build.
+	if (!rocksDBEnabled) {
+		testConfig.storageEngineExcludeTypes.push_back(4);
+	}
+
 	state ProtocolVersion protocolVersion = currentProtocolVersion;
 	if (testConfig.startIncompatibleProcess) {
 		// isolates right most 1 bit of compatibleProtocolVersionMask to make this protocolVersion incompatible

flow/CMakeLists.txt

@@ -53,6 +53,7 @@ set(FLOW_SRCS
   SignalSafeUnwind.cpp
   SignalSafeUnwind.h
   SimpleOpt.h
+  StreamCipher.h
   SystemMonitor.cpp
   SystemMonitor.h
   TDMetric.actor.h
@@ -100,8 +101,7 @@ set(FLOW_SRCS
 if(WITH_TLS AND NOT WIN32)
   set(FLOW_SRCS
     ${FLOW_SRCS}
-    StreamCipher.cpp
-    StreamCipher.h)
+    StreamCipher.cpp)
 endif()
 
 add_library(stacktrace stacktrace.amalgamation.cpp stacktrace.h)

flow/StreamCipher.h

@@ -20,6 +20,14 @@
 
 #pragma once
 
+#if (!defined(TLS_DISABLED) && !defined(_WIN32))
+#define ENCRYPTION_ENABLED 1
+#else
+#define ENCRYPTION_ENABLED 0
+#endif
+
+#if ENCRYPTION_ENABLED
+
 #include "flow/Arena.h"
 #include "flow/FastRef.h"
 #include "flow/flow.h"
@@ -78,3 +86,5 @@ public:
 	StringRef decrypt(unsigned char const* ciphertext, int len, Arena&);
 	StringRef finish(Arena&);
 };
+
+#endif // ENCRYPTION_ENABLED