Skip to content

Commit ad43a5f

Browse files
joshkang97meta-codesync[bot]
authored andcommitted
Pass file metadata to IngestExternalFile to improve ingestion latency (#14837)
Summary: Pull Request resolved: #14837 External file ingestion (`DB::IngestExternalFile[s]`) re-opens every SST file and scans it -- footer, properties, index, filter, and the first/last data blocks -- to recompute the boundary keys, sequence-number bounds, and table properties before committing. For cold, I/O-bound files this scan dominates ingest latency, even when the file is moved/linked rather than copied. This change lets a caller skip that work when it already has the file's metadata. `SstFileWriter::Finish()` now returns a `PreparedFileInfo` with the file size, table properties, and prepared internal `smallest`/`largest` bounds, and a new `IngestExternalFileArg::file_infos` field carries one `PreparedFileInfo` per file into `IngestExternalFiles()`. When set, ingestion reuses that metadata instead of re-opening and scanning each file. The file is still copied/linked, and the checksum is still verified when `verify_checksums_before_ingest` is set (the fast path opens the file only for that). Point-key and range-deletion bounds are folded into the same prepared bound pair, and user-defined-timestamp files (including the "UDT in Memtables only" format, whose boundary keys carry no timestamp) are handled. Internally, ingestion-job metadata acquisition was split into `GetIngestedFileInfoFromFileInfo` (reuse caller metadata) and `GetIngestedFileInfoFromFile` (open + scan). Prepared boundary updates use RocksDB comparators, and the timestamp-stripping path pads prepared bounds back to the internal timestamp shape before installing the file. The `ingestexternalfile` `db_bench` benchmark now also exposes `--ingest_external_file_fill_cache` so ingestion-read block-cache effects can be controlled independently while comparing the file-info fast path against the normal open-and-scan path. A future PR will support creating `PreparedFileInfo` from a existing DB-generated file, so this optimization is not just limited to SstFileWriter. ## Benchmark Results Existing `db_bench ingestexternalfile` benchmark, release build, using an XFS flash-backed filesystem. Files are linked (`move_files`) so the measurement isolates the ingest path rather than file-copy throughput. These numbers used direct reads and `fill_cache=false` to avoid warming the block cache while comparing the file-info fast path against the normal open-and-scan path. Each run used 1M keys/SST, one ingest call, and `db_bench` reported a 62.9 MB estimated file size. The comparison varied the number of files in that ingest call across 10, 30, and 50 files. | Files/batch | Config | Prepare P50 | Run P50 | Run P50/file | Total ingestion P50 | Total P50 drop | | --- | --- | --- | --- | --- | --- | --- | | 10 | Baseline | 10.909 ms | 7.593 ms | 0.759 ms/file | 18.502 ms | -- | | 10 | `file_info=true` | 1.127 ms | 7.541 ms | 0.754 ms/file | 8.668 ms | 53.2% | | 30 | Baseline | 29.049 ms | 23.161 ms | 0.772 ms/file | 52.210 ms | -- | | 30 | `file_info=true` | 2.734 ms | 23.384 ms | 0.779 ms/file | 26.118 ms | 50.0% | | 50 | Baseline | 49.036 ms | 39.224 ms | 0.784 ms/file | 88.260 ms | -- | | 50 | `file_info=true` | 4.133 ms | 39.417 ms | 0.788 ms/file | 43.550 ms | 50.7% | With `fill_cache=false` and direct reads, the metadata fast path cuts `Prepare()` by roughly 90-92%. `Run()` is roughly 0.75-0.79 ms per file across these runs. End-to-end `db_bench` micros/op is almost unchanged because this benchmark includes SST generation and compaction work; use `rocksdb.ingest.external.file.prepare.micros` and `rocksdb.ingest.external.file.run.micros` to isolate ingestion. Benchmark args: `--benchmarks=ingestexternalfile --use_existing_db=0 --num=1000000 --compression_type=none --statistics --use_direct_reads=true --ingest_external_file_batch_size=<10|30|50> --ingest_external_file_num_batches=1 --ingest_external_file_use_file_info=<false|true> --ingest_external_file_fill_cache=false`. Reviewed By: xingbowang Differential Revision: D107721261 fbshipit-source-id: b06ecb7b35f260ec02acf837e7986057bc23cdbf
1 parent f7677f0 commit ad43a5f

15 files changed

Lines changed: 922 additions & 157 deletions

db/db_impl/db_impl.cc

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6815,6 +6815,17 @@ Status DBImpl::PrepareFileIngestion(
68156815
"external_files[" + std::to_string(i) + "] is empty";
68166816
return Status::InvalidArgument(err_msg);
68176817
}
6818+
if (!args[i].file_infos.empty()) {
6819+
if (args[i].file_infos.size() != args[i].external_files.size()) {
6820+
return Status::InvalidArgument("file_infos[" + std::to_string(i) +
6821+
"] size must match external_files[" +
6822+
std::to_string(i) + "] size");
6823+
}
6824+
if (args[i].options.write_global_seqno) {
6825+
return Status::InvalidArgument(
6826+
"write_global_seqno is not supported when file_infos is set");
6827+
}
6828+
}
68186829
if (i && args[i].options.fill_cache != args[i - 1].options.fill_cache) {
68196830
return Status::InvalidArgument(
68206831
"fill_cache should be the same across ingestion options.");
@@ -6909,8 +6920,9 @@ Status DBImpl::PrepareFileIngestion(
69096920
this);
69106921
Status es = ingestion_jobs[i].Prepare(
69116922
args[i].external_files, args[i].files_checksums,
6912-
args[i].files_checksum_func_names, args[i].atomic_replace_range,
6913-
args[i].file_temperature, start_file_number, super_version);
6923+
args[i].files_checksum_func_names, args[i].file_infos,
6924+
args[i].atomic_replace_range, args[i].file_temperature,
6925+
start_file_number, super_version);
69146926
// capture first error only
69156927
if (!es.ok() && status.ok()) {
69166928
status = es;
@@ -6925,8 +6937,9 @@ Status DBImpl::PrepareFileIngestion(
69256937
this);
69266938
Status es = ingestion_jobs[0].Prepare(
69276939
args[0].external_files, args[0].files_checksums,
6928-
args[0].files_checksum_func_names, args[0].atomic_replace_range,
6929-
args[0].file_temperature, next_file_number, super_version);
6940+
args[0].files_checksum_func_names, args[0].file_infos,
6941+
args[0].atomic_replace_range, args[0].file_temperature,
6942+
next_file_number, super_version);
69306943
if (!es.ok()) {
69316944
status = es;
69326945
}

db/external_sst_file_ingestion_job.cc

Lines changed: 215 additions & 68 deletions
Large diffs are not rendered by default.

db/external_sst_file_ingestion_job.h

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,7 @@ class ExternalSstFileIngestionJob {
248248
Status Prepare(const std::vector<std::string>& external_files_paths,
249249
const std::vector<std::string>& files_checksums,
250250
const std::vector<std::string>& files_checksum_func_names,
251+
const std::vector<const PreparedFileInfo*>& file_infos,
251252
const std::optional<RangeOpt>& atomic_replace_range,
252253
const Temperature& file_temperature, uint64_t next_file_number,
253254
SuperVersion* sv);
@@ -323,17 +324,37 @@ class ExternalSstFileIngestionJob {
323324
// different options. For example: when external file does not contain
324325
// timestamps while column family enables UDT in Memtables only feature.
325326
Status SanityCheckTableProperties(const std::string& external_file,
326-
uint64_t new_file_number, SuperVersion* sv,
327-
IngestedFileInfo* file_to_ingest,
328-
std::unique_ptr<TableReader>* table_reader);
327+
const TableProperties& props,
328+
IngestedFileInfo* file_to_ingest);
329329

330330
// Open the external file and populate `file_to_ingest` with all the
331-
// external information we need to ingest this file.
331+
// external information we need to ingest this file. When
332+
// `prepared_file_info` is non-null, its caller-supplied metadata is reused
333+
// instead of opening and scanning the file.
332334
Status GetIngestedFileInfo(const std::string& external_file,
333335
uint64_t new_file_number,
336+
const PreparedFileInfo* prepared_file_info,
334337
IngestedFileInfo* file_to_ingest,
335338
SuperVersion* sv);
336339

340+
// Acquire the per-file metadata from the caller-supplied opaque
341+
// `PreparedFileInfo` (produced by SstFileWriter::Finish) instead of opening
342+
// the file.
343+
Status GetIngestedFileInfoFromFileInfo(
344+
const std::string& external_file,
345+
const PreparedFileInfo& prepared_file_info,
346+
IngestedFileInfo* file_to_ingest);
347+
348+
// Acquire the per-file metadata by opening the external file and scanning it
349+
// (table properties, sequence number bounds, and boundary keys including any
350+
// range-tombstone extensions). Used when no file_info is available. The
351+
// opened `TableReader` is returned via `*table_reader` so the caller can
352+
// reuse it (e.g. to verify the file checksum) without re-opening the file.
353+
Status GetIngestedFileInfoFromFile(
354+
const std::string& external_file, uint64_t new_file_number,
355+
IngestedFileInfo* file_to_ingest, SuperVersion* sv,
356+
std::unique_ptr<TableReader>* out_table_reader);
357+
337358
// If the input files' key range overlaps themselves, this function divides
338359
// them in the user specified order into multiple batches. Where the files
339360
// within a batch do not overlap with each other, but key range could overlap

0 commit comments

Comments
 (0)