Update fdb_database_create_snapshot to take a UID as input

bindings/c/fdb_c.cpp

@@ -398,9 +398,15 @@ extern "C" DLLEXPORT FDBFuture* fdb_database_force_recovery_with_data_loss(FDBDa
 	return (FDBFuture*)(DB(db)->forceRecoveryWithDataLoss(StringRef(dcid, dcid_length)).extractPtr());
 }
 
-extern "C" DLLEXPORT FDBFuture* fdb_database_create_snapshot(FDBDatabase* db, uint8_t const* snap_command,
+extern "C" DLLEXPORT FDBFuture *
-                                                             int snap_command_length) {
+fdb_database_create_snapshot(FDBDatabase *db, uint8_t const *uid,
-	return (FDBFuture*)(DB(db)->createSnapshot(StringRef(snap_command, snap_command_length)).extractPtr());
+                             int uid_length, uint8_t const *snap_command,
+                             int snap_command_length) {
+  return (FDBFuture *)(DB(db)
+                           ->createSnapshot(
+                               StringRef(uid, uid_length),
+                               StringRef(snap_command, snap_command_length))
+                           .extractPtr());
 }
 
 extern "C" DLLEXPORT

bindings/c/foundationdb/fdb_c.h

@@ -185,8 +185,10 @@ extern "C" {
     DLLEXPORT WARN_UNUSED_RESULT FDBFuture*
     fdb_database_force_recovery_with_data_loss( FDBDatabase* db, uint8_t const* dcid, int dcid_length);
 
-    DLLEXPORT WARN_UNUSED_RESULT FDBFuture*
+    DLLEXPORT WARN_UNUSED_RESULT FDBFuture *
-    fdb_database_create_snapshot( FDBDatabase* db, uint8_t const* snap_command, int snap_command_length);
+    fdb_database_create_snapshot(FDBDatabase *db, uint8_t const *uid,
+                                 int uid_length, uint8_t const *snap_command,
+                                 int snap_command_length);
 
     DLLEXPORT void fdb_transaction_destroy( FDBTransaction* tr);
 

bindings/c/test/unit/fdb_api.cpp

@@ -102,8 +102,12 @@ EmptyFuture Database::force_recovery_with_data_loss(FDBDatabase *db, const uint8
   return EmptyFuture(fdb_database_force_recovery_with_data_loss(db, dcid, dcid_length));
 }
 
-EmptyFuture Database::create_snapshot(FDBDatabase* db, const uint8_t* snap_command, int snap_command_length) {
+EmptyFuture Database::create_snapshot(FDBDatabase *db, const uint8_t *uid,
-	return EmptyFuture(fdb_database_create_snapshot(db, snap_command, snap_command_length));
+                                      int uid_length,
+                                      const uint8_t *snap_command,
+                                      int snap_command_length) {
+  return EmptyFuture(fdb_database_create_snapshot(
+      db, uid, uid_length, snap_command, snap_command_length));
 }
 
 // Transaction

bindings/c/test/unit/fdb_api.hpp

@@ -154,7 +154,10 @@ public:
 	static Int64Future reboot_worker(FDBDatabase* db, const uint8_t* address, int address_length, fdb_bool_t check,
 	                                int duration);
   static EmptyFuture force_recovery_with_data_loss(FDBDatabase* db, const uint8_t* dcid, int dcid_length);
-  static EmptyFuture create_snapshot(FDBDatabase* db, const uint8_t* snap_command, int snap_command_length);
+  static EmptyFuture create_snapshot(FDBDatabase *db, const uint8_t *uid,
+                                     int uid_length,
+                                     const uint8_t *snap_command,
+                                     int snap_command_length);
 };
 
 // Wrapper around FDBTransaction, providing the same set of calls as the C API.

bindings/c/test/unit/unit_tests.cpp

@@ -2048,17 +2048,20 @@ TEST_CASE("fdb_database_force_recovery_with_data_loss") {
 
 TEST_CASE("fdb_database_create_snapshot") {
 	std::string snapshot_command = "test";
-	while (1) {
+        std::string uid = "invalid_uid";
-		fdb::EmptyFuture f =
+        while (1) {
-		    fdb::Database::create_snapshot(db, (const uint8_t*)snapshot_command.c_str(), snapshot_command.length());
+          fdb::EmptyFuture f = fdb::Database::create_snapshot(
-		fdb_error_t err = wait_future(f);
+              db, (const uint8_t *)uid.c_str(), uid.length(),
-		if (err == 2505) { // expected error code
+              (const uint8_t *)snapshot_command.c_str(),
-			break;
+              snapshot_command.length());
-		} else {
+          fdb_error_t err = wait_future(f);
-			// Otherwise, something went wrong.
+          if (err == 2509) { // expected error code
-			CHECK(false);
+            break;
-		}
+          } else {
-	}
+            // Otherwise, something went wrong.
+            CHECK(false);
+          }
+        }
 }
 
 TEST_CASE("fdb_error_predicate") {

bindings/flow/fdb_flow.actor.cpp

@@ -105,9 +105,11 @@ namespace FDB {
 		void setDatabaseOption(FDBDatabaseOption option, Optional<StringRef> value = Optional<StringRef>()) override;
 		Future<int64_t> rebootWorker(const StringRef& address, bool check = false, int duration = 0) override;
 		Future<Void> forceRecoveryWithDataLoss(const StringRef& dcid) override;
-	    Future<Void> createSnapshot(const StringRef& snap_command) override;
+                Future<Void>
+                createSnapshot(const StringRef &uid,
+                               const StringRef &snap_command) override;
 
-	private:
+              private:
 		FDBDatabase* db;
 		explicit DatabaseImpl(FDBDatabase* db) : db(db) {}
 
@@ -306,13 +308,18 @@ namespace FDB {
 		});
 	}
 
-    Future<Void> DatabaseImpl::createSnapshot(const StringRef& snap_command) {
+        Future<Void>
-	    return backToFuture<Void>(fdb_database_create_snapshot(db, snap_command.begin(), snap_command.size()),
+        DatabaseImpl::createSnapshot(const StringRef &uid,
-	                              [](Reference<CFuture> f) {
+                                     const StringRef &snap_command) {
-		                              throw_on_error(fdb_future_get_error(f->f));
+          return backToFuture<Void>(
-		                              return Void();
+              fdb_database_create_snapshot(db, uid.begin(), uid.size(),
-	                              });
+                                           snap_command.begin(),
-    }
+                                           snap_command.size()),
+              [](Reference<CFuture> f) {
+                throw_on_error(fdb_future_get_error(f->f));
+                return Void();
+              });
+        }
 
     TransactionImpl::TransactionImpl(FDBDatabase* db) {
 		throw_on_error(fdb_database_create_transaction(db, &tr));

bindings/flow/fdb_flow.h

@@ -127,7 +127,9 @@ namespace FDB {
 		virtual void setDatabaseOption(FDBDatabaseOption option, Optional<StringRef> value = Optional<StringRef>()) = 0;
 		virtual Future<int64_t> rebootWorker(const StringRef& address, bool check = false, int duration = 0) = 0;
 		virtual Future<Void> forceRecoveryWithDataLoss(const StringRef& dcid) = 0;
-	    virtual Future<Void> createSnapshot(const StringRef& snap_command) = 0;
+                virtual Future<Void>
+                createSnapshot(const StringRef &uid,
+                               const StringRef &snap_command) = 0;
     };
 
 	class API {

fdbclient/DatabaseContext.h

@@ -212,9 +212,9 @@ public:
 	// Management API, force the database to recover into DCID, causing the database to lose the most recently committed mutations
 	Future<Void> forceRecoveryWithDataLoss(StringRef dcId);
 	// Management API, create snapshot
-	Future<Void> createSnapshot(StringRef snapshot_command);
+        Future<Void> createSnapshot(StringRef uid, StringRef snapshot_command);
 
-//private: 
+        // private: 
 	explicit DatabaseContext( Reference<AsyncVar<Reference<ClusterConnectionFile>>> connectionFile, Reference<AsyncVar<ClientDBInfo>> clientDBInfo,
 		Future<Void> clientInfoMonitor, TaskPriority taskID, LocalityData const& clientLocality, 
 		bool enableLocalityLoadBalance, bool lockAware, bool internal = true, int apiVersion = Database::API_VERSION_LATEST, bool switchable = false );

fdbclient/IClientApi.h

@@ -91,7 +91,9 @@ public:
 	// Management API, force the database to recover into DCID, causing the database to lose the most recently committed mutations
 	virtual ThreadFuture<Void> forceRecoveryWithDataLoss(const StringRef& dcid) = 0;
 	// Management API, create snapshot
-	virtual ThreadFuture<Void> createSnapshot(const StringRef& snapshot_command) = 0;
+        virtual ThreadFuture<Void>
+        createSnapshot(const StringRef &uid,
+                       const StringRef &snapshot_command) = 0;
 };
 
 class IClientApi {

fdbclient/MultiVersionTransaction.actor.cpp

@@ -311,13 +311,18 @@ ThreadFuture<Void> DLDatabase::forceRecoveryWithDataLoss(const StringRef &dcid)
 	});
 }
 
-ThreadFuture<Void> DLDatabase::createSnapshot(const StringRef& snapshot_command) {
+ThreadFuture<Void>
-	if (!api->databaseCreateSnapshot) {
+DLDatabase::createSnapshot(const StringRef &uid,
-		return unsupported_operation();
+                           const StringRef &snapshot_command) {
-	}
+  if (!api->databaseCreateSnapshot) {
+    return unsupported_operation();
+  }
 
-	FdbCApi::FDBFuture* f = api->databaseCreateSnapshot(db, snapshot_command.begin(), snapshot_command.size());
+  FdbCApi::FDBFuture *f = api->databaseCreateSnapshot(
-	return toThreadFuture<Void>(api, f, [](FdbCApi::FDBFuture* f, FdbCApi* api) { return Void(); });
+      db, uid.begin(), uid.size(), snapshot_command.begin(),
+      snapshot_command.size());
+  return toThreadFuture<Void>(
+      api, f, [](FdbCApi::FDBFuture *f, FdbCApi *api) { return Void(); });
 }
 
 // DLApi
@@ -832,9 +837,12 @@ ThreadFuture<Void> MultiVersionDatabase::forceRecoveryWithDataLoss(const StringR
 	return abortableFuture(f, dbState->dbVar->get().onChange);
 }
 
-ThreadFuture<Void> MultiVersionDatabase::createSnapshot(const StringRef& snapshot_command) {
+ThreadFuture<Void>
-	auto f = dbState->db ? dbState->db->createSnapshot(snapshot_command) : ThreadFuture<Void>(Never());
+MultiVersionDatabase::createSnapshot(const StringRef &uid,
-	return abortableFuture(f, dbState->dbVar->get().onChange);
+                                     const StringRef &snapshot_command) {
+  auto f = dbState->db ? dbState->db->createSnapshot(uid, snapshot_command)
+                       : ThreadFuture<Void>(Never());
+  return abortableFuture(f, dbState->dbVar->get().onChange);
 }
 
 void MultiVersionDatabase::Connector::connect() {

fdbclient/MultiVersionTransaction.h

@@ -68,10 +68,12 @@ struct FdbCApi : public ThreadSafeReferenceCounted<FdbCApi> {
 	void (*databaseDestroy)(FDBDatabase *database);
 	FDBFuture* (*databaseRebootWorker)(FDBDatabase *database, uint8_t const *address, int addressLength, fdb_bool_t check, int duration);
 	FDBFuture* (*databaseForceRecoveryWithDataLoss)(FDBDatabase *database, uint8_t const *dcid, int dcidLength);
-	FDBFuture* (*databaseCreateSnapshot)(FDBDatabase* database, uint8_t const* snapshotCommmand,
+        FDBFuture *(*databaseCreateSnapshot)(FDBDatabase *database,
-	                                     int snapshotCommandLength);
+                                             uint8_t const *uid, int uidLength,
+                                             uint8_t const *snapshotCommmand,
+                                             int snapshotCommandLength);
 
-	//Transaction
+        //Transaction
 	fdb_error_t (*transactionSetOption)(FDBTransaction *tr, FDBTransactionOptions::Option option, uint8_t const *value, int valueLength);
 	void (*transactionDestroy)(FDBTransaction *tr);
 
@@ -201,9 +203,11 @@ public:
 
 	ThreadFuture<int64_t> rebootWorker(const StringRef& address, bool check, int duration) override;
 	ThreadFuture<Void> forceRecoveryWithDataLoss(const StringRef& dcid) override;
-	ThreadFuture<Void> createSnapshot(const StringRef& snapshot_command) override;
+        ThreadFuture<Void>
+        createSnapshot(const StringRef &uid,
+                       const StringRef &snapshot_command) override;
 
-private:
+      private:
 	const Reference<FdbCApi> api;
 	FdbCApi::FDBDatabase* db; // Always set if API version >= 610, otherwise guaranteed to be set when onReady future is set
 	ThreadFuture<Void> ready;
@@ -341,9 +345,11 @@ public:
 
 	ThreadFuture<int64_t> rebootWorker(const StringRef& address, bool check, int duration) override;
 	ThreadFuture<Void> forceRecoveryWithDataLoss(const StringRef& dcid) override;
-	ThreadFuture<Void> createSnapshot(const StringRef& snapshot_command) override;
+        ThreadFuture<Void>
+        createSnapshot(const StringRef &uid,
+                       const StringRef &snapshot_command) override;
 
-private:
+      private:
 
 	struct Connector : ThreadCallback, ThreadSafeReferenceCounted<Connector> {
 		Connector(Reference<DatabaseState> dbState, Reference<ClientInfo> client, std::string clusterFilePath) : dbState(dbState), client(client), clusterFilePath(clusterFilePath), connected(false), cancelled(false) {}

fdbclient/NativeAPI.actor.cpp

@@ -4836,16 +4836,19 @@ Future<Void> DatabaseContext::forceRecoveryWithDataLoss(StringRef dcId) {
 	return forceRecovery(getConnectionFile(), dcId);
 }
 
-ACTOR static Future<Void> createSnapshotActor(DatabaseContext* cx, StringRef snapCmd) {
+ACTOR static Future<Void> createSnapshotActor(DatabaseContext *cx, UID snapUID,
-	UID snapUID = deterministicRandom()->randomUniqueID();
+                                              StringRef snapCmd) {
-	try {
+  wait(mgmtSnapCreate(cx->clone(), snapCmd, snapUID));
-		wait(mgmtSnapCreate(cx->clone(), snapCmd, snapUID));
+  return Void();
-	} catch (Error& e) {
-		throw e;
-	}
-	return Void();
 }
 
-Future<Void> DatabaseContext::createSnapshot(StringRef snapshot_command) {
+Future<Void> DatabaseContext::createSnapshot(StringRef uid,
-	return createSnapshotActor(this, snapshot_command);
+                                             StringRef snapshot_command) {
+  std::string uid_str = uid.toString();
+  if (!std::all_of(uid_str.begin(), uid_str.end(),
+                   [](unsigned char c) { return std::isxdigit(c); }) ||
+      uid_str.size() != 32) {
+    throw snap_invalid_uid_string();
+  }
+  return createSnapshotActor(this, UID::fromString(uid_str), snapshot_command);
 }

fdbclient/ThreadSafeTransaction.cpp

@@ -84,10 +84,15 @@ ThreadFuture<Void> ThreadSafeDatabase::forceRecoveryWithDataLoss(const StringRef
 	} );
 }
 
-ThreadFuture<Void> ThreadSafeDatabase::createSnapshot(const StringRef& snapshot_command) {
+ThreadFuture<Void>
-	DatabaseContext* db = this->db;
+ThreadSafeDatabase::createSnapshot(const StringRef &uid,
-	Key cmd = snapshot_command;
+                                   const StringRef &snapshot_command) {
-	return onMainThread([db, cmd]() -> Future<Void> { return db->createSnapshot(cmd); });
+  DatabaseContext *db = this->db;
+  Key snapUID = uid;
+  Key cmd = snapshot_command;
+  return onMainThread([db, snapUID, cmd]() -> Future<Void> {
+    return db->createSnapshot(snapUID, cmd);
+  });
 }
 
 ThreadSafeDatabase::ThreadSafeDatabase(std::string connFilename, int apiVersion) {

fdbclient/ThreadSafeTransaction.h

@@ -43,9 +43,11 @@ public:
 
 	ThreadFuture<int64_t> rebootWorker(const StringRef& address, bool check, int duration) override;
 	ThreadFuture<Void> forceRecoveryWithDataLoss(const StringRef& dcid) override;
-	ThreadFuture<Void> createSnapshot(const StringRef& snapshot_command) override;
+        ThreadFuture<Void>
+        createSnapshot(const StringRef &uid,
+                       const StringRef &snapshot_command) override;
 
-private:
+      private:
 	friend class ThreadSafeTransaction;
 	DatabaseContext* db;
 public:  // Internal use only

flow/error_definitions.h

@@ -236,6 +236,7 @@ ERROR( snap_path_not_whitelisted, 2505, "Snapshot create binary path not whiteli
 ERROR( snap_not_fully_recovered_unsupported, 2506, "Unsupported when the cluster is not fully recovered")
 ERROR( snap_log_anti_quorum_unsupported, 2507, "Unsupported when log anti quorum is configured")
 ERROR( snap_with_recovery_unsupported, 2508, "Cluster recovery during snapshot operation not supported")
+ERROR( snap_invalid_uid_string, 2509, "The given uid string is not a 32-length hex string")
 
 // 4xxx Internal errors (those that should be generated only by bugs) are decimal 4xxx
 ERROR( unknown_error, 4000, "An unknown error occurred" )  // C++ exception not of type Error