Fix check-sources.sh non-ASCII check and remove non-ASCII from sources (#14729)

Summary:
The non-ASCII character check in check-sources.sh used git grep -P (Perl regex), which requires git compiled with PCRE support. On systems without it, the command fails with exit code 128, which is != 1 (no match), so the check always reported a violation -- effectively dead.

Even in CI where git has PCRE2 support, the check was silently broken: git grep -P uses PCRE2 in UTF mode by default, which interprets [\x80-\xFF] as a Unicode codepoint range (U+0080 to U+00FF). Characters like em-dash (U+2014), arrows (U+2192), and math symbols (U+2248, etc.) fall outside that range and were not detected. Only Latin-1 Supplement characters (U+0080-U+00FF) would have been caught.

Replace with LC_ALL=C git grep using bash $'[\x80-\xff]' literal byte range, which works with basic regex in the C locale, and replace all non-ASCII characters in non-excluded source files:
- em-dash to --
- arrow to ->
- math symbols to ASCII equivalents (~=, <=, >=)
- box-drawing characters to ASCII art

Also exclude .github/ from the check, as scripts there can use non-ascii without disrupting RocksDB builds on non-UTF-8 systems.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/14729

Test Plan: manual / CI (make check-sources passes clean)

Reviewed By: hx235

Differential Revision: D104692574

Pulled By: pdillinger

fbshipit-source-id: 1d884c21056dcd83558b825a04b867f1c08e3f45
This commit is contained in:
Peter Dillinger
2026-05-11 17:02:22 -07:00
committed by meta-codesync[bot]
parent 330962bff6
commit 795f3bd61f
65 changed files with 475 additions and 469 deletions
+1 -1
View File
@@ -37,7 +37,7 @@ if [ "$?" != "1" ]; then
BAD=1
fi
git grep -n -P "[\x80-\xFF]" -- ':!docs' ':!*.md'
LC_ALL=C git grep -n $'[\x80-\xff]' -- ':!docs' ':!*.md' ':!.github'
if [ "$?" != "1" ]; then
echo '^^^^ Use only ASCII characters in source files'
BAD=1
+1 -1
View File
@@ -278,7 +278,7 @@ Status BlobWriteBatchTransformer::MergeCF(uint32_t column_family_id,
Status BlobWriteBatchTransformer::PutBlobIndexCF(uint32_t column_family_id,
const Slice& key,
const Slice& value) {
// Already a blob index pass through unchanged.
// Already a blob index -- pass through unchanged.
return WriteBatchInternal::PutBlobIndex(output_batch_, column_family_id, key,
value);
}
+1 -1
View File
@@ -35,7 +35,7 @@ struct BlobDirectWriteSettings {
CompressionType compression_type = kNoCompression;
// Compression options for newly written blob records.
CompressionOptions compression_opts;
// Raw pointer the Cache is owned by ColumnFamilyOptions and outlives all
// Raw pointer -- the Cache is owned by ColumnFamilyOptions and outlives all
// settings snapshots. Using raw avoids 2 atomic ref-count ops per Put().
Cache* blob_cache = nullptr;
// Blob-cache prepopulation policy for direct-write records.
+1 -1
View File
@@ -2138,7 +2138,7 @@ TEST_F(DBBlobIndexTest, EntityBlobFilterV3Remove) {
db_->PutEntity(WriteOptions(), db_->DefaultColumnFamily(), key, columns));
ASSERT_OK(Flush());
// Compact to trigger the filter entity should be removed
// Compact to trigger the filter -- entity should be removed
ASSERT_OK(db_->CompactRange(CompactRangeOptions(), nullptr, nullptr));
// Key should be gone
+1 -1
View File
@@ -75,7 +75,7 @@ class CompactionBlobResolver : public WideColumnBlobResolver {
bool track_resolve_error_ = false;
// Cache for resolved blob values to avoid re-fetching. Uses a vector of
// (column_index, PinnableSlice) pairs typical entities have few blob
// (column_index, PinnableSlice) pairs -- typical entities have few blob
// columns (<5), making linear scan cheaper than hash map overhead.
std::vector<std::pair<size_t, std::unique_ptr<PinnableSlice>>>
resolved_cache_;
+9 -9
View File
@@ -251,7 +251,7 @@ Compaction* FIFOCompactionPicker::PickSizeCompaction(
// When using blob-aware sizing, use proportional estimation (same
// principle as EstimateTotalDataForSST): each SST "owns"
// effective_size / num_files of total data. This is an approximation
// individual SSTs may reference different amounts of blob data,
// -- individual SSTs may reference different amounts of blob data,
// but uniform distribution is a reasonable estimate for FIFO dropping.
uint64_t remaining_size = effective_size;
const uint64_t num_files = last_level_files.size();
@@ -477,12 +477,12 @@ Compaction* FIFOCompactionPicker::PickIntraL0Compaction(
if (fifo_opts.max_data_files_size == 0) {
ROCKS_LOG_BUFFER(
log_buffer,
"[%s] FIFO kv-ratio compaction: skipping "
"[%s] FIFO kv-ratio compaction: skipping -- "
"max_data_files_size is 0, cannot compute target file size. ",
cf_name.c_str());
} else if (fifo_opts.max_data_files_size < fifo_opts.max_table_files_size) {
ROCKS_LOG_BUFFER(log_buffer,
"[%s] FIFO kv-ratio compaction: skipping "
"[%s] FIFO kv-ratio compaction: skipping -- "
"max_data_files_size (%" PRIu64
") < max_table_files_size "
"(%" PRIu64 ").",
@@ -547,7 +547,7 @@ Compaction* FIFOCompactionPicker::PickRatioBasedIntraL0Compaction(
for (int level = 1; level < vstorage->num_levels(); ++level) {
if (!vstorage->LevelFiles(level).empty()) {
ROCKS_LOG_BUFFER(log_buffer,
"[%s] FIFO kv-ratio compaction: skipping non-L0 "
"[%s] FIFO kv-ratio compaction: skipping -- non-L0 "
"level %d still has %" ROCKSDB_PRIszt
" files (migration in progress)",
cf_name.c_str(), level,
@@ -587,7 +587,7 @@ Compaction* FIFOCompactionPicker::PickRatioBasedIntraL0Compaction(
// once per flush or compaction completion, so no caching is needed.
uint64_t target = 0;
if (mutable_cf_options.max_compaction_bytes > 0) {
// User explicitly set max_compaction_bytes use it as target
// User explicitly set max_compaction_bytes -- use it as target
target = mutable_cf_options.max_compaction_bytes;
} else {
// Auto-calculate from capacity and observed SST/blob ratio
@@ -642,7 +642,7 @@ Compaction* FIFOCompactionPicker::PickRatioBasedIntraL0Compaction(
// of linear) write amplification.
// Build tier boundaries from smallest to largest.
// Stop at 10KB minimum SST files of most workloads are larger than
// Stop at 10KB minimum -- SST files of most workloads are larger than
// this, so lower boundaries would only waste CPU scanning L0 files.
// Files smaller than the lowest boundary simply merge at that boundary.
static constexpr uint64_t kMinTierBoundary = 10 * 1024; // 10KB
@@ -651,7 +651,7 @@ Compaction* FIFOCompactionPicker::PickRatioBasedIntraL0Compaction(
boundaries.push_back(b);
}
if (boundaries.empty()) {
// target itself is below kMinTierBoundary use target as the
// target itself is below kMinTierBoundary -- use target as the
// sole boundary so we can still compact at the target size.
boundaries.push_back(target);
}
@@ -668,7 +668,7 @@ Compaction* FIFOCompactionPicker::PickRatioBasedIntraL0Compaction(
continue;
}
// Found a file < boundary collect contiguous batch
// Found a file < boundary -- collect contiguous batch
std::vector<FileMetaData*> batch;
uint64_t accumulated = 0;
size_t pos = scan;
@@ -713,7 +713,7 @@ Compaction* FIFOCompactionPicker::PickRatioBasedIntraL0Compaction(
return c;
}
// This batch wasn't enough advance past it
// This batch wasn't enough -- advance past it
scan = pos;
}
}
+1 -1
View File
@@ -1101,7 +1101,7 @@ TEST_F(CompactionPickerTest, ReadTriggeredSkipsLastLevel) {
Add(0, 1U, "150", "200", kFileSize, 0, 500, 550);
Add(4, 3U, "301", "350", kFileSize, 0, 101, 150);
// File 3 is at the last non-empty level should NOT be marked for
// File 3 is at the last non-empty level -- should NOT be marked for
// read-triggered compaction. Bottommost file cleanup is handled
// separately by ComputeBottommostFilesMarkedForCompaction().
file_map_[3U].first->stats.num_collapsible_entry_reads_sampled.store(
+8 -8
View File
@@ -186,14 +186,14 @@ TEST_F(DBBasicTest,
}
// When the per-CF log_number actually advances, the per-CF skip must NOT
// fire the edit carries real information and must be emitted.
// fire -- the edit carries real information and must be emitted.
TEST_F(DBBasicTest, OptimizeManifestForRecoveryEmitsPerCFWhenLogAdvances) {
Options options = CurrentOptions();
options.create_if_missing = true;
options.optimize_manifest_for_recovery = true;
DestroyAndReopen(options);
ASSERT_OK(Put("k", "v"));
// Write data and close WITHOUT flushing the WAL has un-replayed
// Write data and close WITHOUT flushing -- the WAL has un-replayed
// records, so on reopen recovery flushes the memtable and the per-CF
// edit's log_number must advance past the prior WAL.
Close();
@@ -230,7 +230,7 @@ TEST_F(DBBasicTest, OptimizeManifestForRecoveryPreservesWalTracking) {
// best_efforts_recovery requires a fresh MANIFEST + CURRENT to be
// produced on every open (the salvage contract). Even with the option
// on, none of the SkippedNoopEdit branches must fire under
// best_efforts_recovery otherwise CURRENT can be left missing or
// best_efforts_recovery -- otherwise CURRENT can be left missing or
// stale (regression caught by DBBasicTest.RecoverWithNoCurrentFile and
// DBTest2.BestEffortsRecoveryWithSstUniqueIdVerification under an
// option-on default).
@@ -267,7 +267,7 @@ TEST_F(DBBasicTest, OptimizeManifestForRecoveryMultiCF) {
ASSERT_OK(Put(1, "k", "v1"));
ASSERT_OK(Put(2, "k", "v2"));
ASSERT_OK(Flush(0));
// CF 1 and 2 keep dirty memtables recovery must emit their per-CF
// CF 1 and 2 keep dirty memtables -- recovery must emit their per-CF
// edits (log_number advance from WAL replay).
Close();
@@ -662,7 +662,7 @@ TEST_F(DBBasicTest, ReuseManifestOnOpenStillRotatesOnSizeCap) {
ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->ClearAllCallBacks();
// The Flush after Reopen must have triggered a rotation given the
// tiny size cap proves the size-driven rotation path still runs
// tiny size cap -- proves the size-driven rotation path still runs
// when descriptor_log_ is bound by reuse.
EXPECT_GT(rotations.load(), 0);
EXPECT_EQ("v", Get("k"));
@@ -685,7 +685,7 @@ TEST_F(DBBasicTest, ReuseManifestOnOpenMultiCF) {
Close();
ReopenWithColumnFamilies({"default", "alpha", "beta"}, options);
// Trigger more writes post-reopen on each CF these get appended to
// Trigger more writes post-reopen on each CF -- these get appended to
// the reused MANIFEST.
ASSERT_OK(Put(0, "k2", "v0b"));
ASSERT_OK(Put(1, "k2", "v1b"));
@@ -777,7 +777,7 @@ TEST_F(DBBasicTest, WritableFileWriterInitialFileSizeAdoptsExistingSize) {
ASSERT_OK(raw->Close());
}
// Reopen and wrap in a WritableFileWriter without initial_file_size
// Reopen and wrap in a WritableFileWriter without initial_file_size --
// GetFileSize() should be 0 (the constructor's default).
std::unique_ptr<FSWritableFile> fs_file;
ASSERT_OK(env->GetFileSystem()->ReopenWritableFile(
@@ -805,7 +805,7 @@ TEST_F(DBBasicTest, WritableFileWriterInitialFileSizeAdoptsExistingSize) {
}
// Tail corruption: appending garbage bytes to the MANIFEST after a
// clean close must prevent reuse the physical size exceeds the
// clean close must prevent reuse -- the physical size exceeds the
// last valid record end.
TEST_F(DBBasicTest, ReuseManifestOnOpenSkipsOnTailCorruption) {
Options options = CurrentOptions();
+1 -1
View File
@@ -542,7 +542,7 @@ TEST_F(DBBlockCacheTest, WarmCacheWithDataBlocksDuringCompaction) {
EXPECT_TRUE(tracking_cache->HasPriority(Cache::Priority::BOTTOM));
EXPECT_FALSE(tracking_cache->HasPriority(Cache::Priority::LOW));
// Compaction output is in cache reads should have zero misses.
// Compaction output is in cache -- reads should have zero misses.
auto data_miss_before =
options.statistics->getTickerCount(BLOCK_CACHE_DATA_MISS);
ASSERT_EQ(value + "2", Get("key"));
+6 -6
View File
@@ -11833,7 +11833,7 @@ TEST_F(DBCompactionTest, RoundRobinCleanCutWithSharedBoundary) {
// 2. A post-verification step fails (injected here via sync point), setting
// compact_->status to error while each subcompaction's status stays OK.
// 3. SubcompactionState::Cleanup checks individual status (OK) and skips
// ReleaseObsolete the cache entries leak.
// ReleaseObsolete -- the cache entries leak.
// 4. FaultInjectionTestFS injects metadata read errors, causing GetChildren
// to fail in FindObsoleteFiles.
// 5. Close()'s FindObsoleteFiles also fails to find the orphan for the same
@@ -11864,7 +11864,7 @@ TEST_F(DBCompactionTest, LeakedTableCacheEntryOnCompactionFailure) {
// fail while individual subcompaction statuses stay OK (so Cleanup skips
// ReleaseObsolete). The filesystem deactivation makes GetChildren fail
// in FindObsoleteFiles, preventing the backstop from evicting the leaked
// cache entries matching the crash test's metadata read fault injection.
// cache entries -- matching the crash test's metadata read fault injection.
std::atomic<bool> inject_error{true};
ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->SetCallBack(
"CompactionJob::Run():AfterVerifyOutputFiles", [&](void* arg) {
@@ -11876,7 +11876,7 @@ TEST_F(DBCompactionTest, LeakedTableCacheEntryOnCompactionFailure) {
// Enable metadata read fault injection on the bg compaction thread after
// the compaction job finishes but before FindObsoleteFiles runs. This
// makes GetChildren fail (metadata read), matching crash test's
// --open_metadata_read_fault_one_in=8. Only metadata reads fail
// --open_metadata_read_fault_one_in=8. Only metadata reads fail --
// logging and other IO operations continue normally.
ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->SetCallBack(
"BackgroundCallCompaction:1", [&](void*) {
@@ -11889,7 +11889,7 @@ TEST_F(DBCompactionTest, LeakedTableCacheEntryOnCompactionFailure) {
ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->EnableProcessing();
// Trigger compaction fails after VerifyOutputFiles.
// Trigger compaction -- fails after VerifyOutputFiles.
Status s = dbfull()->TEST_CompactRange(0, nullptr, nullptr);
ASSERT_NOK(s);
@@ -11971,7 +11971,7 @@ TEST_F(DBCompactionTest, LeakedTableCacheEntryOnInstallFailure) {
ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->EnableProcessing();
// Trigger compaction Run() succeeds but Install() fails.
// Trigger compaction -- Run() succeeds but Install() fails.
Status s = dbfull()->TEST_CompactRange(0, nullptr, nullptr);
ASSERT_NOK(s);
@@ -12032,7 +12032,7 @@ TEST_F(DBCompactionTest, ObsoleteFileTableCacheEntryWithConcurrentRef) {
TableCache::Lookup(table_cache, target_file_number);
ASSERT_NE(concurrent_handle, nullptr);
// Call ReleaseObsolete directly this is what PurgeObsoleteFiles calls
// Call ReleaseObsolete directly -- this is what PurgeObsoleteFiles calls
// when a file becomes obsolete. Pass nullptr for the handle to make
// ReleaseObsolete do its own Lookup internally.
TableCache::ReleaseObsolete(table_cache, target_file_number,
+1 -1
View File
@@ -3883,7 +3883,7 @@ TEST_F(DBFlushTest, LeakedTableCacheEntryOnFlushInstallFailure) {
ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->EnableProcessing();
// Trigger flush BuildTable succeeds but LogAndApply fails.
// Trigger flush -- BuildTable succeeds but LogAndApply fails.
Status s = Flush();
ASSERT_NOK(s);
+1 -1
View File
@@ -7407,7 +7407,7 @@ Status DBImpl::GetCreationTimeOfOldestFile(uint64_t* creation_time) {
// For modern DBs, manifest carries file_creation_time and the
// first call returns the real value. We only need to wait for
// BGWorkAsyncFileOpen on legacy DBs whose manifest lacks
// file_creation_time those rely on the pinned reader, which is
// file_creation_time -- those rely on the pinned reader, which is
// null until async file open completes.
if (ctime == 0 && immutable_db_options_.open_files_async) {
std::call_once(waited_for_async_open, [&]() {
+1 -1
View File
@@ -1206,7 +1206,7 @@ Status DBImpl::MaybeUpdateNextFileNumber(RecoveryContext* recovery_ctx) {
normalized_fpath += kFilePathSeparator;
normalized_fpath.append(fname);
// Use >= so a crashed-mid-allocation file at exactly
// prev_next_file_number triggers the advance otherwise the next
// prev_next_file_number triggers the advance -- otherwise the next
// NewFileNumber() would collide with it.
if (number >= prev_next_file_number) {
on_disk_file_advanced_counter = true;
+1 -1
View File
@@ -1553,7 +1553,7 @@ Status DB::OpenAndCompact(
}
}
// 5. Open db As Secondary (skip WAL recovery remote compaction only
// 5. Open db As Secondary (skip WAL recovery -- remote compaction only
// needs LSM state from MANIFEST, not memtable data from WAL replay)
std::unique_ptr<DB> db;
std::vector<ColumnFamilyHandle*> handles;
+4 -4
View File
@@ -623,7 +623,7 @@ bool DBIter::FindNextUserEntryInternal(bool skipping_saved_key) {
PERF_COUNTER_ADD(internal_delete_skipped_count, 1);
MarkMemtableForFlushForPerOpTrigger(mem_hidden_op_scanned);
// Track contiguous tombstones for range conversion.
// Skip if outside seek prefix the top-of-loop check
// Skip if outside seek prefix -- the top-of-loop check
// flushed any pending run, but we must also avoid starting
// a new run outside the prefix.
if (min_tombstones_for_range_conversion_ > 0 &&
@@ -1097,7 +1097,7 @@ void DBIter::PrevInternal() {
if (min_tombstones_for_range_conversion_ > 0 &&
range_tomb_end_key_.Size() > 0 && timestamp_lb_ == nullptr) {
if (!valid_ && found_visible && PrefixCheck(saved_key_without_ts)) {
// Key was deleted and is within the seek prefix track it.
// Key was deleted and is within the seek prefix -- track it.
TrackContiguousTombstone(saved_key_.GetUserKey(),
/*always_update_first_key=*/true);
} else if (valid_) {
@@ -1141,7 +1141,7 @@ void DBIter::PrevInternal() {
// Sets ikey_ to the last visible entry's internal key. When found_visible
// is false, ikey_ is not updated and may contain stale data.
// Sets found_visible to true if at least one entry passed the IsVisible()
// check (seqno <= snapshot). When false, no entry was visible the key
// check (seqno <= snapshot). When false, no entry was visible -- the key
// does not exist at this snapshot and should not be treated as a tombstone.
// Returns false if an error occurred, and !status().ok() and !valid_.
//
@@ -1196,7 +1196,7 @@ bool DBIter::FindValueForCurrentKey(bool& found_visible) {
break;
}
// Entry survived the visibility check at least one visible version
// Entry survived the visibility check -- at least one visible version
// exists for this user key.
found_visible = true;
+1 -1
View File
@@ -589,7 +589,7 @@ class DBIter final : public Iterator {
// If enough contiguous tombstones have been tracked, insert a range
// tombstone [first_key, end_key) into the mutable memtable.
// end_key is the exclusive upper bound typically the next live key.
// end_key is the exclusive upper bound -- typically the next live key.
void MaybeInsertRangeTombstone(const Slice& end_key);
void ResetContiguousTombstoneTracking() {
contiguous_tombstone_count_ = 0;
+22 -22
View File
@@ -4151,7 +4151,7 @@ TEST_P(DBIteratorTest, PrefixSameAsStartSeekToNonInDomainKey) {
ASSERT_OK(Put("abc2", "v2"));
ASSERT_OK(Put("abc3", "v3"));
// Seek to "ab" (2 bytes) out-of-domain for FixedPrefixTransform(3).
// Seek to "ab" (2 bytes) -- out-of-domain for FixedPrefixTransform(3).
// ShouldSetPrefix returns false for out-of-domain targets, so no prefix
// constraint is set. The seek should find "abc1" and iteration should
// proceed without prefix_same_as_start enforcement.
@@ -5820,13 +5820,13 @@ TEST_P(ReadPathRangeTombstoneTest, ExhaustedIteratorWithBounds) {
// Both directions encounter two tombstone runs (a-d and g-j).
ASSERT_EQ(attempted_insert_ranges_.size(), 2);
if (Forward()) {
// Forward: sees a-d tombstones first live e terminates [a, e).
// Then g-j exhaustion past j, saved_key_="j" fallback [g, j),
// Forward: sees a-d tombstones first -> live e terminates -> [a, e).
// Then g-j -> exhaustion past j, saved_key_="j" fallback -> [g, j),
// covering g,h,i. j remains a point tombstone.
AssertRange(0, "a", "e");
AssertRange(1, "g", "j");
} else {
// Reverse: sees j-g tombstones first, then f,e live, then d-a exhausts.
// Reverse: sees j-g tombstones first, then f,e live, then d-a -> exhausts.
AssertRange(0, "g", "z");
AssertRange(1, "a", "e");
}
@@ -5863,8 +5863,8 @@ TEST_P(ReadPathRangeTombstoneTest, ExhaustedIteratorNoBounds) {
VerifyIteration({"e", "f"});
if (Forward()) {
// Forward: a-d run terminates at live e [a, e). g-j run exhausts
// past j saved_key_ fallback [g, j), covering g,h,i with j as a
// Forward: a-d run terminates at live e -> [a, e). g-j run exhausts
// past j -> saved_key_ fallback -> [g, j), covering g,h,i with j as a
// point tombstone.
ASSERT_EQ(attempted_insert_ranges_.size(), 2);
AssertRange(0, "a", "e");
@@ -6061,7 +6061,7 @@ TEST_P(ReadPathRangeTombstoneTest, PrefixFilterDefaultReadOptions) {
}
TEST_P(ReadPathRangeTombstoneTest, PrefixFilterTotalOrderSeek) {
// total_order_seek=true disables prefix filtering all files visible.
// total_order_seek=true disables prefix filtering -- all files visible.
// The delete run spans from prefix 'b' into prefix 'c', terminated by
// live key "cb" from L0. The tombstone [ba, cb) crosses prefix boundaries,
// which is safe because total_order_seek makes all files visible.
@@ -6103,7 +6103,7 @@ TEST_P(ReadPathRangeTombstoneTest, PrefixFilterTotalOrderSeek) {
ASSERT_OK(Flush());
// Memtable: contiguous deletes spanning prefixes 'b' and 'c'.
// No live key between "bb" and "cb" the run crosses prefix boundaries.
// No live key between "bb" and "cb" -- the run crosses prefix boundaries.
ASSERT_OK(put("aa", "below"));
ASSERT_OK(del("ba"));
ASSERT_OK(del("bb"));
@@ -6311,7 +6311,7 @@ TEST_P(ReadPathRangeTombstoneTest, PrefixFilterOutOfDomainSeek) {
ro.prefix_same_as_start = true;
auto it = std::unique_ptr<Iterator>(db_->NewIterator(ro));
if (Forward()) {
// Seek with a 1-byte key out-of-domain for FixedPrefixTransform(4).
// Seek with a 1-byte key -- out-of-domain for FixedPrefixTransform(4).
// prefix_same_as_start cannot set a prefix bound for this target.
it->Seek("b");
while (it->Valid()) {
@@ -6674,7 +6674,7 @@ TEST_P(ReadPathRangeTombstoneTest, ExhaustedWithUDT) {
ASSERT_EQ(attempted_insert_ranges_.size(), 1);
if (Forward()) {
// Forward exhaustion past h: saved_key_="h"+ts fallback [e+ts, h+ts).
// Forward exhaustion past h: saved_key_="h"+ts fallback -> [e+ts, h+ts).
AssertRange(0, std::string("e") + ts, std::string("h") + ts);
} else {
// Reverse exhaustion: range is [a+ts, e+ts).
@@ -6687,9 +6687,9 @@ TEST_P(ReadPathRangeTombstoneTest, ExhaustedWithUDT) {
TEST_P(ReadPathRangeTombstoneTest, SeekForPrevTombstone) {
// SeekForPrev lands directly on tombstones. No upper bound. Forward
// exhaustion past h falls back to saved_key_="h" [e, h), covering
// exhaustion past h falls back to saved_key_="h" -> [e, h), covering
// e,f,g with h as a point tombstone.
// Reverse: SeekForPrev("h") Delete(h,g,f,e) live "d" range [e, h).
// Reverse: SeekForPrev("h") -> Delete(h,g,f,e) -> live "d" -> range [e, h).
for (bool use_udt : {false, true}) {
SCOPED_TRACE(use_udt ? "with UDT" : "without UDT");
Options options = CurrentOptions();
@@ -6727,7 +6727,7 @@ TEST_P(ReadPathRangeTombstoneTest, SeekForPrevTombstone) {
keys.push_back(iter->key().ToString());
}
} else {
// SeekForPrev("h") lands on Delete(h), traverses g,f,e finds "d".
// SeekForPrev("h") lands on Delete(h), traverses g,f,e -> finds "d".
for (iter->SeekForPrev("h"); iter->Valid(); iter->Prev()) {
keys.push_back(iter->key().ToString());
}
@@ -6738,7 +6738,7 @@ TEST_P(ReadPathRangeTombstoneTest, SeekForPrevTombstone) {
ASSERT_EQ(attempted_insert_ranges_.size(), 1);
if (Forward()) {
// Forward exhausts past h saved_key_="h" fallback [e, h),
// Forward exhausts past h -> saved_key_="h" fallback -> [e, h),
// covering e,f,g with h as a point tombstone.
if (use_udt) {
AssertRange(0, std::string("e") + ts, std::string("h") + ts);
@@ -6761,11 +6761,11 @@ TEST_P(ReadPathRangeTombstoneTest, SeekForPrevTombstone) {
TEST_P(ReadPathRangeTombstoneTest, UpperBoundTombstone) {
// iterate_upper_bound lands past the data. Both directions see tombstones
// e-h. Forward exhausts naturally past h (no key in DB upper) so the
// e-h. Forward exhausts naturally past h (no key in DB >= upper) so the
// forward path falls back to saved_key_ ("h") as the exclusive end_key,
// covering n-1 of n deletes [e, h). The remaining tombstone for h
// covering n-1 of n deletes -- [e, h). The remaining tombstone for h
// stays as a point delete. Reverse captures the live key "d" before the
// run and uses it via range_tomb_end_key_ [e, i) when SeekToLast
// run and uses it via range_tomb_end_key_ -- [e, i) when SeekToLast
// delegates to SeekForPrev("i").
for (bool use_udt : {false, true}) {
SCOPED_TRACE(use_udt ? "with UDT" : "without UDT");
@@ -6837,8 +6837,8 @@ TEST_P(ReadPathRangeTombstoneTest, UpperBoundTombstone) {
TEST_P(ReadPathRangeTombstoneTest, LowerBoundTruncatesReverse) {
// Keys a-j, delete a-h. Lower bound "e" truncates reverse iteration
// mid-tombstone-run. Forward: tombstones e-h ended by live key i [e, i).
// Reverse: tombstones h,g,f,e then lower_bound hit flush [e, i).
// mid-tombstone-run. Forward: tombstones e-h ended by live key i -> [e, i).
// Reverse: tombstones h,g,f,e then lower_bound hit -> flush [e, i).
for (bool use_udt : {false, true}) {
SCOPED_TRACE(use_udt ? "with UDT" : "without UDT");
Options options = CurrentOptions();
@@ -6975,14 +6975,14 @@ TEST_P(ReadPathRangeTombstoneTest, InvisibleKeysDontBreakTombstoneRun) {
ASSERT_OK(Put("f", "vf"));
ASSERT_OK(Flush());
// Delete b, d visible tombstones.
// Delete b, d -- visible tombstones.
ASSERT_OK(Delete("b"));
ASSERT_OK(Delete("d"));
// Snapshot S. At S: a(live), b(del), d(del), f(live).
const Snapshot* snap = db_->GetSnapshot();
// Write c, e AFTER the snapshot invisible at S.
// Write c, e AFTER the snapshot -- invisible at S.
// At S the iterator sees: a(live), b(del), c(invis), d(del),
// e(invis), f(live).
ASSERT_OK(Put("c", "vc"));
@@ -6994,7 +6994,7 @@ TEST_P(ReadPathRangeTombstoneTest, InvisibleKeysDontBreakTombstoneRun) {
VerifyIteration({"a", "f"}, ro);
// Invisible keys c and e should not break the tombstone run.
// 2 tombstones (b, d) threshold range [b, f).
// 2 tombstones (b, d) >= threshold -> range [b, f).
ASSERT_EQ(attempted_insert_ranges_.size(), 1);
AssertRange(0, "b", "f");
+1 -1
View File
@@ -8214,7 +8214,7 @@ TEST_P(OpenFilesAsyncTest, GetCreationTimeOfOldestFileSkipsWaitForModernDB) {
Options options = CurrentOptions();
ASSERT_NO_FATAL_FAILURE(SetupData(options));
// If WaitForAsyncFileOpen is entered, the test fails modern DBs must
// If WaitForAsyncFileOpen is entered, the test fails -- modern DBs must
// never need the wait.
std::atomic<bool> waited{false};
ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->SetCallBack(
+3 -3
View File
@@ -107,7 +107,7 @@ TEST_F(DBTest2, ReadOnlyDBWalInDbPathInitialized) {
ASSERT_OK(Put("key2", "value2"));
Close();
// Reopen as read-only wal_in_db_path_ must be properly initialized.
// Reopen as read-only -- wal_in_db_path_ must be properly initialized.
// Before the fix, closing this DB would read an uninitialized bool in
// DeleteObsoleteFileImpl, which UBSan catches as undefined behavior.
std::unique_ptr<DB> db_ptr;
@@ -115,7 +115,7 @@ TEST_F(DBTest2, ReadOnlyDBWalInDbPathInitialized) {
std::string value;
ASSERT_OK(db_ptr->Get(ReadOptions(), "key1", &value));
ASSERT_EQ("value1", value);
// Close the read-only DB this triggers PurgeObsoleteFiles which reads
// Close the read-only DB -- this triggers PurgeObsoleteFiles which reads
// wal_in_db_path_. Under UBSan, an uninitialized bool here would fail.
db_ptr.reset();
@@ -8173,7 +8173,7 @@ TEST_F(DBTest2, FastSstOpenDisableAfterMetadataPersisted) {
ASSERT_EQ(1, test_fs->GetMetadataRetrievedCount());
db.reset();
// Reopen with fast_sst_open DISABLED metadata is in the MANIFEST
// Reopen with fast_sst_open DISABLED -- metadata is in the MANIFEST
// but should NOT be passed to the filesystem
options.fast_sst_open = false;
test_fs->ResetCounters();
+2 -2
View File
@@ -218,7 +218,7 @@ TEST_P(IterKeySwapTest, SwapAndDestroy) {
b_use_secondary] = GetParam();
std::string expected_a, expected_b;
// Storage for pinned keys must outlive the IterKeys.
// Storage for pinned keys -- must outlive the IterKeys.
std::string a_storage, b_storage;
IterKey a;
expected_a =
@@ -236,7 +236,7 @@ TEST_P(IterKeySwapTest, SwapAndDestroy) {
// After swap: a has b's old data, b has a's old data.
ASSERT_EQ(a.GetUserKey().ToString(), expected_b);
ASSERT_EQ(b.GetUserKey().ToString(), expected_a);
} // b destroyed here must not corrupt a's data
} // b destroyed here -- must not corrupt a's data
// a must still hold valid data after b's destruction.
ASSERT_EQ(a.GetUserKey().ToString(), expected_b);
+3 -3
View File
@@ -1696,7 +1696,7 @@ TEST_F(EventListenerTest, BackgroundJobPressure) {
Env::Priority::LOW);
sleeping_task.WaitUntilSleeping();
// Phase 1: No pressure 3 SST files (below slowdown trigger=4).
// Phase 1: No pressure -- 3 SST files (below slowdown trigger=4).
for (int i = 0; i < 3; i++) {
ASSERT_OK(Put("k" + std::to_string(i), std::string(100, 'x')));
ASSERT_OK(Flush());
@@ -1714,7 +1714,7 @@ TEST_F(EventListenerTest, BackgroundJobPressure) {
// MaybeScheduleFlushOrCompaction() runs before the pressure callback and
// may schedule new flush work, making flush counts non-deterministic.
// Phase 2: Build pressure flush past slowdown trigger (4 L0 SST files).
// Phase 2: Build pressure -- flush past slowdown trigger (4 L0 SST files).
// Compaction is blocked, so L0 SST files pile up.
listener->Reset();
{
@@ -1750,7 +1750,7 @@ TEST_F(EventListenerTest, BackgroundJobPressure) {
ASSERT_TRUE(found_compaction_scheduled);
ASSERT_TRUE(found_high_proximity);
// Phase 3: Relieve pressure unblock compaction, wait for completion.
// Phase 3: Relieve pressure -- unblock compaction, wait for completion.
listener->Reset();
sleeping_task.WakeUp();
sleeping_task.WaitUntilDone();
+1 -1
View File
@@ -320,7 +320,7 @@ class ReadOnlyMemTable {
virtual uint64_t ApproximateOldestKeyTime() const = 0;
// Inserts a range tombstone [start_key, end_key) that is logically redundant
// it is derived from existing point tombstones observed during iteration
// -- it is derived from existing point tombstones observed during iteration
// and does not delete any data that isn't already deleted. This is a
// best-effort optimization. It allows future reads to skip iterating over
// continuous single deletion tombstones.
+1 -1
View File
@@ -30,7 +30,7 @@ PinnedTableReader& PinnedTableReader::operator=(
TableReader* r = other.reader_.load(std::memory_order_acquire);
// Only read handle_ when reader_ is non-null. Pin() writes handle_ before
// reader_ (with release), so a non-null reader_ guarantees handle_ is stable.
// If reader_ is null, Pin() may be in progress avoid reading handle_.
// If reader_ is null, Pin() may be in progress -- avoid reading handle_.
handle_ = (r != nullptr) ? other.handle_ : nullptr;
reader_.store(r, std::memory_order_release);
return *this;
+6 -6
View File
@@ -4219,10 +4219,10 @@ void VersionStorageInfo::ComputeFilesMarkedForReadTriggeredCompaction(
return;
}
// Skip files at the last non-empty level there is no lower level to
// Skip files at the last non-empty level -- there is no lower level to
// compact into. Exception: L0 files are allowed even when L0 is the last
// non-empty level, because in single-level universal or FIFO compaction, L0
// files can be compacted together (L0 L0).
// files can be compacted together (L0 -> L0).
int last_non_empty = num_non_empty_levels_ - 1;
for (int level = 0; level < num_levels(); level++) {
@@ -5692,7 +5692,7 @@ Status VersionSet::Close(FSDirectory* db_dir, InstrumentedMutex* mu) {
content_manifest_name, fs_->OptimizeForManifestRead(file_options_),
&manifest_file, nullptr);
if (!content_io_s.ok()) {
// Surface I/O errors to the caller users who call DB::Close() and
// Surface I/O errors to the caller -- users who call DB::Close() and
// check the status should know about filesystem problems.
s = content_io_s;
ROCKS_LOG_ERROR(db_options_->info_log,
@@ -5744,7 +5744,7 @@ Status VersionSet::Close(FSDirectory* db_dir, InstrumentedMutex* mu) {
corrupt_io_s.PermitUncheckedError();
io_error_info.io_status.PermitUncheckedError();
if (content_check == 0) {
// First check failed rewrite and verify again
// First check failed -- rewrite and verify again
ROCKS_LOG_ERROR(db_options_->info_log,
"MANIFEST content verification on Close failed, "
"filename %s, rewriting manifest\n",
@@ -5755,7 +5755,7 @@ Status VersionSet::Close(FSDirectory* db_dir, InstrumentedMutex* mu) {
s = LogAndApply(cfd, ReadOptions(), WriteOptions(), &recovery_edit, mu,
db_dir);
} else {
// Rewritten manifest is also corrupt likely a recurring filesystem
// Rewritten manifest is also corrupt -- likely a recurring filesystem
// issue. Surface it so DB::Close() callers can detect the problem.
ROCKS_LOG_ERROR(db_options_->info_log,
"MANIFEST content verification on Close failed again "
@@ -6717,7 +6717,7 @@ Status VersionSet::ReopenManifestForAppend(const std::string& manifest_path) {
FileOptions opt_file_opts = GetFileOptionsForManifestWrite();
// Bail if the on-disk size diverges from what Recover's Reader
// consumed likely a torn tail from a prior crash; mid-block append
// consumed -- likely a torn tail from a prior crash; mid-block append
// would mis-frame the record stream.
uint64_t physical_size = 0;
IOStatus stat_s = fs_->GetFileSize(manifest_path, IOOptions(), &physical_size,
+1 -1
View File
@@ -1726,7 +1726,7 @@ class VersionSet {
// MANIFEST as before) if ReopenWritableFile fails.
Status ReopenManifestForAppend(const std::string& manifest_path);
// FileOptions for MANIFEST writes applies the FS's
// FileOptions for MANIFEST writes -- applies the FS's
// OptimizeForManifestWrite tuning, then re-applies the user-configured
// temperature so a custom FS can't override it.
FileOptions GetFileOptionsForManifestWrite() const;
+5 -5
View File
@@ -2440,7 +2440,7 @@ TEST_F(VersionSetTest, ManifestContentValidationOnCloseClean) {
// Verify content validation actually ran
ASSERT_TRUE(content_validation_ran);
// No corruption counter should be zero
// No corruption -- counter should be zero
ASSERT_EQ(0, stats->getTickerCount(MANIFEST_VALIDATION_FAILURE_COUNT));
// Manifest path should be unchanged (no rotation)
@@ -2513,7 +2513,7 @@ TEST_F(VersionSetTest, ManifestContentValidationOnCloseCorruptRecord) {
TEST_F(VersionSetTest, ManifestContentValidationOnCloseDisabled) {
// Default (option disabled), corrupt manifest after writer close,
// verify no rotation occurred corrupt manifest persists.
// verify no rotation occurred -- corrupt manifest persists.
NewDB();
auto stats = CreateDBStatistics();
imm_db_options_.statistics = stats;
@@ -2538,7 +2538,7 @@ TEST_F(VersionSetTest, ManifestContentValidationOnCloseDisabled) {
ASSERT_FALSE(content_validation_ran);
// Validation disabled counter should be zero
// Validation disabled -- counter should be zero
ASSERT_EQ(0, stats->getTickerCount(MANIFEST_VALIDATION_FAILURE_COUNT));
// Manifest path should be unchanged (no rotation since validation is off)
@@ -2585,7 +2585,7 @@ TEST_F(VersionSetTest, ManifestContentValidationOnCloseSizeCheckFails) {
SyncPoint::GetInstance()->DisableProcessing();
// Size check caught the issue; content validation on rewritten manifest
// should pass no content validation failure recorded
// should pass -- no content validation failure recorded
ASSERT_EQ(0, stats->getTickerCount(MANIFEST_VALIDATION_FAILURE_COUNT));
// Size check should have triggered rotation
@@ -2697,7 +2697,7 @@ TEST_F(VersionSetTest, ManifestContentValidationOnCloseOpenFails) {
ASSERT_TRUE(close_s.IsIOError()) << close_s.ToString();
// File couldn't be opened no content validation ran
// File couldn't be opened -- no content validation ran
ASSERT_EQ(0, stats->getTickerCount(MANIFEST_VALIDATION_FAILURE_COUNT));
}
+2 -2
View File
@@ -2776,7 +2776,7 @@ TEST_F(DBWideBasicTest, EntityBlobBlockCacheTierGet) {
// Reopen to clear any caches
Reopen(options);
// Read with kBlockCacheTier blob is not in cache
// Read with kBlockCacheTier -- blob is not in cache
ReadOptions read_opts;
read_opts.read_tier = kBlockCacheTier;
PinnableSlice result;
@@ -2998,7 +2998,7 @@ TEST_F(DBWideBasicTest, MergeEntityWithBlobColumnsBlockCacheTier) {
// Reopen to clear caches
Reopen(options);
// Read with kBlockCacheTier blob not in cache, merge needs blob resolution
// Read with kBlockCacheTier -- blob not in cache, merge needs blob resolution
ReadOptions read_opts;
read_opts.read_tier = kBlockCacheTier;
PinnableSlice result;
+1 -1
View File
@@ -59,7 +59,7 @@ Status ReadPathBlobResolver::ResolveColumn(size_t column_index,
blob_resolver_util::FindBlobColumn(blob_columns_, column_index);
if (blob_index_ptr == nullptr) {
// Inline column return the value directly
// Inline column -- return the value directly
*resolved_value = (*columns_)[column_index].value();
} else {
// Check if already resolved
+2 -2
View File
@@ -391,7 +391,7 @@ static BlobIndex MakeBlobIndex(uint64_t file_number, uint64_t offset,
return bi;
}
// Helper: V2 serialize DeserializeV2 round-trip, returning
// Helper: V2 serialize -> DeserializeV2 round-trip, returning
// deserialized columns and blob column info.
static void V2SerializeAndDeserialize(
const std::vector<std::pair<std::string, std::string>>& columns,
@@ -798,7 +798,7 @@ TEST_F(WideColumnSerializationTest, RandomizedSerializeDeserializeRoundTrip) {
}
}
// V2 serialize DeserializeV2 round-trip
// V2 serialize -> DeserializeV2 round-trip
std::string serialized;
std::vector<WideColumn> deserialized;
std::vector<std::pair<size_t, BlobIndex>> blob_out;
+2 -2
View File
@@ -197,10 +197,10 @@ class DbStressFSWrapper : public FileSystemWrapper {
assert(!file_opts.file_checksum_func_name.empty());
if (file_opts.file_checksum_func_name == kUnknownFileChecksumFuncName ||
file_opts.file_checksum_func_name == kNoFileChecksumFuncName) {
// No checksum available checksum value must be empty
// No checksum available -- checksum value must be empty
assert(file_opts.file_checksum.empty());
} else {
// A real checksum function checksum value must be present
// A real checksum function -- checksum value must be present
assert(!file_opts.file_checksum.empty());
}
}
+1 -1
View File
@@ -2900,7 +2900,7 @@ class NonBatchedOpsStressTest : public StressTest {
op_logs += "N";
}
// backward scan skip when backward iteration is not supported
// backward scan -- skip when backward iteration is not supported
if (FLAGS_test_backward_scan) {
key_str = Key(ub - 1);
iter->SeekForPrev(key_str);
+2 -2
View File
@@ -1892,7 +1892,7 @@ TEST_F(EnvPosixTest, SupportedOpsNoAsyncIOOnIOUringInitFailure) {
int64_t thread_supported_ops2 = 0;
std::thread t([&]() {
fs->SupportedOps(thread_supported_ops);
// Second call on the same thread cached failure, no retry.
// Second call on the same thread -- cached failure, no retry.
fs->SupportedOps(thread_supported_ops2);
});
t.join();
@@ -1903,7 +1903,7 @@ TEST_F(EnvPosixTest, SupportedOpsNoAsyncIOOnIOUringInitFailure) {
// Second call should also lack kAsyncIO.
ASSERT_EQ(thread_supported_ops2 & (1 << FSSupportedOps::kAsyncIO), 0);
ASSERT_NE(thread_supported_ops2 & (1 << FSSupportedOps::kFSPrefetch), 0);
// CreateIOUring must have been called exactly once not retried.
// CreateIOUring must have been called exactly once -- not retried.
ASSERT_EQ(create_count, 1);
SyncPoint::GetInstance()->DisableProcessing();
+1 -1
View File
@@ -221,7 +221,7 @@ IOStatus GenerateOneFileChecksum(
std::unique_ptr<FSRandomAccessFile> r_file;
FileOptions fopts = file_options;
if (fopts.file_checksum.empty()) {
// No expected checksum is known this is a from-scratch computation.
// No expected checksum is known -- this is a from-scratch computation.
fopts.file_checksum_func_name = kNoFileChecksumFuncName;
}
io_s = fs->NewRandomAccessFile(file_path, fopts, &r_file, nullptr);
+1 -1
View File
@@ -3899,7 +3899,7 @@ TEST_P(FSBufferPrefetchTest, FSBufferPrefetchStatsInternals) {
&result, &s, for_compaction);
// Platforms that don't have IO uring may not support async IO.
// With the ReadAsync sync fallback, s will be OK even when async IO is
// unavailable detect by checking if the second buffer has an async read
// unavailable -- detect by checking if the second buffer has an async read
// in progress.
if (use_async_prefetch && s.IsNotSupported()) {
return;
+6 -6
View File
@@ -946,7 +946,7 @@ struct AdvancedColumnFamilyOptions {
// (estimated_reads / file_size) exceeds this threshold. This helps reduce
// read amplification for hot keys by compacting frequently-read files.
//
// Only "collapsible" reads are counted lookups that return NotFound
// Only "collapsible" reads are counted -- lookups that return NotFound
// (bloom filter false positive), Delete/SingleDeletion (tombstone), or
// Merge (partial result). These are reads where the file contributed no
// final value and compaction would eliminate the wasted work.
@@ -972,15 +972,15 @@ struct AdvancedColumnFamilyOptions {
// r * S * B = 2 * (1 + F) * S
// r = 2 * (1 + F) / B
//
// With F = 10, B = 4096: r = 22 / 4096 0.005.
// With F = 10, B = 4096: r = 22 / 4096 ~= 0.005.
//
// With a block-cache hit rate h (0 h < 1), each collapsible read
// With a block-cache hit rate h (0 <= h < 1), each collapsible read
// only costs (1 - h) * B bytes of actual disk IO, so:
// r = 2 * (1 + F) / ((1 - h) * B)
//
// h = 0 r 0.005
// h = 0.5 r 0.01
// h = 0.9 r 0.05
// h = 0 -> r ~= 0.005
// h = 0.5 -> r ~= 0.01
// h = 0.9 -> r ~= 0.05
//
// A recommended starting point is 0.01, which avoids triggering
// compactions that cost more IO than they save for most cache-friendly
+1 -1
View File
@@ -420,7 +420,7 @@ class CompactionFilter : public Customizable {
// Returns true if the filter overrides FilterV4 and can handle
// WideColumnBlobResolver (lazy blob loading). When false (default),
// FilterV4 delegates to FilterV3 with fully resolved column values
// FilterV4 delegates to FilterV3 with fully resolved column values --
// blob columns are eagerly fetched before the filter is called, ensuring
// backward compatibility with FilterV3-only filters.
//
+1 -1
View File
@@ -556,7 +556,7 @@ struct IOErrorInfo {
uint64_t offset;
};
// EXPERIMENTAL under active development, fields may change.
// EXPERIMENTAL -- under active development, fields may change.
// Point-in-time snapshot of background job pressure for one DB: how busy
// compaction and flush are, and how close the DB is to write-stalling.
struct BackgroundJobPressure {
+1 -1
View File
@@ -2952,7 +2952,7 @@ struct SizeApproximationOptions {
// blob file data in the key range. When enabled, the total blob file size
// is prorated by the ratio of SST data in the range to the total SST data:
//
// blob_size_in_range total_blob_size * (sst_in_range / total_sst)
// blob_size_in_range ~= total_blob_size * (sst_in_range / total_sst)
//
// Limitations of this approximation:
// - Assumes blob data is distributed proportionally to SST data, which
+2 -2
View File
@@ -43,7 +43,7 @@ class UserDefinedIndexBuilder {
kOther = 3, // Other types (e.g., blob reference, wide-column entity).
// The value format is type-specific and may not be the
// actual user data.
kTypeMax, // Sentinel must be last. Value may change across releases.
kTypeMax, // Sentinel -- must be last. Value may change across releases.
};
// File offset and size of the data block
@@ -106,7 +106,7 @@ class UserDefinedIndexBuilder {
// (e.g., trie-based indexes) can leave this as a no-op.
//
// @key: The user key (without sequence number or type suffix).
// @type: The entry type kValue (Put), kDelete, kMerge, or kOther.
// @type: The entry type -- kValue (Put), kDelete, kMerge, or kOther.
// For kDelete entries, the value may be empty. For kOther, the
// value format is type-specific and may not be actual user data.
// @value: The associated value (may be empty for deletions).
+3 -3
View File
@@ -559,7 +559,7 @@ TEST_F(InlineSkipTest, MultiGetDuplicateKeysWithCallbackWalk) {
}
// Callback that walks forward through multiple entries before stopping.
// This simulates the Merge operand accumulation in SaveValue the callback
// This simulates the Merge operand accumulation in SaveValue -- the callback
// returns true for entries until it reaches one >= stop_at, simulating
// walking through merge chain entries.
struct WalkingCallbackArg {
@@ -1068,11 +1068,11 @@ static void ConcurrentMultiGetWorker(void* arg) {
// Validate all results: each found result must equal its query key
// (since all queried keys were inserted, an exact match is expected).
// However, due to concurrent inserts, a key sampled from the ring
// might not yet be visible so we only check that if a result is
// might not yet be visible -- so we only check that if a result is
// found, it equals the query key (i.e., no wrong key returned).
for (size_t j = 0; j < batch_size; j++) {
if (results[j] != 0 && results[j] != query_keys[j]) {
// Got a result that doesn't match the query either a bug or
// Got a result that doesn't match the query -- either a bug or
// the exact key wasn't inserted and we got the next one. Since
// all our query keys are inserted, this means the result should
// be >= query. For the just-inserted key we already checked
+2 -2
View File
@@ -434,14 +434,14 @@ void InstallStackTraceHandler() {
// Ignore SIGPIPE so that broken-pipe writes (e.g. to a closed stdout)
// return EPIPE instead of killing the process.
signal(SIGPIPE, SIG_IGN);
// Crash signals invoke full stack trace + ring buffer
// Crash signals -- invoke full stack trace + ring buffer
signal(SIGILL, StackTraceHandler);
signal(SIGSEGV, StackTraceHandler);
signal(SIGBUS, StackTraceHandler);
signal(SIGABRT, StackTraceHandler);
signal(SIGFPE, StackTraceHandler);
signal(SIGQUIT, StackTraceHandler);
// Termination signals print ring buffer only, no stack trace
// Termination signals -- print ring buffer only, no stack trace
signal(SIGTERM, TerminationHandler);
signal(SIGINT, TerminationHandler);
atexit(AtExit);
@@ -41,7 +41,7 @@ void BlockBasedTableIterator::SeekImpl(const Slice* target,
return;
}
// MultiScan requires an explicit seek key SeekToFirst() is not supported
// MultiScan requires an explicit seek key -- SeekToFirst() is not supported
if (multi_scan_read_set_ && !target) {
multi_scan_status_ = Status::InvalidArgument("No seek key for MultiScan");
RecordTick(table_->GetStatistics(), MULTISCAN_SEEK_ERRORS);
@@ -683,7 +683,7 @@ void BlockBasedTableIterator::FindBlockForward() {
if (multi_scan_index_iter_ &&
multi_scan_index_iter_->IsScanRangeExhausted()) {
if (multi_scan_index_iter_->HasMoreScanRanges()) {
// More ranges remain signal out-of-bound so DBIter/LevelIter
// More ranges remain -- signal out-of-bound so DBIter/LevelIter
// will trigger the next Seek for the next scan range.
is_out_of_bound_ = true;
}
+1 -1
View File
@@ -52,7 +52,7 @@ struct DataBlockFooter {
// limit is adequate because a 4GiB block (maximum due to 32-bit block size)
// with restart_interval=1 and minimum entries (12 bytes: 3 varint bytes +
// 9-byte internal key + empty value) plus 4-byte restart offsets = 16 bytes
// per restart, fits at most (2^32 - 4) / 16 268 million restarts.
// per restart, fits at most (2^32 - 4) / 16 ~= 268 million restarts.
static constexpr uint32_t kMaxNumRestarts = (1u << 28) - 1;
// Maximum encoded length of a DataBlockFooter (for buffer sizing).
+10 -9
View File
@@ -65,7 +65,7 @@ void MultiScanIndexIterator::Seek(const Slice& target) {
// Enforce forward-only seek
if (!prev_seek_key_.empty() && icomp_.Compare(target, prev_seek_key_) <= 0) {
// Seek key is not moving forward keep current position
// Seek key is not moving forward -- keep current position
return;
}
prev_seek_key_ = target.ToString();
@@ -87,11 +87,12 @@ void MultiScanIndexIterator::Seek(const Slice& target) {
// The following diagram explains different seek targets seeking at various
// positions on the table, while the next_scan_idx_ points to PreparedRange 2.
//
// next_scan_idx_: ------------------
//
// next_scan_idx_: ------------------+
// |
// v
// table: : __[PreparedRange 1]__[PreparedRange 2]__[PreparedRange 3]__
// Seek target: <----- Case 1 ------><------------- Case 2 -------------->
//
// Seek target: <----- Case 1 ------>^<------------- Case 2 -------------->
// |
// Case 3
//
// Case 1: seek before the start of next prepared range. This could happen
@@ -103,7 +104,7 @@ void MultiScanIndexIterator::Seek(const Slice& target) {
if (compare_next_scan_start_result < 0) {
// Case 1: Seek before the start of the next prepared range
if (next_scan_idx_ == 0) {
// Should not happen seek before first prepared range
// Should not happen -- seek before first prepared range
assert(false && "Seek target before the first prepared range");
valid_ = false;
return;
@@ -122,7 +123,7 @@ void MultiScanIndexIterator::Seek(const Slice& target) {
valid_ = false;
return;
}
// Seek within a gap advance to the right scan range and find block
// Seek within a gap -- advance to the right scan range and find block
SeekToBlock(&user_seek_target);
} else if (compare_next_scan_start_result > 0) {
// Case 2: Seek after the start of the next prepared range
@@ -220,7 +221,7 @@ void MultiScanIndexIterator::SeekToBlockIdx(size_t block_idx) {
void MultiScanIndexIterator::SetExhausted() {
scan_range_exhausted_ = true;
if (next_scan_idx_ < block_index_ranges_per_scan_.size()) {
// More ranges remain signal out-of-bound for current range.
// More ranges remain -- signal out-of-bound for current range.
valid_ = true;
// Position at the start of the next range so that the next Seek()
// can find it. We need to be "valid" so that FindBlockForward sets
@@ -232,7 +233,7 @@ void MultiScanIndexIterator::SetExhausted() {
}
valid_ = false;
} else {
// Last range natural EOF. Don't set out-of-bound so LevelIterator
// Last range -- natural EOF. Don't set out-of-bound so LevelIterator
// advances to the next file.
valid_ = false;
}
@@ -58,7 +58,7 @@ class MultiScanIndexIterator : public InternalIteratorBase<IndexValue> {
// Move to the first block of the first scan range.
void SeekToFirst() override;
// Not supported sets valid_ = false.
// Not supported -- sets valid_ = false.
void SeekForPrev(const Slice& target) override;
void SeekToLast() override;
void Prev() override;
+3 -3
View File
@@ -8633,7 +8633,7 @@ TEST_P(UserDefinedIndexTest, ValueTypeMappingViaDBFlush) {
// ValueTypes by writing various operation types via the DB API, flushing,
// and inspecting what the TestUserDefinedIndexBuilder received.
if (is_reverse_comparator_) {
// Skip for reverse comparator the key ordering makes this test
// Skip for reverse comparator -- the key ordering makes this test
// unnecessarily complex and the mapping logic is comparator-independent.
ROCKSDB_GTEST_BYPASS("Skipped for reverse comparator");
return;
@@ -8735,7 +8735,7 @@ TEST_P(UserDefinedIndexTest, CompactionWithSnapshotsAndUDI) {
ASSERT_OK(db->Delete(WriteOptions(), "key_bb"));
ASSERT_OK(db->Flush(FlushOptions()));
// Compact L0 L1. With the snapshot held, both versions of key_aa
// Compact L0 -> L1. With the snapshot held, both versions of key_aa
// and the delete tombstone for key_bb must be preserved in the compaction
// output. The UDI builder receives multiple entries for key_aa.
ASSERT_OK(db->CompactRange(CompactRangeOptions(), nullptr, nullptr));
@@ -8747,7 +8747,7 @@ TEST_P(UserDefinedIndexTest, CompactionWithSnapshotsAndUDI) {
const auto& log = user_defined_index_factory->key_type_log_;
ASSERT_FALSE(log.empty());
// Count total occurrences of key_aa across all builders at least 4:
// Count total occurrences of key_aa across all builders -- at least 4:
// flush1 (v1) + flush2 (v2) + compaction (v2, v1).
int key_aa_count = 0;
int key_bb_count = 0;
+1 -1
View File
@@ -1936,7 +1936,7 @@ def execute_cmd(cmd, timeout=None, timeout_pstack=False):
hit_timeout = True
if timeout_pstack:
os.system("pstack %d" % pid)
child.terminate() # SIGTERM triggers TerminationHandler
child.terminate() # SIGTERM -- triggers TerminationHandler
try:
outs, errs = child.communicate(timeout=3)
print("TERMINATED %d\n" % child.pid)
+5 -5
View File
@@ -205,7 +205,7 @@ def collect_changed_lines(repo_root, diff_base_arg=None):
)
merge_into(all_changed, parse_diff_for_changed_lines(diff_staged))
# Untracked files treat every line as changed
# Untracked files -- treat every line as changed
untracked_out, _ = run_cmd(
["git", "ls-files", "--others", "--exclude-standard"], cwd=repo_root
)
@@ -510,7 +510,7 @@ def main():
jobs = args.jobs or os.cpu_count() or 4
# ------------------------------------------------------------------
# Step 1 detect changes
# Step 1 -- detect changes
# ------------------------------------------------------------------
log("=" * 70)
log("Step 1: Detecting changes")
@@ -529,7 +529,7 @@ def main():
log(f" {f} ({len(changed_lines[f])} lines)")
# ------------------------------------------------------------------
# Step 2 select compilable files present in compile_commands.json
# Step 2 -- select compilable files present in compile_commands.json
# ------------------------------------------------------------------
db_files = load_compile_db(compile_db_path, repo_root)
cc_changed = sorted(
@@ -549,7 +549,7 @@ def main():
log("=" * 70)
# ------------------------------------------------------------------
# Step 3 run clang-tidy in parallel via ThreadPoolExecutor
# Step 3 -- run clang-tidy in parallel via ThreadPoolExecutor
# ------------------------------------------------------------------
all_raw_output = []
all_filtered = []
@@ -603,7 +603,7 @@ def main():
log(f"\nFull clang-tidy output saved to {args.output}")
# ------------------------------------------------------------------
# Step 4 report filtered results
# Step 4 -- report filtered results
# ------------------------------------------------------------------
log(f"\n{'=' * 70}")
log(f"Step 3: Results (wall time {wall_time:.1f}s)")
+2 -2
View File
@@ -16,7 +16,7 @@
namespace ROCKSDB_NAMESPACE {
// Wraps a value of type T and tracks whether it has been mutated since the
// last Reset(). Reset() short-circuits when the value is not dirty useful
// last Reset(). Reset() short-circuits when the value is not dirty -- useful
// when T::Reset() is non-trivial (allocations, container clears) and the
// value is rarely populated on a hot path.
//
@@ -41,7 +41,7 @@ class DirtyTracked {
const T* operator->() const { return &value_; }
const T& operator*() const { return value_; }
// Explicit mutating access marks dirty, returns a mutable pointer.
// Explicit mutating access -- marks dirty, returns a mutable pointer.
T* mut() {
dirty_ = true;
return &value_;
+1 -1
View File
@@ -216,7 +216,7 @@ Status ReadSet::ReadIndex(size_t block_index, CachableEntry<Block>* out) {
// Case 3: Block needs synchronous read (pending or never-dispatched blocks).
// No ReleaseMemory() needed here because blocks reaching this path never had
// TryAcquireMemory() called they were either pending prefetch or skipped
// TryAcquireMemory() called -- they were either pending prefetch or skipped
// during SubmitJob. block_sizes_[block_index] may be > 0 (set during
// SubmitJob for all uncached blocks) but that does not imply memory was
// acquired.
+4 -4
View File
@@ -1830,7 +1830,7 @@ TEST_F(IODispatcherTest, MemoryReleasedAfterReadIndexThenReleaseBlock) {
// Some memory should have been granted for prefetch
ASSERT_GT(stats->getTickerCount(PREFETCH_MEMORY_BYTES_GRANTED), 0);
// Read all blocks ReadIndex moves values out of pinned_blocks_.
// Read all blocks -- ReadIndex moves values out of pinned_blocks_.
// This also triggers TryDispatchPendingPrefetches as memory is released,
// which acquires more memory for pending groups. So granted grows during
// this loop.
@@ -1840,7 +1840,7 @@ TEST_F(IODispatcherTest, MemoryReleasedAfterReadIndexThenReleaseBlock) {
ASSERT_NE(block.GetValue(), nullptr);
}
// Release all blocks should be a no-op for memory accounting since
// Release all blocks -- should be a no-op for memory accounting since
// ReadIndex already released memory when moving values out
for (size_t i = 0; i < block_handles.size(); ++i) {
read_set->ReleaseBlock(i);
@@ -1894,12 +1894,12 @@ TEST_F(IODispatcherTest, DestructorReleasesMemoryAfterReadIndex) {
ASSERT_GT(granted, 0);
// Read all blocks via ReadIndex (moves values out of pinned_blocks_)
// but do NOT call ReleaseBlock let the destructor handle cleanup
// but do NOT call ReleaseBlock -- let the destructor handle cleanup
for (size_t i = 0; i < block_handles.size(); ++i) {
CachableEntry<Block> block;
ASSERT_OK(read_set->ReadIndex(i, &block));
}
// read_set goes out of scope destructor should release all memory
// read_set goes out of scope -- destructor should release all memory
}
uint64_t granted = stats->getTickerCount(PREFETCH_MEMORY_BYTES_GRANTED);
+1 -1
View File
@@ -3436,7 +3436,7 @@ TEST_F(BackupEngineTest, FileDetailsHaveCorrectFileType) {
bool found_options = false;
for (const auto& file_info : backup_info.file_details) {
// No file should have the default kTempFile type ParseFileName should
// No file should have the default kTempFile type -- ParseFileName should
// have successfully identified all backup files.
EXPECT_NE(file_info.file_type, kTempFile)
<< "Unexpected kTempFile for: " << file_info.relative_filename;
+1 -1
View File
@@ -443,7 +443,7 @@ TEST_F(CheckpointTest, ExportEmptyColumnFamily) {
options.create_if_missing = true;
CreateAndReopenWithCF({}, options);
// Do NOT put any data the default CF has no levels.
// Do NOT put any data -- the default CF has no levels.
Checkpoint* checkpoint;
ASSERT_OK(Checkpoint::Create(db_.get(), &checkpoint));
@@ -47,7 +47,7 @@ class SortedRunBuilderImpl : public SortedRunBuilder {
// Universal compaction for merging all L0 files
db_options.compaction_style = kCompactionStyleUniversal;
// Disable auto compaction we compact manually at the end
// Disable auto compaction -- we compact manually at the end
db_options.disable_auto_compactions = true;
// Memory budget
@@ -75,7 +75,7 @@ class SortedRunBuilderImpl : public SortedRunBuilder {
db_options.table_factory = options_.table_factory;
}
// WAL is not needed resumability comes from flushed SSTs
// WAL is not needed -- resumability comes from flushed SSTs
db_options.manual_wal_flush = true;
db_options.wal_bytes_per_sync = 0;
@@ -309,7 +309,7 @@ TEST_F(SortedRunBuilderTest, NumEntriesAfterDedup) {
std::unique_ptr<SortedRunBuilder> builder;
ASSERT_OK(SortedRunBuilder::Create(opts, &builder));
// Add duplicate keys pre-Finish count includes duplicates
// Add duplicate keys -- pre-Finish count includes duplicates
ASSERT_OK(builder->Add("key1", "old_value"));
ASSERT_OK(builder->Add("key1", "new_value"));
ASSERT_OK(builder->Add("key2", "value2"));
@@ -317,7 +317,7 @@ TEST_F(SortedRunBuilderTest, NumEntriesAfterDedup) {
ASSERT_OK(builder->Finish());
// After Finish(), duplicates are resolved only 2 unique keys remain
// After Finish(), duplicates are resolved -- only 2 unique keys remain
ASSERT_EQ(builder->GetNumEntries(), 2);
}
@@ -412,7 +412,7 @@ TEST_F(SortedRunBuilderTest, DuplicateKeys) {
std::unique_ptr<SortedRunBuilder> builder;
ASSERT_OK(SortedRunBuilder::Create(opts, &builder));
// Add duplicate keys later value should win (RocksDB semantics)
// Add duplicate keys -- later value should win (RocksDB semantics)
ASSERT_OK(builder->Add("key1", "old_value"));
ASSERT_OK(builder->Add("key1", "new_value"));
ASSERT_OK(builder->Add("key2", "value2"));
@@ -510,7 +510,7 @@ TEST_F(SortedRunBuilderTest, DestructionWithoutFinish) {
std::unique_ptr<SortedRunBuilder> builder;
ASSERT_OK(SortedRunBuilder::Create(opts, &builder));
ASSERT_OK(builder->Add("key1", "value1"));
// Intentionally not calling Finish() destructor should clean up
// Intentionally not calling Finish() -- destructor should clean up
}
// Verify temp dir is cleaned up
@@ -4118,7 +4118,7 @@ TEST_P(WritePreparedTransactionTest, RangeTombstoneInsertionWithWritePrepared) {
ASSERT_OK(db->Delete(WriteOptions(), std::string(1, c)));
}
// Begin a transaction and prepare it the prepare_seq will be higher
// Begin a transaction and prepare it -- the prepare_seq will be higher
// than the tombstone seqnos above.
std::unique_ptr<Transaction> txn(db->BeginTransaction(WriteOptions()));
ASSERT_NE(txn, nullptr);
@@ -4173,14 +4173,14 @@ TEST_P(WritePreparedTransactionTest,
}
ASSERT_OK(db->Flush(FlushOptions()));
// Prepare a transaction first the prepare_seq will be low.
// Prepare a transaction first -- the prepare_seq will be low.
std::unique_ptr<Transaction> txn(db->BeginTransaction(WriteOptions()));
ASSERT_NE(txn, nullptr);
ASSERT_OK(txn->SetName("txn1"));
ASSERT_OK(txn->Put("z", "txn_val"));
ASSERT_OK(txn->Prepare());
// Now delete 5 contiguous keys these tombstones get seqnos AFTER
// Now delete 5 contiguous keys -- these tombstones get seqnos AFTER
// the prepare_seq, so max_tombstone_seq >= min_uncommitted.
for (char c = 'c'; c <= 'g'; c++) {
ASSERT_OK(db->Delete(WriteOptions(), std::string(1, c)));
+5 -5
View File
@@ -53,7 +53,7 @@ class WritePreparedTxnDB;
// db_impl_->WriteImpl(write_options, GetWriteBatch(),
// ..., !DISABLE_MEMTABLE, ...);
//
// !DISABLE_MEMTABLE is false memtable is enabled. This is the defining
// !DISABLE_MEMTABLE is false -- memtable is enabled. This is the defining
// characteristic of "WritePrepared": the actual data (Put("key1", "value1"))
// is written to the memtable at Prepare time.
//
@@ -68,7 +68,7 @@ class WritePreparedTxnDB;
//
// The data is now durable (WAL) and in the memtable, but not yet visible
// to readers. Readers use GetLastPublishedSequence() which consults a
// commit map since prepare_seq is in the PreparedHeap but not yet in the
// commit map -- since prepare_seq is in the PreparedHeap but not yet in the
// CommitCache, readers know this data is uncommitted and skip it.
//
// -- Phase 2: Commit (CommitInternal) --
@@ -92,7 +92,7 @@ class WritePreparedTxnDB;
// Destination | What gets written | Sequence
// ------------|---------------------|-----------
// WAL | Commit(txn1) marker | commit_seq
// Memtable | Nothing |
// Memtable | Nothing | --
//
// The PreReleaseCallback (WritePreparedCommitEntryPreReleaseCallback)
// updates the CommitCache to record that prepare_seq was committed at
@@ -101,7 +101,7 @@ class WritePreparedTxnDB;
//
// -- Why two queues help --
//
// The Commit phase doesn't touch the memtable it only writes a small
// The Commit phase doesn't touch the memtable -- it only writes a small
// marker to WAL and updates an in-memory commit map. By routing this
// through a separate queue, Commit writes don't have to wait behind other
// transactions' Prepare writes (which do the expensive memtable insertion
@@ -119,7 +119,7 @@ class WritePreparedTxnDB;
// FetchAdd alloc seq 9 | 10 | 9
// Write WAL + memtable
// SetLastSequence 10 | 10 | 9
// (published_seq not advanced yet data is uncommitted)
// (published_seq not advanced yet -- data is uncommitted)
//
// Commit (2nd queue):
// FetchAdd alloc seq 10 | 11 | 9
@@ -795,7 +795,7 @@ TEST_P(WriteUnpreparedTransactionTest, RangeTombstoneMultipleBatchesAndCommit) {
ASSERT_OK(txn->Put("b", "txn_b")); // batch 1, bounds start of run
if (middle_tombstone) {
// Txn Delete("e") fills the gap in the middle of committed deletes
// c, d, [e], f, g making a contiguous run of 5 that contains an
// c, d, [e], f, g -- making a contiguous run of 5 that contains an
// uncommitted seqno in the middle.
ASSERT_OK(txn->Delete("e"));
} else {
@@ -886,7 +886,7 @@ TEST_P(WriteUnpreparedTransactionTest,
ASSERT_NE(txn, nullptr);
txn->SetSnapshot();
// Multiple unprepared writes these get seqnos beyond the snapshot.
// Multiple unprepared writes -- these get seqnos beyond the snapshot.
// CalcMaxVisibleSeq returns max(last_unprep_seqno, snapshot_seq), so
// the committed deletions (seqno between snap and unprep) are visible.
ASSERT_OK(txn->Put("x", "txn_x"));
@@ -991,7 +991,7 @@ TEST_P(WriteUnpreparedTransactionTest, RangeTombstoneOwnDeletionsAndRollback) {
SyncPoint::GetInstance()->DisableProcessing();
SyncPoint::GetInstance()->ClearAllCallBacks();
// Rollback own Deletes and Puts are undone.
// Rollback -- own Deletes and Puts are undone.
ASSERT_OK(txn->Rollback());
// After rollback: committed deletes c-g remain, own deletes j-n are
+4 -4
View File
@@ -247,7 +247,7 @@ class BitvectorBuilder {
//
// Created from serialized data (e.g., read from an SST file meta-block) or
// from a BitvectorBuilder. The bitvector does NOT own the underlying memory
// when created from a Slice the caller must ensure the data outlives this
// when created from a Slice -- the caller must ensure the data outlives this
// object.
//
// Select acceleration: select hints store the rank LUT index where every
@@ -379,7 +379,7 @@ class Bitvector {
// select1(i): Position of the i-th 1-bit (0-indexed).
// Returns num_bits_ if there are fewer than (i+1) 1-bits.
// Inlined for hot-path performance the hint lookup + linear scan is
// Inlined for hot-path performance -- the hint lookup + linear scan is
// branch-predictor friendly and avoids function call overhead.
inline uint64_t FindNthOneBit(uint64_t i) const {
if (i >= num_ones_) {
@@ -422,7 +422,7 @@ class Bitvector {
// Find the next set bit at or after position `pos`.
// Returns num_bits_ if no set bit is found.
// Inlined for hot-path performance called on every dense Seek, Advance,
// Inlined for hot-path performance -- called on every dense Seek, Advance,
// and DescendToLeftmostLeaf.
inline uint64_t NextSetBit(uint64_t pos) const {
if (pos >= num_bits_) {
@@ -580,7 +580,7 @@ class EliasFano {
// Custom move operations to re-seat low_words_ after moving owned_low_data_.
// The default move would leave low_words_ pointing into the moved-from
// object's owned_low_data_ buffer. For SSO-sized strings (owned_low_data_
// with <= ~22 bytes, e.g. count_=1 low_bits_=8 8 bytes), std::string
// with <= ~22 bytes, e.g. count_=1 low_bits_=8 -> 8 bytes), std::string
// move copies the SSO buffer to a new address rather than transferring a
// heap pointer, leaving low_words_ dangling. For heap-allocated strings
// move transfers the pointer (address preserved), but we re-seat
+16 -16
View File
@@ -255,12 +255,12 @@ void LoudsTrieBuilder::Finish() {
if (lcp > 0) {
auto& pl = levels[lcp - 1];
if (!pl.has_child.empty() && !pl.has_child.back()) {
// Leaf internal transition.
// Leaf -> internal transition.
pl.has_child.back() = true;
// Handle migration: move the leaf's handle to a NEW child node at
// level lcp as a prefix key. The child node is always new because
// this label just transitioned from leaf to internal no child
// this label just transitioned from leaf to internal -- no child
// node existed before for this branch.
//
// The leaf_handle must be valid (>= 0) because the label was marked
@@ -306,7 +306,7 @@ void LoudsTrieBuilder::Finish() {
// created left-to-right as sorted keys are processed). We iterate by
// level and by node within each level:
// - For each node: emit prefix key handle (if any), then for each label:
// if leaf emit handle; if internal skip (child visited at next
// if leaf -> emit handle; if internal -> skip (child visited at next
// level).
// - Build dense/sparse bitvectors from labels and has_child flags.
// =========================================================================
@@ -393,7 +393,7 @@ void LoudsTrieBuilder::Finish() {
emit_leaf(static_cast<size_t>(ld.prefix_handle[ni]));
}
// Skip pure leaf nodes (no labels) they are accounted for by
// Skip pure leaf nodes (no labels) -- they are accounted for by
// has_child=0 in their parent. They don't produce LOUDS entries.
if (label_start == label_end) {
if (ld.is_prefix[ni]) {
@@ -640,7 +640,7 @@ void LoudsTrieBuilder::SerializeAll() {
continue;
}
// Check if the child node is a prefix key if so, cannot skip it.
// Check if the child node is a prefix key -- if so, cannot skip it.
{
uint64_t child_sparse_node = bv_s_louds.Rank1(cs + 1) - 1;
if (child_sparse_node < bv_s_is_prefix_key.NumBits() &&
@@ -674,7 +674,7 @@ void LoudsTrieBuilder::SerializeAll() {
break;
}
// Check if next node is a prefix key stop chain here.
// Check if next node is a prefix key -- stop chain here.
{
uint64_t next_sparse_node = bv_s_louds.Rank1(next_cs + 1) - 1;
if (next_sparse_node < bv_s_is_prefix_key.NumBits() &&
@@ -1118,7 +1118,7 @@ Status LoudsTrie::InitFromData(const Slice& data) {
// Validate dense section counts against bitvector sizes. These counts
// were read from the header (untrusted data) and will be used for leaf
// ordinal computation during traversal. Inconsistent values would cause
// incorrect rank arithmetic leading to wrong leaf indices wrong block
// incorrect rank arithmetic leading to wrong leaf indices -> wrong block
// handles.
if (dense_node_count_ > 0) {
// Each dense node uses 256 bits in d_labels_.
@@ -2150,7 +2150,7 @@ bool LoudsTrieIterator::SeekImpl(const Slice& target) {
return DescendToLeftmostLeaf(false, SparseChildNodeNum(cs));
}
} // if constexpr (kHasChains)
// No chain normal child lookup.
// No chain -- normal child lookup.
sparse_start = trie_->s_child_start_pos_[child_idx];
sparse_end = trie_->s_child_end_pos_[child_idx];
have_sparse_bounds = true;
@@ -2386,7 +2386,7 @@ bool LoudsTrieIterator::SeekImpl(const Slice& target) {
return DescendToLeftmostLeaf(false, SparseChildNodeNum(cs));
}
} // if constexpr (kHasChains)
// No chain normal child lookup.
// No chain -- normal child lookup.
sparse_start = trie_->s_child_start_pos_[child_idx];
sparse_end = trie_->s_child_end_pos_[child_idx];
have_sparse_bounds = true;
@@ -2558,7 +2558,7 @@ bool LoudsTrieIterator::Advance() {
}
}
// No sibling found at this level pop and continue backtracking.
// No sibling found at this level -- pop and continue backtracking.
path_.pop_back();
if (key_len_ > 0) {
key_len_--;
@@ -2572,7 +2572,7 @@ bool LoudsTrieIterator::Advance() {
bool LoudsTrieIterator::DescendToRightmostLeaf(bool in_dense,
uint64_t node_num) {
// Mirror of DescendToLeftmostLeaf: go to the rightmost (largest) child
// at each level. Does NOT check prefix keys they are the smallest leaf
// at each level. Does NOT check prefix keys -- they are the smallest leaf
// at a node, so they come last in reverse order and are handled by
// Retreat() when backtracking past all children.
@@ -2595,7 +2595,7 @@ bool LoudsTrieIterator::DescendToRightmostLeaf(bool in_dense,
}
uint64_t last = trie_->d_labels_.PrevSetBit(node_end);
if (last >= trie_->d_labels_.NumBits() || last < base) {
// Empty node shouldn't happen in a valid trie, but guard anyway.
// Empty node -- shouldn't happen in a valid trie, but guard anyway.
valid_ = false;
return false;
}
@@ -2705,13 +2705,13 @@ bool LoudsTrieIterator::Retreat() {
cd, cd ? child : child - trie_->dense_node_count_);
}
// No previous sibling check prefix key at this node.
// No previous sibling -- check prefix key at this node.
// Prefix keys are the smallest leaf at a node, so in reverse order
// they come after all children.
if (trie_->d_is_prefix_key_.NumBits() > 0 &&
node_num < trie_->d_is_prefix_key_.NumBits() &&
trie_->d_is_prefix_key_.GetBit(node_num)) {
// Pop the current level before returning the prefix key the
// Pop the current level before returning the prefix key -- the
// prefix key doesn't have its own path entry.
path_.pop_back();
if (key_len_ > 0) {
@@ -2743,7 +2743,7 @@ bool LoudsTrieIterator::Retreat() {
return DescendToRightmostLeaf(false, SparseChildNodeNum(prev_pos));
}
// At first label no previous sibling. Check prefix key.
// At first label -- no previous sibling. Check prefix key.
uint64_t sparse_node = SparseNodeNum(cur_sparse_pos);
if (trie_->s_is_prefix_key_.NumBits() > 0 &&
sparse_node < trie_->s_is_prefix_key_.NumBits() &&
@@ -2759,7 +2759,7 @@ bool LoudsTrieIterator::Retreat() {
}
}
// No sibling and no prefix key pop and continue backtracking.
// No sibling and no prefix key -- pop and continue backtracking.
path_.pop_back();
if (key_len_ > 0) {
key_len_--;
+3 -3
View File
@@ -350,7 +350,7 @@ class LoudsTrie {
// plus compact arrays indexed by chain ordinal (Rank1 on the bitmap).
//
// Lookup during Seek:
// 1. s_chain_bitmap_.GetBit(child_idx) has chain?
// 1. s_chain_bitmap_.GetBit(child_idx) -- has chain?
// 2. chain_idx = s_chain_bitmap_.Rank1(child_idx + 1) - 1
// 3. s_chain_lens_[chain_idx], s_chain_suffix_offsets_[chain_idx], etc.
//
@@ -628,7 +628,7 @@ class LoudsTrieIterator {
// Descend from the given node to the rightmost leaf in its subtree,
// pushing entries onto path_ and building key_buf_. Sets
// leaf_index_ and valid_. Returns true if a leaf was found.
// Unlike DescendToLeftmostLeaf, this does NOT check prefix keys
// Unlike DescendToLeftmostLeaf, this does NOT check prefix keys --
// prefix keys are the smallest leaf at a node, so in reverse order
// they are visited last (handled by Retreat when backtracking).
bool DescendToRightmostLeaf(bool in_dense, uint64_t node_num);
@@ -668,7 +668,7 @@ class LoudsTrieIterator {
// sparse levels, the byte is s_labels_[pos].
//
// Key reconstruction appends one byte per trie level in the Seek/Next
// hot loop, so the append operation must be as cheap as possible a
// hot loop, so the append operation must be as cheap as possible -- a
// single inlined store + increment with no function call overhead. The
// buffer is heap-allocated once in the constructor to MaxDepth()+1 bytes.
//
+15 -15
View File
@@ -2971,7 +2971,7 @@ TEST_P(TrieIndexDBTest, EmptyDBOperations) {
ASSERT_OK(iter->status());
}
// Create an SST, delete its only key, compact DB has no live data but
// Create an SST, delete its only key, compact -> DB has no live data but
// the trie code path was exercised during flush.
ASSERT_OK(db_->Put(WriteOptions(), "temp", "val"));
ASSERT_OK(db_->Flush(FlushOptions()));
@@ -3019,7 +3019,7 @@ TEST_P(TrieIndexDBTest, SeekEdgeCases) {
ASSERT_TRUE(iter->Valid());
ASSERT_EQ(iter->key().ToString(), "ddd");
// Between keys (eee fff).
// Between keys (eee -> fff).
iter->Seek("eee");
ASSERT_TRUE(iter->Valid());
ASSERT_EQ(iter->key().ToString(), "fff");
@@ -3137,7 +3137,7 @@ TEST_P(TrieIndexDBTest, OverlappingL0SSTs) {
}
}
// Compact all L0 L1, re-verify.
// Compact all L0 -> L1, re-verify.
ASSERT_OK(db_->CompactRange(CompactRangeOptions(), nullptr, nullptr));
for (const auto& ro : {StandardIndexReadOptions(), TrieIndexReadOptions()}) {
SCOPED_TRACE(ro.table_index_factory ? "trie" : "standard");
@@ -3367,7 +3367,7 @@ TEST_P(TrieIndexDBTest, IteratorUpperBound) {
{StandardIndexReadOptions(), TrieIndexReadOptions()}) {
SCOPED_TRACE(base_ro.table_index_factory ? "trie" : "standard");
// Upper bound = "dd" should see aa, bb, cc only.
// Upper bound = "dd" -> should see aa, bb, cc only.
std::string ub_str = "dd";
Slice ub(ub_str);
ReadOptions ro = base_ro;
@@ -3380,7 +3380,7 @@ TEST_P(TrieIndexDBTest, IteratorUpperBound) {
ASSERT_OK(iter->status());
ASSERT_EQ(keys, (std::vector<std::string>{"aa", "bb", "cc"}));
// Upper bound = "aa" should see nothing.
// Upper bound = "aa" -> should see nothing.
std::string ub2_str = "aa";
Slice ub2(ub2_str);
ReadOptions ro2 = base_ro;
@@ -3390,7 +3390,7 @@ TEST_P(TrieIndexDBTest, IteratorUpperBound) {
ASSERT_FALSE(iter2->Valid());
ASSERT_OK(iter2->status());
// Upper bound after all data should see everything.
// Upper bound after all data -> should see everything.
std::string ub3_str = "zz";
Slice ub3(ub3_str);
ReadOptions ro3 = base_ro;
@@ -3471,7 +3471,7 @@ TEST_P(TrieIndexDBTest, ManySmallSSTs) {
options_.disable_auto_compactions = true;
ASSERT_OK(OpenDB());
// 50 flushes, 2 keys each 50 SSTs.
// 50 flushes, 2 keys each -> 50 SSTs.
for (int f = 0; f < 50; f++) {
char k1[16];
char k2[16];
@@ -3578,7 +3578,7 @@ TEST_P(TrieIndexDBTest, MixedSSTsWithAndWithoutUDI) {
}
options_.disable_auto_compactions = true;
// Phase 1: Write with UDI SST1 has UDI + standard index.
// Phase 1: Write with UDI -> SST1 has UDI + standard index.
ASSERT_OK(OpenDB());
ASSERT_OK(db_->Put(WriteOptions(), "key_01", "udi_val1"));
ASSERT_OK(db_->Put(WriteOptions(), "key_02", "udi_val2"));
@@ -3586,7 +3586,7 @@ TEST_P(TrieIndexDBTest, MixedSSTsWithAndWithoutUDI) {
ASSERT_OK(db_->Close());
db_.reset();
// Phase 2: Reopen WITHOUT UDI, write more SST2 has only standard index.
// Phase 2: Reopen WITHOUT UDI, write more -> SST2 has only standard index.
ASSERT_OK(OpenDBWithoutUDI());
ASSERT_OK(db_->Put(WriteOptions(), "key_03", "noudi_val3"));
ASSERT_OK(db_->Put(WriteOptions(), "key_04", "noudi_val4"));
@@ -3595,7 +3595,7 @@ TEST_P(TrieIndexDBTest, MixedSSTsWithAndWithoutUDI) {
db_.reset();
// Phase 3: Reopen WITH UDI again. SST1 uses trie, SST2 falls back to
// standard index (UDI block missing logged warning, graceful fallback).
// standard index (UDI block missing -> logged warning, graceful fallback).
options_.disable_auto_compactions = true;
ASSERT_OK(OpenDB());
@@ -3608,7 +3608,7 @@ TEST_P(TrieIndexDBTest, MixedSSTsWithAndWithoutUDI) {
ASSERT_NO_FATAL_FAILURE(
VerifyScanBothIndexes({"key_01", "key_02", "key_03", "key_04"}));
// Compact: merges UDI + non-UDI SSTs new SST has UDI.
// Compact: merges UDI + non-UDI SSTs -> new SST has UDI.
ASSERT_OK(db_->CompactRange(CompactRangeOptions(), nullptr, nullptr));
ASSERT_NO_FATAL_FAILURE(
VerifyScanBothIndexes({"key_01", "key_02", "key_03", "key_04"}));
@@ -4236,7 +4236,7 @@ TEST_P(TrieIndexDBTest, PrimaryUDIBackwardCompatibility) {
TEST_P(TrieIndexDBTest, MigrationFullPath) {
// Tests the complete recommended migration path:
// Step 1: No UDI Step 2: UDI secondary Step 3: Compact all SSTs
// Step 1: No UDI -> Step 2: UDI secondary -> Step 3: Compact all SSTs ->
// Step 4: UDI primary
// Step 1: Start without UDI. Write some data.
@@ -4317,7 +4317,7 @@ TEST_P(TrieIndexDBTest, MigrationPrimaryRejectsPreUDISSTs) {
}
TEST_P(TrieIndexDBTest, RollbackFromPrimaryToSecondary) {
// Tests the rollback path: primary compact with secondary remove UDI.
// Tests the rollback path: primary -> compact with secondary -> remove UDI.
// Start in primary mode. Write data.
ASSERT_OK(OpenDBPrimary(/*block_size=*/128));
@@ -4617,7 +4617,7 @@ TEST_P(TrieIndexDBTest, GetEntityWithExplicitSnapshotComparison) {
ASSERT_OK(db_->Put(WriteOptions(), "regular_key", "regular_val_v2"));
ASSERT_OK(db_->Flush(FlushOptions()));
// Read at snapshot through both indexes should see v1 data.
// Read at snapshot through both indexes -- should see v1 data.
for (auto base_ro : {StandardIndexReadOptions(), TrieIndexReadOptions()}) {
SCOPED_TRACE(base_ro.table_index_factory ? "trie" : "standard");
base_ro.snapshot = snap;
@@ -4644,7 +4644,7 @@ TEST_P(TrieIndexDBTest, GetEntityWithExplicitSnapshotComparison) {
.IsNotFound());
}
// Read without snapshot should see v2 data.
// Read without snapshot -- should see v2 data.
for (auto base_ro : {StandardIndexReadOptions(), TrieIndexReadOptions()}) {
SCOPED_TRACE(base_ro.table_index_factory ? "trie" : "standard");
+16 -16
View File
@@ -36,7 +36,7 @@ Slice TrieIndexBuilder::AddIndexEntry(const Slice& last_key_in_current_block,
// comparator. FindShortestSeparator takes `*start` as both input and output:
// input: *start == last_key_in_current_block
// output: *start modified to shortest string in [start, limit)
// If first_key_in_next_block is nullptr, this is the last block use a
// If first_key_in_next_block is nullptr, this is the last block -- use a
// short successor of the last key.
Slice separator;
// True when last_key and first_key_in_next_block are the same user key
@@ -85,7 +85,7 @@ Slice TrieIndexBuilder::AddIndexEntry(const Slice& last_key_in_current_block,
// ":", but the data block only contains keys up to "9\xff\xff". A seek
// targeting a key in that gap (e.g., "9\xff\xff\x01") would find a
// block via the trie that contains no matching data, causing iterator
// desynchronization the trie index returns a valid block while the
// desynchronization -- the trie index returns a valid block while the
// standard index correctly reports no match.
separator = last_key_in_current_block;
@@ -157,9 +157,9 @@ Status TrieIndexBuilder::Finish(Slice* index_contents) {
if (use_seqno) {
// Feed de-duplicated separators to the trie with seqno side-table metadata.
// Consecutive identical separators form a "run" only the first occurrence
// goes into the trie (as the primary block). The remaining blocks in the
// run are stored as overflow blocks in the side-table.
// Consecutive identical separators form a "run" -- only the first
// occurrence goes into the trie (as the primary block). The remaining
// blocks in the run are stored as overflow blocks in the side-table.
//
// For non-boundary separators (different user keys), the tag is 0
// (sentinel meaning "no seqno correction needed"), matching the standard
@@ -213,11 +213,11 @@ Status TrieIndexBuilder::Finish(Slice* index_contents) {
assert(buffered_entries_.empty());
}
// Release buffered entries no longer needed after feeding to the trie.
// Release buffered entries -- no longer needed after feeding to the trie.
buffered_entries_.clear();
buffered_entries_.shrink_to_fit();
// Always finish the trie builder, even with 0 keys this produces a valid
// Always finish the trie builder, even with 0 keys -- this produces a valid
// serialized trie that can be parsed by NewReader. Without this, an empty
// Slice would be returned, causing InitFromData to fail with "data too short
// for header".
@@ -343,12 +343,12 @@ Status TrieIndexIterator::SeekAndGetResult(const Slice& target,
ResetOverflowState();
// Always seek with user key only the trie stores user-key separators.
// Always seek with user key only -- the trie stores user-key separators.
// When seqno encoding is active, post-seek correction handles the seqno.
if (!iter_.Seek(target)) {
// No leaf has a key >= target: the target is past all blocks in this SST.
// Return kUnknown (not kOutOfBound) because exhausting this SST's trie
// says nothing about the upper bound the next SST on the level may
// says nothing about the upper bound -- the next SST on the level may
// still contain in-bound keys. kOutOfBound would cause LevelIterator to
// stop scanning the level prematurely.
result->bound_check_result = IterBoundCheck::kUnknown;
@@ -357,7 +357,7 @@ Status TrieIndexIterator::SeekAndGetResult(const Slice& target,
}
// Set the result key (always a user key, no suffix stripping needed).
// Reuse current_key_scratch_ capacity avoids heap allocation after warmup.
// Reuse current_key_scratch_ capacity -- avoids heap allocation after warmup.
{
Slice trie_key = iter_.Key();
current_key_scratch_.assign(trie_key.data(), trie_key.size());
@@ -412,7 +412,7 @@ Status TrieIndexIterator::SeekAndGetResult(const Slice& target,
// the next trie leaf (the block after the run).
if (!iter_.Next()) {
// Exhausted all blocks: target is past the end of this SST.
// Return kUnknown see comment in Seek path above.
// Return kUnknown -- see comment in Seek path above.
result->bound_check_result = IterBoundCheck::kUnknown;
result->key = Slice();
return Status::OK();
@@ -428,7 +428,7 @@ Status TrieIndexIterator::SeekAndGetResult(const Slice& target,
overflow_base_idx_ = 0;
// Check if the new leaf also has overflow (unlikely but possible
// with adjacent same-key runs for different user keys).
// iter_.Valid() is guaranteed here Next() returned true above.
// iter_.Valid() is guaranteed here -- Next() returned true above.
if (has_seqno_encoding_) {
uint64_t new_leaf = iter_.LeafIndex();
overflow_run_size_ = trie_->GetLeafBlockCount(new_leaf);
@@ -480,11 +480,11 @@ Status TrieIndexIterator::NextAndGetResult(IterateResult* result) {
UserDefinedIndexBuilder::BlockHandle TrieIndexIterator::value() {
if (overflow_run_index_ == 0) {
// Primary block use the trie leaf's handle.
// Primary block -- use the trie leaf's handle.
auto handle = iter_.Value();
return UserDefinedIndexBuilder::BlockHandle{handle.offset, handle.size};
}
// Overflow block use the side-table handle.
// Overflow block -- use the side-table handle.
// overflow_run_index_ is 1-based, overflow array is 0-based.
uint32_t overflow_idx = overflow_base_idx_ + overflow_run_index_ - 1;
auto handle = trie_->GetOverflowHandle(overflow_idx);
@@ -494,7 +494,7 @@ UserDefinedIndexBuilder::BlockHandle TrieIndexIterator::value() {
IterBoundCheck TrieIndexIterator::CheckBounds(
const Slice& reference_key) const {
if (!prepared_ || scan_opts_.empty()) {
// No bounds to check always in-bound.
// No bounds to check -- always in-bound.
return IterBoundCheck::kInbound;
}
@@ -549,7 +549,7 @@ size_t TrieIndexReader::ApproximateMemoryUsage() const {
// and handle arrays, so the base cost is the serialized data size. On top
// of that, InitFromData() heap-allocates child position lookup tables
// (s_child_start_pos_ and s_child_end_pos_) for Select-free sparse
// traversal 8 bytes per sparse internal node.
// traversal -- 8 bytes per sparse internal node.
return data_size_ + trie_.ApproximateAuxMemoryUsage();
}
+3 -3
View File
@@ -71,7 +71,7 @@ class TrieIndexBuilder final : public UserDefinedIndexBuilder {
std::string* separator_scratch,
const IndexEntryContext& context) override;
// Called for each key added to the SST. Currently a no-op the trie is
// Called for each key added to the SST. Currently a no-op -- the trie is
// built entirely from separator keys provided via AddIndexEntry().
void OnKeyAdded(const Slice& key, ValueType type,
const Slice& value) override;
@@ -100,7 +100,7 @@ class TrieIndexBuilder final : public UserDefinedIndexBuilder {
// We buffer all separator entries during building, then at Finish() feed
// them to the trie with seqno side-table metadata.
//
// Always set to true in AddIndexEntry() seqno encoding is
// Always set to true in AddIndexEntry() -- seqno encoding is
// unconditionally enabled. The 8-byte per-leaf overhead is always incurred.
bool must_use_separator_with_seq_;
@@ -172,7 +172,7 @@ class TrieIndexIterator final : public UserDefinedIndexIterator {
// is the seek target, for Next this is the previous separator key.
// The trie stores separator keys (upper bounds on block contents), not
// first-in-block keys, so we cannot compare the current separator against
// the limit directly see the UDI API contract in user_defined_index.h.
// the limit directly -- see the UDI API contract in user_defined_index.h.
IterBoundCheck CheckBounds(const Slice& reference_key) const;
const Comparator* comparator_;
File diff suppressed because it is too large Load Diff