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

Jump to bottom

Pass SST file checksum information through OnTableFileCreated #7108

Closed
wants to merge 2 commits into from
Closed

Pass SST file checksum information through OnTableFileCreated #7108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
eac406d
initial changes
zhichao-cao Jul 8, 2020
8d98c6f
Added checksum info
zhichao-cao Jul 9, 2020
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
2 changes: 2 additions & 0 deletions HISTORY.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@

### Public API Change
* Expose kTypeDeleteWithTimestamp in EntryType and update GetEntryType() accordingly.
* Added file_checksum and file_checksum_func_name to TableFileCreationInfo, which can pass the table file checksum information through the OnTableFileCreated callback during flush and compaction.


## 6.12 (2020-07-28)
### Public API Change
Expand Down
10 changes: 8 additions & 2 deletions db/builder.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ Status BuildTable(

std::string fname = TableFileName(ioptions.cf_paths, meta->fd.GetNumber(),
meta->fd.GetPathId());
std::string file_checksum = kUnknownFileChecksum;
std::string file_checksum_func_name = kUnknownFileChecksumFuncName;
#ifndef ROCKSDB_LITE
EventHelpers::NotifyTableFileCreationStarted(
ioptions.listeners, dbname, column_family_name, fname, job_id, reason);
Expand Down Expand Up @@ -133,7 +135,8 @@ Status BuildTable(
if (!s.ok()) {
EventHelpers::LogAndNotifyTableFileCreationFinished(
event_logger, ioptions.listeners, dbname, column_family_name, fname,
job_id, meta->fd, kInvalidBlobFileNumber, tp, reason, s);
job_id, meta->fd, kInvalidBlobFileNumber, tp, reason, s,
file_checksum, file_checksum_func_name);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not get rid of the two variables and use kUnknown here?

return s;
}
file->SetIOPriority(io_priority);
Expand Down Expand Up @@ -232,6 +235,8 @@ Status BuildTable(
// Add the checksum information to file metadata.
meta->file_checksum = file_writer->GetFileChecksum();
meta->file_checksum_func_name = file_writer->GetFileChecksumFuncName();
file_checksum = meta->file_checksum;
file_checksum_func_name = meta->file_checksum_func_name;
}

if (s.ok()) {
Expand Down Expand Up @@ -292,7 +297,8 @@ Status BuildTable(
// Output to event logger and fire events.
EventHelpers::LogAndNotifyTableFileCreationFinished(
event_logger, ioptions.listeners, dbname, column_family_name, fname,
job_id, meta->fd, meta->oldest_blob_file_number, tp, reason, s);
job_id, meta->fd, meta->oldest_blob_file_number, tp, reason, s,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Get rid of the temporary vars and use meta->file_checksum here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will deal with the case that (io_status->ok() && !empty) is not satisfied, meta->file_checksum/file_checksum_func_name will be empty. I'm thinking it would be better to pass Unknown in this case.

file_checksum, file_checksum_func_name);

return s;
}
Expand Down
10 changes: 8 additions & 2 deletions db/compaction/compaction_job.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1198,6 +1198,8 @@ Status CompactionJob::FinishCompactionOutputFile(

ColumnFamilyData* cfd = sub_compact->compaction->column_family_data();
const Comparator* ucmp = cfd->user_comparator();
std::string file_checksum = kUnknownFileChecksum;
std::string file_checksum_func_name = kUnknownFileChecksumFuncName;

// Check for iterator errors
Status s = input_status;
Expand Down Expand Up @@ -1397,6 +1399,8 @@ Status CompactionJob::FinishCompactionOutputFile(
meta->file_checksum = sub_compact->outfile->GetFileChecksum();
meta->file_checksum_func_name =
sub_compact->outfile->GetFileChecksumFuncName();
file_checksum = meta->file_checksum;
file_checksum_func_name = meta->file_checksum_func_name;
}
if (s.ok()) {
s = io_s;
Expand Down Expand Up @@ -1453,7 +1457,8 @@ Status CompactionJob::FinishCompactionOutputFile(
EventHelpers::LogAndNotifyTableFileCreationFinished(
event_logger_, cfd->ioptions()->listeners, dbname_, cfd->GetName(), fname,
job_id_, output_fd, oldest_blob_file_number, tp,
TableFileCreationReason::kCompaction, s);
TableFileCreationReason::kCompaction, s, file_checksum,
file_checksum_func_name);

#ifndef ROCKSDB_LITE
// Report new file to SstFileManagerImpl
Expand Down Expand Up @@ -1582,7 +1587,8 @@ Status CompactionJob::OpenCompactionOutputFile(
EventHelpers::LogAndNotifyTableFileCreationFinished(
event_logger_, cfd->ioptions()->listeners, dbname_, cfd->GetName(),
fname, job_id_, FileDescriptor(), kInvalidBlobFileNumber,
TableProperties(), TableFileCreationReason::kCompaction, s);
TableProperties(), TableFileCreationReason::kCompaction, s,
kUnknownFileChecksum, kUnknownFileChecksumFuncName);
return s;
}

Expand Down
9 changes: 7 additions & 2 deletions db/event_helpers.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,17 @@ void EventHelpers::LogAndNotifyTableFileCreationFinished(
const std::string& db_name, const std::string& cf_name,
const std::string& file_path, int job_id, const FileDescriptor& fd,
uint64_t oldest_blob_file_number, const TableProperties& table_properties,
TableFileCreationReason reason, const Status& s) {
TableFileCreationReason reason, const Status& s,
const std::string& file_checksum,
const std::string& file_checksum_func_name) {
if (s.ok() && event_logger) {
JSONWriter jwriter;
AppendCurrentTime(&jwriter);
jwriter << "cf_name" << cf_name << "job" << job_id << "event"
<< "table_file_creation"
<< "file_number" << fd.GetNumber() << "file_size"
<< fd.GetFileSize();
<< fd.GetFileSize() << "file_checksum" << file_checksum
<< "file_checksum_func_name" << file_checksum_func_name;

// table_properties
{
Expand Down Expand Up @@ -154,6 +157,8 @@ void EventHelpers::LogAndNotifyTableFileCreationFinished(
info.table_properties = table_properties;
info.reason = reason;
info.status = s;
info.file_checksum = file_checksum;
info.file_checksum_func_name = file_checksum_func_name;
for (auto& listener : listeners) {
listener->OnTableFileCreated(info);
}
Expand Down
4 changes: 3 additions & 1 deletion db/event_helpers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@ class EventHelpers {
const std::string& db_name, const std::string& cf_name,
const std::string& file_path, int job_id, const FileDescriptor& fd,
uint64_t oldest_blob_file_number, const TableProperties& table_properties,
TableFileCreationReason reason, const Status& s);
TableFileCreationReason reason, const Status& s,
const std::string& file_checksum,
const std::string& file_checksum_func_name);
static void LogAndNotifyTableFileDeletion(
EventLogger* event_logger, int job_id,
uint64_t file_number, const std::string& file_path,
Expand Down
4 changes: 4 additions & 0 deletions db/listener_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,8 @@ class TestFlushListener : public EventListener {
ASSERT_GT(info.table_properties.raw_value_size, 0U);
ASSERT_GT(info.table_properties.num_data_blocks, 0U);
ASSERT_GT(info.table_properties.num_entries, 0U);
ASSERT_EQ(info.file_checksum, kUnknownFileChecksum);
ASSERT_EQ(info.file_checksum_func_name, kUnknownFileChecksumFuncName);

#ifdef ROCKSDB_USING_THREAD_STATUS
// Verify the id of the current thread that created this table
Expand Down Expand Up @@ -751,6 +753,8 @@ class TableFileCreationListener : public EventListener {
ASSERT_GT(info.cf_name.size(), 0U);
ASSERT_GT(info.file_path.size(), 0U);
ASSERT_GT(info.job_id, 0);
ASSERT_EQ(info.file_checksum, kUnknownFileChecksum);
ASSERT_EQ(info.file_checksum_func_name, kUnknownFileChecksumFuncName);
if (info.status.ok()) {
ASSERT_GT(info.table_properties.data_size, 0U);
ASSERT_GT(info.table_properties.raw_key_size, 0U);
Expand Down
4 changes: 4 additions & 0 deletions include/rocksdb/listener.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ struct TableFileCreationInfo : public TableFileCreationBriefInfo {
TableProperties table_properties;
// The status indicating whether the creation was successful or not.
Status status;
// The checksum of the table file being created
std::string file_checksum;
// The checksum function name of checksum generator used for this table file
std::string file_checksum_func_name;
};

enum class CompactionReason : int {
Expand Down