Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix flaky, dubious LdbCmdTest::*DumpFileChecksum* #8898

Closed
wants to merge 1 commit into from
Closed

Fix flaky, dubious LdbCmdTest::*DumpFileChecksum* #8898

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 14 additions & 12 deletions tools/ldb_cmd.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1206,9 +1206,9 @@ void ManifestDumpCommand::DoCommand() {
// ----------------------------------------------------------------------------
namespace {

void GetLiveFilesChecksumInfoFromVersionSet(Options options,
const std::string& db_path,
FileChecksumList* checksum_list) {
Status GetLiveFilesChecksumInfoFromVersionSet(Options options,
const std::string& db_path,
FileChecksumList* checksum_list) {
EnvOptions sopt;
Status s;
std::string dbname(db_path);
Expand Down Expand Up @@ -1238,9 +1238,7 @@ void GetLiveFilesChecksumInfoFromVersionSet(Options options,
if (s.ok()) {
s = versions.GetLiveFilesChecksumInfo(checksum_list);
}
if (!s.ok()) {
fprintf(stderr, "Error Status: %s", s.ToString().c_str());
}
return s;
}

} // namespace
Expand Down Expand Up @@ -1279,14 +1277,14 @@ void FileChecksumDumpCommand::DoCommand() {
// ......

std::unique_ptr<FileChecksumList> checksum_list(NewFileChecksumList());
GetLiveFilesChecksumInfoFromVersionSet(options_, db_path_,
checksum_list.get());
if (checksum_list != nullptr) {
Status s = GetLiveFilesChecksumInfoFromVersionSet(options_, db_path_,
checksum_list.get());
if (s.ok() && checksum_list != nullptr) {
std::vector<uint64_t> file_numbers;
std::vector<std::string> checksums;
std::vector<std::string> checksum_func_names;
Status s = checksum_list->GetAllFileChecksums(&file_numbers, &checksums,
&checksum_func_names);
s = checksum_list->GetAllFileChecksums(&file_numbers, &checksums,
&checksum_func_names);
if (s.ok()) {
for (size_t i = 0; i < file_numbers.size(); i++) {
assert(i < file_numbers.size());
Expand All @@ -1301,8 +1299,12 @@ void FileChecksumDumpCommand::DoCommand() {
fprintf(stdout, "%" PRId64 ", %s, %s\n", file_numbers[i],
checksum_func_names[i].c_str(), checksum.c_str());
}
fprintf(stdout, "Print SST file checksum information finished \n");
}
fprintf(stdout, "Print SST file checksum information finished \n");
}

if (!s.ok()) {
exec_state_ = LDBCommandExecuteResult::Failed(s.ToString());
}
}

Expand Down
67 changes: 51 additions & 16 deletions tools/ldb_cmd_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,15 +309,20 @@ TEST_F(LdbCmdTest, DumpFileChecksumNoChecksum) {
ASSERT_OK(db->Put(wopts, buf, v));
}
ASSERT_OK(db->Flush(fopts));
ASSERT_OK(db->Close());
delete db;

char arg1[] = "./ldb";
char arg2[1024];
snprintf(arg2, sizeof(arg2), "--db=%s", dbname.c_str());
char arg3[] = "file_checksum_dump";
char* argv[] = {arg1, arg2, arg3};
char arg4[] = "--hex";
char* argv[] = {arg1, arg2, arg3, arg4};

ASSERT_EQ(0,
LDBCommandRunner::RunCommand(3, argv, opts, LDBOptions(), nullptr));
LDBCommandRunner::RunCommand(4, argv, opts, LDBOptions(), nullptr));

ASSERT_OK(DB::Open(opts, dbname, &db));

// Verify each sst file checksum value and checksum name
FileChecksumTestHelper fct_helper(opts, db, dbname);
Expand All @@ -336,8 +341,13 @@ TEST_F(LdbCmdTest, DumpFileChecksumNoChecksum) {
FileChecksumTestHelper fct_helper_ac(opts, db, dbname);
ASSERT_OK(fct_helper_ac.VerifyEachFileChecksum());

ASSERT_OK(db->Close());
delete db;

ASSERT_EQ(0,
LDBCommandRunner::RunCommand(3, argv, opts, LDBOptions(), nullptr));
LDBCommandRunner::RunCommand(4, argv, opts, LDBOptions(), nullptr));

ASSERT_OK(DB::Open(opts, dbname, &db));

// Verify the checksum information in memory is the same as that in Manifest;
std::vector<LiveFileMetaData> live_files;
Expand Down Expand Up @@ -390,14 +400,19 @@ TEST_F(LdbCmdTest, BlobDBDumpFileChecksumNoChecksum) {
ASSERT_OK(db->Put(wopts, oss.str(), v));
}
ASSERT_OK(db->Flush(fopts));
ASSERT_OK(db->Close());
delete db;

char arg1[] = "./ldb";
std::string arg2_str = "--db=" + dbname;
char arg3[] = "file_checksum_dump";
char* argv[] = {arg1, const_cast<char*>(arg2_str.c_str()), arg3};
char arg4[] = "--hex";
char* argv[] = {arg1, const_cast<char*>(arg2_str.c_str()), arg3, arg4};

ASSERT_EQ(0,
LDBCommandRunner::RunCommand(3, argv, opts, LDBOptions(), nullptr));
LDBCommandRunner::RunCommand(4, argv, opts, LDBOptions(), nullptr));

ASSERT_OK(DB::Open(opts, dbname, &db));

// Verify each sst and blob file checksum value and checksum name
FileChecksumTestHelper fct_helper(opts, db, dbname);
Expand All @@ -419,10 +434,11 @@ TEST_F(LdbCmdTest, BlobDBDumpFileChecksumNoChecksum) {
FileChecksumTestHelper fct_helper_ac(opts, db, dbname);
ASSERT_OK(fct_helper_ac.VerifyEachFileChecksum());

ASSERT_EQ(0,
LDBCommandRunner::RunCommand(3, argv, opts, LDBOptions(), nullptr));

ASSERT_OK(db->Close());
delete db;

ASSERT_EQ(0,
LDBCommandRunner::RunCommand(4, argv, opts, LDBOptions(), nullptr));
}

TEST_F(LdbCmdTest, DumpFileChecksumCRC32) {
Expand Down Expand Up @@ -469,15 +485,20 @@ TEST_F(LdbCmdTest, DumpFileChecksumCRC32) {
ASSERT_OK(db->Put(wopts, buf, v));
}
ASSERT_OK(db->Flush(fopts));
ASSERT_OK(db->Close());
delete db;

char arg1[] = "./ldb";
char arg2[1024];
snprintf(arg2, sizeof(arg2), "--db=%s", dbname.c_str());
char arg3[] = "file_checksum_dump";
char* argv[] = {arg1, arg2, arg3};
char arg4[] = "--hex";
char* argv[] = {arg1, arg2, arg3, arg4};

ASSERT_EQ(0,
LDBCommandRunner::RunCommand(3, argv, opts, LDBOptions(), nullptr));
LDBCommandRunner::RunCommand(4, argv, opts, LDBOptions(), nullptr));

ASSERT_OK(DB::Open(opts, dbname, &db));

// Verify each sst file checksum value and checksum name
FileChecksumTestHelper fct_helper(opts, db, dbname);
Expand All @@ -496,14 +517,21 @@ TEST_F(LdbCmdTest, DumpFileChecksumCRC32) {
FileChecksumTestHelper fct_helper_ac(opts, db, dbname);
ASSERT_OK(fct_helper_ac.VerifyEachFileChecksum());

ASSERT_OK(db->Close());
delete db;

ASSERT_EQ(0,
LDBCommandRunner::RunCommand(3, argv, opts, LDBOptions(), nullptr));
LDBCommandRunner::RunCommand(4, argv, opts, LDBOptions(), nullptr));

ASSERT_OK(DB::Open(opts, dbname, &db));

// Verify the checksum information in memory is the same as that in Manifest;
std::vector<LiveFileMetaData> live_files;
db->GetLiveFilesMetaData(&live_files);
delete db;
ASSERT_OK(fct_helper_ac.VerifyChecksumInManifest(live_files));

ASSERT_OK(db->Close());
delete db;
}

TEST_F(LdbCmdTest, BlobDBDumpFileChecksumCRC32) {
Expand Down Expand Up @@ -551,14 +579,19 @@ TEST_F(LdbCmdTest, BlobDBDumpFileChecksumCRC32) {
ASSERT_OK(db->Put(wopts, oss.str(), v));
}
ASSERT_OK(db->Flush(fopts));
ASSERT_OK(db->Close());
delete db;

char arg1[] = "./ldb";
std::string arg2_str = "--db=" + dbname;
char arg3[] = "file_checksum_dump";
char* argv[] = {arg1, const_cast<char*>(arg2_str.c_str()), arg3};
char arg4[] = "--hex";
char* argv[] = {arg1, const_cast<char*>(arg2_str.c_str()), arg3, arg4};

ASSERT_EQ(0,
LDBCommandRunner::RunCommand(3, argv, opts, LDBOptions(), nullptr));
LDBCommandRunner::RunCommand(4, argv, opts, LDBOptions(), nullptr));

ASSERT_OK(DB::Open(opts, dbname, &db));

// Verify each sst and blob file checksum value and checksum name
FileChecksumTestHelper fct_helper(opts, db, dbname);
Expand All @@ -580,9 +613,11 @@ TEST_F(LdbCmdTest, BlobDBDumpFileChecksumCRC32) {
FileChecksumTestHelper fct_helper_ac(opts, db, dbname);
ASSERT_OK(fct_helper_ac.VerifyEachFileChecksum());

ASSERT_EQ(0,
LDBCommandRunner::RunCommand(3, argv, opts, LDBOptions(), nullptr));
ASSERT_OK(db->Close());
delete db;

ASSERT_EQ(0,
LDBCommandRunner::RunCommand(4, argv, opts, LDBOptions(), nullptr));
}

TEST_F(LdbCmdTest, OptionParsing) {
Expand Down