Remove unnecessary copies in BackupContainer code

This commit is contained in:
sfc-gh-tclinkenbeard 2020-10-21 22:19:15 -07:00
parent 1d28285cc5
commit 6a27619544
10 changed files with 50 additions and 51 deletions

View File

@ -248,7 +248,7 @@ std::vector<std::string> IBackupContainer::getURLFormats() {
}
// Get an IBackupContainer based on a container URL string
Reference<IBackupContainer> IBackupContainer::openContainer(std::string url) {
Reference<IBackupContainer> IBackupContainer::openContainer(const std::string& url) {
static std::map<std::string, Reference<IBackupContainer>> m_cache;
Reference<IBackupContainer>& r = m_cache[url];
@ -338,7 +338,7 @@ ACTOR Future<std::vector<std::string>> listContainers_impl(std::string baseURL)
}
}
Future<std::vector<std::string>> IBackupContainer::listContainers(std::string baseURL) {
Future<std::vector<std::string>> IBackupContainer::listContainers(const std::string& baseURL) {
return listContainers_impl(baseURL);
}

View File

@ -40,7 +40,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(const std::string& fileName) : m_fileName(fileName), m_offset(0) {}
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;
@ -247,7 +247,7 @@ public:
int64_t totalBytes) = 0;
// Open a file for read by name
virtual Future<Reference<IAsyncFile>> readFile(std::string name) = 0;
virtual Future<Reference<IAsyncFile>> readFile(const std::string& name) = 0;
// Returns the key ranges in the snapshot file. This is an expensive function
// and should only be used in simulation for sanity check.
@ -289,9 +289,9 @@ public:
bool logsOnly = false, Version beginVersion = -1) = 0;
// Get an IBackupContainer based on a container spec string
static Reference<IBackupContainer> openContainer(std::string url);
static Reference<IBackupContainer> openContainer(const std::string& url);
static std::vector<std::string> getURLFormats();
static Future<std::vector<std::string>> listContainers(std::string baseURL);
static Future<std::vector<std::string>> listContainers(const std::string& baseURL);
std::string getURL() const {
return URL;

View File

@ -247,7 +247,7 @@ Future<bool> BackupContainerAzureBlobStore::exists() {
});
}
Future<Reference<IAsyncFile>> BackupContainerAzureBlobStore::readFile(std::string fileName) {
Future<Reference<IAsyncFile>> BackupContainerAzureBlobStore::readFile(const std::string& fileName) {
return BackupContainerAzureBlobStoreImpl::readFile(this, fileName);
}
@ -256,7 +256,7 @@ Future<Reference<IBackupFile>> BackupContainerAzureBlobStore::writeFile(const st
}
Future<BackupContainerFileSystem::FilesAndSizesT> BackupContainerAzureBlobStore::listFiles(
std::string path, std::function<bool(std::string const&)> folderPathFilter) {
const std::string& path, std::function<bool(std::string const&)> folderPathFilter) {
return asyncTaskThread.execAsync([client = this->client.get(), containerName = this->containerName, path = path,
folderPathFilter = folderPathFilter] {
FilesAndSizesT result;
@ -265,7 +265,7 @@ Future<BackupContainerFileSystem::FilesAndSizesT> BackupContainerAzureBlobStore:
});
}
Future<Void> BackupContainerAzureBlobStore::deleteFile(std::string fileName) {
Future<Void> BackupContainerAzureBlobStore::deleteFile(const std::string& fileName) {
return asyncTaskThread.execAsync(
[containerName = this->containerName, fileName = fileName, client = client.get()]() {
client->delete_blob(containerName, fileName).wait();

View File

@ -50,14 +50,14 @@ public:
Future<bool> exists() override;
Future<Reference<IAsyncFile>> readFile(std::string fileName) override;
Future<Reference<IAsyncFile>> readFile(const std::string& fileName) override;
Future<Reference<IBackupFile>> writeFile(const std::string& fileName) override;
Future<FilesAndSizesT> listFiles(std::string path = "",
Future<FilesAndSizesT> listFiles(const std::string& path = "",
std::function<bool(std::string const&)> folderPathFilter = nullptr) override;
Future<Void> deleteFile(std::string fileName) override;
Future<Void> deleteFile(const std::string& fileName) override;
Future<Void> deleteContainer(int* pNumDeleted) override;
};

View File

@ -734,7 +734,7 @@ std::string BackupContainerFileSystem::snapshotFolderString(Version snapshotBegi
return format("kvranges/snapshot.%018" PRId64, snapshotBeginVersion);
}
Version BackupContainerFileSystem::extractSnapshotBeginVersion(std::string path) {
Version BackupContainerFileSystem::extractSnapshotBeginVersion(const std::string& path) {
Version snapshotBeginVersion;
if (sscanf(path.c_str(), "kvranges/snapshot.%018" SCNd64, &snapshotBeginVersion) == 1) {
return snapshotBeginVersion;
@ -776,7 +776,7 @@ Future<Reference<IBackupFile>> BackupContainerFileSystem::writeRangeFile(Version
format("/%d/", snapshotFileCount / (BUGGIFY ? 1 : 5000)) + fileName);
}
std::string BackupContainerFileSystem::fileNameOnly(std::string path) {
std::string BackupContainerFileSystem::fileNameOnly(const std::string& path) {
// Find the last forward slash position, defaulting to 0 if not found
int pos = path.find_last_of('/');
if (pos == std::string::npos) {
@ -790,7 +790,7 @@ std::string BackupContainerFileSystem::fileNameOnly(std::string path) {
return path.substr(pos + 1);
}
bool BackupContainerFileSystem::pathToRangeFile(RangeFile& out, std::string path, int64_t size) {
bool BackupContainerFileSystem::pathToRangeFile(RangeFile& out, const std::string& path, int64_t size) {
std::string name = fileNameOnly(path);
RangeFile f;
f.fileName = path;
@ -804,7 +804,7 @@ bool BackupContainerFileSystem::pathToRangeFile(RangeFile& out, std::string path
return false;
}
bool BackupContainerFileSystem::pathToLogFile(LogFile& out, std::string path, int64_t size) {
bool BackupContainerFileSystem::pathToLogFile(LogFile& out, const std::string& path, int64_t size) {
std::string name = fileNameOnly(path);
LogFile f;
f.fileName = path;
@ -824,7 +824,7 @@ bool BackupContainerFileSystem::pathToLogFile(LogFile& out, std::string path, in
return false;
}
bool BackupContainerFileSystem::pathToKeyspaceSnapshotFile(KeyspaceSnapshotFile& out, std::string path) {
bool BackupContainerFileSystem::pathToKeyspaceSnapshotFile(KeyspaceSnapshotFile& out, const std::string& path) {
std::string name = fileNameOnly(path);
KeyspaceSnapshotFile f;
f.fileName = path;
@ -1264,7 +1264,7 @@ Future<KeyRange> BackupContainerFileSystem::getSnapshotFileKeyRange(const RangeF
return getSnapshotFileKeyRange_impl(Reference<BackupContainerFileSystem>::addRef(this), file);
}
Optional<RestorableFileSet> BackupContainerFileSystem::getRestoreSetFromLogs(std::vector<LogFile> logs,
Optional<RestorableFileSet> BackupContainerFileSystem::getRestoreSetFromLogs(const std::vector<LogFile>& logs,
Version targetVersion,
RestorableFileSet restorable) {
Version end = logs.begin()->endVersion;
@ -1277,7 +1277,6 @@ Optional<RestorableFileSet> BackupContainerFileSystem::getRestoreSetFromLogs(std
return Optional<RestorableFileSet>();
}
Future<Optional<RestorableFileSet>> BackupContainerFileSystem::getRestoreSet(Version targetVersion,
VectorRef<KeyRangeRef> keyRangesFilter,
bool logsOnly, Version beginVersion) {

View File

@ -103,17 +103,17 @@ public:
// Although not required, an implementation can avoid traversing unwanted subfolders
// by calling folderPathFilter(absoluteFolderPath) and checking for a false return value.
using FilesAndSizesT = std::vector<std::pair<std::string, int64_t>>;
virtual Future<FilesAndSizesT> listFiles(std::string path = "",
virtual Future<FilesAndSizesT> listFiles(const std::string& path = "",
std::function<bool(std::string const&)> folderPathFilter = nullptr) = 0;
// Open a file for read by fileName
Future<Reference<IAsyncFile>> readFile(std::string fileName) override = 0;
Future<Reference<IAsyncFile>> readFile(const std::string& fileName) override = 0;
// Open a file for write by fileName
virtual Future<Reference<IBackupFile>> writeFile(const std::string& fileName) = 0;
// Delete a file
virtual Future<Void> deleteFile(std::string fileName) = 0;
virtual Future<Void> deleteFile(const std::string& fileName) = 0;
// Delete entire container. During the process, if pNumDeleted is not null it will be
// updated with the count of deleted files so that progress can be seen.
@ -134,7 +134,7 @@ public:
static std::string snapshotFolderString(Version snapshotBeginVersion);
// Extract the snapshot begin version from a path
static Version extractSnapshotBeginVersion(std::string path);
static Version extractSnapshotBeginVersion(const std::string& path);
// The innermost folder covers 100,000 seconds (1e11 versions) which is 5,000 mutation log files at current
// settings.
@ -150,13 +150,13 @@ public:
// Find what should be the filename of a path by finding whatever is after the last forward or backward slash, or
// failing to find those, the whole string.
static std::string fileNameOnly(std::string path);
static std::string fileNameOnly(const std::string& path);
static bool pathToRangeFile(RangeFile& out, std::string path, int64_t size);
static bool pathToRangeFile(RangeFile& out, const std::string& path, int64_t size);
static bool pathToLogFile(LogFile& out, std::string path, int64_t size);
static bool pathToLogFile(LogFile& out, const std::string& path, int64_t size);
static bool pathToKeyspaceSnapshotFile(KeyspaceSnapshotFile& out, std::string path);
static bool pathToKeyspaceSnapshotFile(KeyspaceSnapshotFile& out, const std::string& path);
Future<std::pair<std::vector<RangeFile>, std::map<std::string, KeyRange>>> readKeyspaceSnapshot(
KeyspaceSnapshotFile snapshot);
@ -233,7 +233,7 @@ public:
Future<KeyRange> getSnapshotFileKeyRange(const RangeFile& file) final;
static Optional<RestorableFileSet> getRestoreSetFromLogs(std::vector<LogFile> logs, Version targetVersion,
static Optional<RestorableFileSet> getRestoreSetFromLogs(const std::vector<LogFile>& logs, Version targetVersion,
RestorableFileSet restorable);
Future<Optional<RestorableFileSet>> getRestoreSet(Version targetVersion, VectorRef<KeyRangeRef> keyRangesFilter,
@ -241,7 +241,7 @@ public:
private:
struct VersionProperty {
VersionProperty(Reference<BackupContainerFileSystem> bc, std::string name)
VersionProperty(Reference<BackupContainerFileSystem> bc, const std::string& name)
: bc(bc), path("properties/" + name) {}
Reference<BackupContainerFileSystem> bc;
std::string path;

View File

@ -91,7 +91,7 @@ std::string BackupContainerLocalDirectory::getURLFormat() {
return "file://</path/to/base/dir/>";
}
BackupContainerLocalDirectory::BackupContainerLocalDirectory(std::string url) {
BackupContainerLocalDirectory::BackupContainerLocalDirectory(const std::string& url) {
std::string path;
if (url.find("file://") != 0) {
TraceEvent(SevWarn, "BackupContainerLocalDirectory")
@ -121,7 +121,7 @@ BackupContainerLocalDirectory::BackupContainerLocalDirectory(std::string url) {
m_path = path;
}
Future<std::vector<std::string>> BackupContainerLocalDirectory::listURLs(std::string url) {
Future<std::vector<std::string>> BackupContainerLocalDirectory::listURLs(const std::string& url) {
std::string path;
if (url.find("file://") != 0) {
TraceEvent(SevWarn, "BackupContainerLocalDirectory")
@ -164,7 +164,7 @@ Future<bool> BackupContainerLocalDirectory::exists() {
return directoryExists(m_path);
}
Future<Reference<IAsyncFile>> BackupContainerLocalDirectory::readFile(std::string path) {
Future<Reference<IAsyncFile>> BackupContainerLocalDirectory::readFile(const std::string& path) {
int flags = IAsyncFile::OPEN_NO_AIO | IAsyncFile::OPEN_READONLY | IAsyncFile::OPEN_UNCACHED;
// Simulation does not properly handle opening the same file from multiple machines using a shared filesystem,
// so create a symbolic link to make each file opening appear to be unique. This could also work in production
@ -226,13 +226,13 @@ Future<Reference<IBackupFile>> BackupContainerLocalDirectory::writeFile(const st
return map(f, [=](Reference<IAsyncFile> f) { return Reference<IBackupFile>(new BackupFile(path, f, fullPath)); });
}
Future<Void> BackupContainerLocalDirectory::deleteFile(std::string path) {
Future<Void> BackupContainerLocalDirectory::deleteFile(const std::string& path) {
::deleteFile(joinPath(m_path, path));
return Void();
}
Future<BackupContainerFileSystem::FilesAndSizesT> BackupContainerLocalDirectory::listFiles(
std::string path, std::function<bool(std::string const&)>) {
const std::string& path, std::function<bool(std::string const&)>) {
return listFiles_impl(path, m_path);
}

View File

@ -32,22 +32,22 @@ public:
static std::string getURLFormat();
BackupContainerLocalDirectory(std::string url);
BackupContainerLocalDirectory(const std::string& url);
static Future<std::vector<std::string>> listURLs(std::string url);
static Future<std::vector<std::string>> listURLs(const std::string& url);
Future<Void> create() final;
// The container exists if the folder it resides in exists
Future<bool> exists() final;
Future<Reference<IAsyncFile>> readFile(std::string path) final;
Future<Reference<IAsyncFile>> readFile(const std::string& path) final;
Future<Reference<IBackupFile>> writeFile(const std::string& path) final;
Future<Void> deleteFile(std::string path) final;
Future<Void> deleteFile(const std::string& path) final;
Future<FilesAndSizesT> listFiles(std::string path, std::function<bool(std::string const&)>) final;
Future<FilesAndSizesT> listFiles(const std::string& path, std::function<bool(std::string const&)>) final;
Future<Void> deleteContainer(int* pNumDeleted) final;

View File

@ -120,7 +120,7 @@ public:
const std::string BackupContainerS3BlobStoreImpl::DATAFOLDER = "data";
const std::string BackupContainerS3BlobStoreImpl::INDEXFOLDER = "backups";
std::string BackupContainerS3BlobStore::dataPath(const std::string path) {
std::string BackupContainerS3BlobStore::dataPath(const std::string& path) {
return BackupContainerS3BlobStoreImpl::DATAFOLDER + "/" + m_name + "/" + path;
}
@ -129,7 +129,7 @@ std::string BackupContainerS3BlobStore::indexEntry() {
return BackupContainerS3BlobStoreImpl::INDEXFOLDER + "/" + m_name;
}
BackupContainerS3BlobStore::BackupContainerS3BlobStore(Reference<BlobStoreEndpoint> bstore, std::string name,
BackupContainerS3BlobStore::BackupContainerS3BlobStore(Reference<BlobStoreEndpoint> bstore, const std::string& name,
const BlobStoreEndpoint::ParametersT& params)
: m_bstore(bstore), m_name(name), m_bucket("FDB_BACKUPS_V2") {
@ -158,7 +158,7 @@ std::string BackupContainerS3BlobStore::getURLFormat() {
return BlobStoreEndpoint::getURLFormat(true) + " (Note: The 'bucket' parameter is required.)";
}
Future<Reference<IAsyncFile>> BackupContainerS3BlobStore::readFile(std::string path) {
Future<Reference<IAsyncFile>> BackupContainerS3BlobStore::readFile(const std::string& path) {
return Reference<IAsyncFile>(new AsyncFileReadAheadCache(
Reference<IAsyncFile>(new AsyncFileBlobStoreRead(m_bstore, m_bucket, dataPath(path))),
m_bstore->knobs.read_block_size, m_bstore->knobs.read_ahead_blocks, m_bstore->knobs.concurrent_reads_per_file,
@ -166,7 +166,7 @@ Future<Reference<IAsyncFile>> BackupContainerS3BlobStore::readFile(std::string p
}
Future<std::vector<std::string>> BackupContainerS3BlobStore::listURLs(Reference<BlobStoreEndpoint> bstore,
std::string bucket) {
const std::string& bucket) {
return BackupContainerS3BlobStoreImpl::listURLs(bstore, bucket);
}
@ -175,12 +175,12 @@ Future<Reference<IBackupFile>> BackupContainerS3BlobStore::writeFile(const std::
path, Reference<IAsyncFile>(new AsyncFileBlobStoreWrite(m_bstore, m_bucket, dataPath(path)))));
}
Future<Void> BackupContainerS3BlobStore::deleteFile(std::string path) {
Future<Void> BackupContainerS3BlobStore::deleteFile(const std::string& path) {
return m_bstore->deleteObject(m_bucket, dataPath(path));
}
Future<BackupContainerFileSystem::FilesAndSizesT> BackupContainerS3BlobStore::listFiles(
std::string path, std::function<bool(std::string const&)> pathFilter) {
const std::string& path, std::function<bool(std::string const&)> pathFilter) {
return BackupContainerS3BlobStoreImpl::listFiles(Reference<BackupContainerS3BlobStore>::addRef(this), path,
pathFilter);
}

View File

@ -32,7 +32,7 @@ class BackupContainerS3BlobStore final : public BackupContainerFileSystem,
// All backup data goes into a single bucket
std::string m_bucket;
std::string dataPath(const std::string path);
std::string dataPath(const std::string& path);
// Get the path of the backups's index entry
std::string indexEntry();
@ -40,7 +40,7 @@ class BackupContainerS3BlobStore final : public BackupContainerFileSystem,
friend class BackupContainerS3BlobStoreImpl;
public:
BackupContainerS3BlobStore(Reference<BlobStoreEndpoint> bstore, std::string name,
BackupContainerS3BlobStore(Reference<BlobStoreEndpoint> bstore, const std::string& name,
const BlobStoreEndpoint::ParametersT& params);
void addref() override;
@ -48,15 +48,15 @@ public:
static std::string getURLFormat();
Future<Reference<IAsyncFile>> readFile(std::string path) final;
Future<Reference<IAsyncFile>> readFile(const std::string& path) final;
static Future<std::vector<std::string>> listURLs(Reference<BlobStoreEndpoint> bstore, std::string bucket);
static Future<std::vector<std::string>> listURLs(Reference<BlobStoreEndpoint> bstore, const std::string& bucket);
Future<Reference<IBackupFile>> writeFile(const std::string& path) final;
Future<Void> deleteFile(std::string path) final;
Future<Void> deleteFile(const std::string& path) final;
Future<FilesAndSizesT> listFiles(std::string path, std::function<bool(std::string const&)> pathFilter) final;
Future<FilesAndSizesT> listFiles(const std::string& path, std::function<bool(std::string const&)> pathFilter) final;
Future<Void> create() final;