Skip to content

Commit 47d839a

Browse files
zhichao-caoriversand963
authored andcommitted
Updated GenerateOneFileChecksum to use requested_checksum_func_name (facebook#7586)
Summary: CreateFileChecksumGenerator may uses requested_checksum_func_name in generator context to decide which generator will be used. GenerateOneFileChecksum has not being updated to use it, which will always get the generator when the name is empty. Fix it. Pull Request resolved: facebook#7586 Test Plan: make check Reviewed By: riversand963 Differential Revision: D24491989 Pulled By: zhichao-cao fbshipit-source-id: d9fdfdd431240f0a9a2e781ddbd48a7d6c609aad
1 parent 8b298e7 commit 47d839a

File tree

3 files changed

+39
-13
lines changed

3 files changed

+39
-13
lines changed

db/external_sst_file_ingestion_job.cc

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -192,10 +192,13 @@ Status ExternalSstFileIngestionJob::Prepare(
192192
// Step 1: generate the checksum for ingested sst file.
193193
if (need_generate_file_checksum_) {
194194
for (size_t i = 0; i < files_to_ingest_.size(); i++) {
195-
std::string generated_checksum, generated_checksum_func_name;
195+
std::string generated_checksum;
196+
std::string generated_checksum_func_name;
197+
std::string requested_checksum_func_name;
196198
IOStatus io_s = GenerateOneFileChecksum(
197199
fs_.get(), files_to_ingest_[i].internal_file_path,
198-
db_options_.file_checksum_gen_factory.get(), &generated_checksum,
200+
db_options_.file_checksum_gen_factory.get(),
201+
requested_checksum_func_name, &generated_checksum,
199202
&generated_checksum_func_name,
200203
ingestion_options_.verify_checksums_readahead_size,
201204
db_options_.allow_mmap_reads, io_tracer_);
@@ -830,11 +833,13 @@ IOStatus ExternalSstFileIngestionJob::GenerateChecksumForIngestedFile(
830833
// file checksum generated during Prepare(). This step will be skipped.
831834
return IOStatus::OK();
832835
}
833-
std::string file_checksum, file_checksum_func_name;
836+
std::string file_checksum;
837+
std::string file_checksum_func_name;
838+
std::string requested_checksum_func_name;
834839
IOStatus io_s = GenerateOneFileChecksum(
835840
fs_.get(), file_to_ingest->internal_file_path,
836-
db_options_.file_checksum_gen_factory.get(), &file_checksum,
837-
&file_checksum_func_name,
841+
db_options_.file_checksum_gen_factory.get(), requested_checksum_func_name,
842+
&file_checksum, &file_checksum_func_name,
838843
ingestion_options_.verify_checksums_readahead_size,
839844
db_options_.allow_mmap_reads, io_tracer_);
840845
if (!io_s.ok()) {

file/file_util.cc

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -123,22 +123,42 @@ bool IsWalDirSameAsDBPath(const ImmutableDBOptions* db_options) {
123123
return same;
124124
}
125125

126-
IOStatus GenerateOneFileChecksum(FileSystem* fs, const std::string& file_path,
127-
FileChecksumGenFactory* checksum_factory,
128-
std::string* file_checksum,
129-
std::string* file_checksum_func_name,
130-
size_t verify_checksums_readahead_size,
131-
bool allow_mmap_reads,
132-
std::shared_ptr<IOTracer>& io_tracer) {
126+
// requested_checksum_func_name brings the function name of the checksum
127+
// generator in checksum_factory. Checksum factories may use or ignore
128+
// requested_checksum_func_name.
129+
IOStatus GenerateOneFileChecksum(
130+
FileSystem* fs, const std::string& file_path,
131+
FileChecksumGenFactory* checksum_factory,
132+
const std::string& requested_checksum_func_name, std::string* file_checksum,
133+
std::string* file_checksum_func_name,
134+
size_t verify_checksums_readahead_size, bool allow_mmap_reads,
135+
std::shared_ptr<IOTracer>& io_tracer) {
133136
if (checksum_factory == nullptr) {
134137
return IOStatus::InvalidArgument("Checksum factory is invalid");
135138
}
136139
assert(file_checksum != nullptr);
137140
assert(file_checksum_func_name != nullptr);
138141

139142
FileChecksumGenContext gen_context;
143+
gen_context.requested_checksum_func_name = requested_checksum_func_name;
144+
gen_context.file_name = file_path;
140145
std::unique_ptr<FileChecksumGenerator> checksum_generator =
141146
checksum_factory->CreateFileChecksumGenerator(gen_context);
147+
if (checksum_generator == nullptr) {
148+
std::string msg =
149+
"Cannot get the file checksum generator based on the requested "
150+
"checksum function name: " +
151+
requested_checksum_func_name +
152+
" from checksum factory: " + checksum_factory->Name();
153+
return IOStatus::InvalidArgument(msg);
154+
}
155+
156+
// For backward compatable, requested_checksum_func_name can be empty.
157+
// If we give the requested checksum function name, we expect it is the
158+
// same name of the checksum generator.
159+
assert(!checksum_generator || requested_checksum_func_name.empty() ||
160+
requested_checksum_func_name == checksum_generator->Name());
161+
142162
uint64_t size;
143163
IOStatus io_s;
144164
std::unique_ptr<RandomAccessFileReader> reader;

file/file_util.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ extern bool IsWalDirSameAsDBPath(const ImmutableDBOptions* db_options);
3535

3636
extern IOStatus GenerateOneFileChecksum(
3737
FileSystem* fs, const std::string& file_path,
38-
FileChecksumGenFactory* checksum_factory, std::string* file_checksum,
38+
FileChecksumGenFactory* checksum_factory,
39+
const std::string& requested_checksum_func_name, std::string* file_checksum,
3940
std::string* file_checksum_func_name,
4041
size_t verify_checksums_readahead_size, bool allow_mmap_reads,
4142
std::shared_ptr<IOTracer>& io_tracer);

0 commit comments

Comments
 (0)