Skip to content

Commit

Permalink
Fix double slashes in user-provided db path. (facebook#8439)
Browse files Browse the repository at this point in the history
Summary:
At the moment, the following command : "`./ --db=mypath/ dump_file_files`" returns a series of erronous names with double slashes, ie: "`mypath//000xxx.sst`", including manifest file names with double slashes "`mypath//MANIFEST-00XXX`", whereas "`./ --db=mypath dump_file_files`" correctly returns "`mypath/000xxx.sst`" and "`mypath/MANIFEST-00XXX`".

This (very short) PR simply checks if there is a need to add or remove any '`/`' character when the `db_path` and `manifest_filename`/sst `filenames` are concatenated.

Pull Request resolved: facebook#8439

Reviewed By: akankshamahajan15

Differential Revision: D29301349

Pulled By: bjlemaire

fbshipit-source-id: 3e9e58f9749d278b654ae838fcee13ad698705a8
  • Loading branch information
bjlemaire authored and facebook-github-bot committed Jun 22, 2021
1 parent f89423a commit 0a1aed4
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 1 deletion.
11 changes: 10 additions & 1 deletion tools/ldb_cmd.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3352,6 +3352,11 @@ void DBFileDumperCommand::DoCommand() {
// remove the trailing '\n'
manifest_filename.resize(manifest_filename.size() - 1);
std::string manifest_filepath = db_->GetName() + "/" + manifest_filename;
// Correct concatenation of filepath and filename:
// Check that there is no double slashes (or more!) when concatenation
// happens.
manifest_filepath = NormalizePath(manifest_filepath);

std::cout << manifest_filepath << std::endl;
DumpManifestFile(options_, manifest_filepath, false, false, false);
std::cout << std::endl;
Expand All @@ -3361,7 +3366,11 @@ void DBFileDumperCommand::DoCommand() {
std::vector<LiveFileMetaData> metadata;
db_->GetLiveFilesMetaData(&metadata);
for (auto& fileMetadata : metadata) {
std::string filename = fileMetadata.db_path + fileMetadata.name;
std::string filename = fileMetadata.db_path + "/" + fileMetadata.name;
// Correct concatenation of filepath and filename:
// Check that there is no double slashes (or more!) when concatenation
// happens.
filename = NormalizePath(filename);
std::cout << filename << " level:" << fileMetadata.level << std::endl;
std::cout << "------------------------------" << std::endl;
DumpSstFile(options_, filename, false, true);
Expand Down
28 changes: 28 additions & 0 deletions tools/ldb_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -423,8 +423,36 @@ def testDumpLiveFiles(self):
self.assertRunOK("delete x1", "OK")
self.assertRunOK("put x3 y3", "OK")
dumpFilePath = os.path.join(self.TMP_DIR, "dump2")

# Test that if the user provides a db path that ends with
# a slash '/', there is no double (or more!) slashes in the
# SST and manifest file names.

# Add a '/' at the end of dbPath (which normally shouldnt contain any)
if dbPath[-1] != "/":
dbPath += "/"

# Call the dump_live_files function with the edited dbPath name.
self.assertTrue(self.dumpLiveFiles("--db=%s" % dbPath, dumpFilePath))

# Investigate the output
with open(dumpFilePath, "r") as tmp:
data = tmp.read()

# Check that all the SST filenames have a correct full path (no multiple '/').
sstFileList = re.findall(r"%s.*\d+.sst" % dbPath, data)
for sstFilename in sstFileList:
filenumber = re.findall(r"\d+.sst", sstFilename)[0]
self.assertEqual(sstFilename, dbPath+filenumber)

# Check that all the manifest filenames
# have a correct full path (no multiple '/').
manifestFileList = re.findall(r"%s.*MANIFEST-\d+" % dbPath, data)
for manifestFilename in manifestFileList:
filenumber = re.findall(r"(?<=MANIFEST-)\d+", manifestFilename)[0]
self.assertEqual(manifestFilename, dbPath+"MANIFEST-"+filenumber)


def getManifests(self, directory):
return glob.glob(directory + "/MANIFEST-*")

Expand Down

0 comments on commit 0a1aed4

Please sign in to comment.