Skip to content

Commit 6053fd0

Browse files
committed
fix: backport review fixes from upstream PRs #270 and #273
Cherry-picked from upstream contribution branches: - 19aaf45: hash-index conflict test for seqno-aware seek - 044fdf9: accurate key_len comment in blob reader (conflict resolved: kept fork's more detailed error docs)
1 parent 644e2fe commit 6053fd0

2 files changed

Lines changed: 75 additions & 2 deletions

File tree

src/table/data_block/mod.rs

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1351,4 +1351,76 @@ mod tests {
13511351

13521352
Ok(())
13531353
}
1354+
1355+
#[test]
1356+
fn data_block_point_read_seqno_aware_seek_hash_conflict() -> crate::Result<()> {
1357+
// Multiple versions of the same key with a hash index enabled.
1358+
// Duplicate user keys hash to the same bucket, producing MARKER_CONFLICT,
1359+
// which forces point_read through the seek_to_key_seqno fallback path.
1360+
let items = [
1361+
InternalValue::from_components(b"a", b"a5", 5, Value),
1362+
InternalValue::from_components(b"a", b"a4", 4, Value),
1363+
InternalValue::from_components(b"a", b"a3", 3, Value),
1364+
InternalValue::from_components(b"a", b"a2", 2, Value),
1365+
InternalValue::from_components(b"a", b"a1", 1, Value),
1366+
];
1367+
1368+
for restart_interval in 1..=4 {
1369+
let bytes = DataBlock::encode_into_vec(&items, restart_interval, 1.33)?;
1370+
1371+
let data_block = DataBlock::new(Block {
1372+
data: bytes.into(),
1373+
header: Header {
1374+
block_type: BlockType::Data,
1375+
checksum: Checksum::from_raw(0),
1376+
data_length: 0,
1377+
uncompressed_length: 0,
1378+
},
1379+
});
1380+
1381+
// Verify hash index is present and the duplicate key triggers conflict
1382+
assert!(
1383+
data_block
1384+
.hash_bucket_count()
1385+
.expect("should have built hash index")
1386+
> 0,
1387+
"restart_interval={restart_interval}: hash index should be built",
1388+
);
1389+
1390+
// seqno=4 -> first version with seqno < 4, i.e. seqno=3
1391+
assert_eq!(
1392+
Some(items[2].clone()),
1393+
data_block.point_read(b"a", 4),
1394+
"restart_interval={restart_interval}: seqno=4 should return v3 via conflict fallback",
1395+
);
1396+
1397+
// seqno=3 -> seqno=2
1398+
assert_eq!(
1399+
Some(items[3].clone()),
1400+
data_block.point_read(b"a", 3),
1401+
"restart_interval={restart_interval}: seqno=3 should return v2 via conflict fallback",
1402+
);
1403+
1404+
// seqno=6 -> latest (seqno=5)
1405+
assert_eq!(
1406+
Some(items[0].clone()),
1407+
data_block.point_read(b"a", 6),
1408+
"restart_interval={restart_interval}: seqno=6 should return v5 via conflict fallback",
1409+
);
1410+
1411+
// seqno=1 -> no visible version
1412+
assert!(
1413+
data_block.point_read(b"a", 1).is_none(),
1414+
"restart_interval={restart_interval}: seqno=1 should return None via conflict fallback",
1415+
);
1416+
1417+
// Non-existent key
1418+
assert!(
1419+
data_block.point_read(b"z", SeqNo::MAX).is_none(),
1420+
"restart_interval={restart_interval}: key 'z' should not exist",
1421+
);
1422+
}
1423+
1424+
Ok(())
1425+
}
13541426
}

src/vlog/blob_file/reader.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,8 +122,9 @@ impl<'a> Reader<'a> {
122122
{
123123
// NOTE: Checksum is computed over the caller-provided key (not the on-disk
124124
// key bytes). This matches the writer, which hashes caller key + value.
125-
// On-disk key corruption is caught by the key_len cross-check above;
126-
// content-level key verification would require changing the checksum
125+
// The key_len cross-check above catches length mismatches (truncation or
126+
// wrong-handle reads), but not length-preserving key corruption.
127+
// Full content-level key verification would require changing the checksum
127128
// contract and is out of scope for this security hardening.
128129
let checksum = {
129130
let mut hasher = xxhash_rust::xxh3::Xxh3::default();

0 commit comments

Comments
 (0)