Merge pull request #4199 from sfc-gh-satherton/file-backup-write-buffering

Add write buffering to BackupContainerLocalDirectory::BackupFile

fdbclient/BackupContainer.actor.cpp

@@ -1277,15 +1277,39 @@ public:
 
 	class BackupFile : public IBackupFile, ReferenceCounted<BackupFile> {
 	public:
-		BackupFile(std::string fileName, Reference<IAsyncFile> file, std::string finalFullPath) : IBackupFile(fileName), m_file(file), m_finalFullPath(finalFullPath) {}
+		BackupFile(std::string fileName, Reference<IAsyncFile> file, std::string finalFullPath)
+			: IBackupFile(fileName), m_file(file), m_finalFullPath(finalFullPath), m_writeOffset(0)
+		{
+			m_buffer.reserve(m_buffer.arena(), CLIENT_KNOBS->BACKUP_LOCAL_FILE_WRITE_BLOCK);
+		}
 
 		Future<Void> append(const void *data, int len) {
-			Future<Void> r = m_file->write(data, len, m_offset);
-			m_offset += len;
+			m_buffer.append(m_buffer.arena(), (const uint8_t *)data, len);
+
+			if(m_buffer.size() >= CLIENT_KNOBS->BACKUP_LOCAL_FILE_WRITE_BLOCK) {
+				return flush(CLIENT_KNOBS->BACKUP_LOCAL_FILE_WRITE_BLOCK);
+			}
+
+			return Void();
+		}
+
+		Future<Void> flush(int size) {
+			ASSERT(size <= m_buffer.size());
+
+			// Keep a reference to the old buffer
+			Standalone<VectorRef<uint8_t>> old = m_buffer;
+			// Make a new buffer, initialized with the excess bytes over the block size from the old buffer
+			m_buffer = Standalone<VectorRef<uint8_t>>(old.slice(size, old.size()));
+
+			// Write the old buffer to the underlying file and update the write offset
+			Future<Void> r = holdWhile(old, m_file->write(old.begin(), size, m_writeOffset));
+			m_writeOffset += size;
+
 			return r;
 		}
 
 		ACTOR static Future<Void> finish_impl(Reference<BackupFile> f) {
+			wait(f->flush(f->m_buffer.size()));
 			wait(f->m_file->truncate(f->size()));  // Some IAsyncFile implementations extend in whole block sizes.
 			wait(f->m_file->sync());
 			std::string name = f->m_file->getFilename();
@@ -1294,6 +1318,10 @@ public:
 			return Void();
 		}
 
+		int64_t size() const {
+			return m_buffer.size() + m_writeOffset;
+		}
+
 		Future<Void> finish() {
 			return finish_impl(Reference<BackupFile>::addRef(this));
 		}
@@ -1303,6 +1331,8 @@ public:
 
 	private:
 		Reference<IAsyncFile> m_file;
+		Standalone<VectorRef<uint8_t>> m_buffer;
+		int64_t m_writeOffset;
 		std::string m_finalFullPath;
 	};
 
@@ -1435,7 +1465,7 @@ public:
 
 	class BackupFile : public IBackupFile, ReferenceCounted<BackupFile> {
 	public:
-		BackupFile(std::string fileName, Reference<IAsyncFile> file) : IBackupFile(fileName), m_file(file) {}
+		BackupFile(std::string fileName, Reference<IAsyncFile> file) : IBackupFile(fileName), m_file(file), m_offset(0) {}
 
 		Future<Void> append(const void *data, int len) {
 			Future<Void> r = m_file->write(data, len, m_offset);
@@ -1448,10 +1478,15 @@ public:
 			return map(m_file->sync(), [=](Void _) { self->m_file.clear(); return Void(); });
 		}
 
+		int64_t size() const {
+			return m_offset;
+		}
+
 		void addref() { return ReferenceCounted<BackupFile>::addref(); }
 		void delref() { return ReferenceCounted<BackupFile>::delref(); }
 	private:
 		Reference<IAsyncFile> m_file;
+		int64_t m_offset;
 	};
 
 	Future<Reference<IBackupFile>> writeFile(std::string path) {

fdbclient/BackupContainer.h

@@ -36,7 +36,7 @@ Future<Version> timeKeeperVersionFromDatetime(std::string const &datetime, Datab
 // TODO: Move the log file and range file format encoding/decoding stuff to this file and behind interfaces.
 class IBackupFile {
 public:
-	IBackupFile(std::string fileName) : m_fileName(fileName), m_offset(0) {}
+	IBackupFile(std::string fileName) : m_fileName(fileName) {}
 	virtual ~IBackupFile() {}
 	// Backup files are append-only and cannot have more than 1 append outstanding at once.
 	virtual Future<Void> append(const void *data, int len) = 0;
@@ -44,16 +44,13 @@ public:
 	inline std::string getFileName() const {
 		return m_fileName;
 	}
-	inline int64_t size() const {
-		return m_offset;
-	}
+	virtual int64_t size() const = 0;
 	virtual void addref() = 0;
 	virtual void delref() = 0;
 
 	Future<Void> appendStringRefWithLen(Standalone<StringRef> s);
 protected:
 	std::string m_fileName;
-	int64_t m_offset;
 };
 
 // Structures for various backup components

fdbclient/Knobs.cpp

@@ -115,6 +115,7 @@ ClientKnobs::ClientKnobs(bool randomize) {
 	init( TASKBUCKET_MAX_TASK_KEYS,               1000 ); if( randomize && BUGGIFY ) TASKBUCKET_MAX_TASK_KEYS = 20;
 
 	//Backup
+	init( BACKUP_LOCAL_FILE_WRITE_BLOCK,     1024*1024 ); if( randomize && BUGGIFY ) BACKUP_LOCAL_FILE_WRITE_BLOCK = 100;
 	init( BACKUP_CONCURRENT_DELETES,               100 );
 	init( BACKUP_SIMULATED_LIMIT_BYTES,		       1e6 ); if( randomize && BUGGIFY ) BACKUP_SIMULATED_LIMIT_BYTES = 1000;
 	init( BACKUP_GET_RANGE_LIMIT_BYTES,		       1e6 );

fdbclient/Knobs.h

@@ -117,6 +117,7 @@ public:
 	int TASKBUCKET_MAX_TASK_KEYS;
 
 	// Backup
+	int BACKUP_LOCAL_FILE_WRITE_BLOCK;
 	int BACKUP_CONCURRENT_DELETES;
 	int BACKUP_SIMULATED_LIMIT_BYTES;
 	int BACKUP_GET_RANGE_LIMIT_BYTES;