Skip to content

Commit c7a41c4

Browse files
committed
chore(kv): elide redundant index rewrites on identical in-place updates
An in-place update that left an indexed column unchanged still removed and re-inserted the entry, at two index writes per index per PUT. Both writes were correct, so this is churn rather than a defect. Single-field and composite indexes now skip both operations when the extracted bytes are unchanged AND the (value, primary key) pair is already present. The membership gate matters: a row written before a backfill=false registration is absent from the index, and the next identical PUT has to file it in rather than elide it. Changed values keep the existing remove+insert path. Sorted indexes are untouched -- OrderStatTree::insert already returns early on an identical sort key. Fixes #279
1 parent 9998320 commit c7a41c4

3 files changed

Lines changed: 155 additions & 43 deletions

File tree

‎nodedb/src/engine/kv/index/composite.rs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,14 @@ impl KvCompositeIndex {
100100
}
101101
}
102102

103+
/// Whether the index contains the composite (field_values, primary_key) pair.
104+
pub(crate) fn contains(&self, field_values: &[&[u8]], primary_key: &[u8]) -> bool {
105+
let key = Self::build_key(field_values);
106+
self.tree
107+
.get(&key)
108+
.is_some_and(|keys| keys.contains(primary_key))
109+
}
110+
103111
/// Exact-match lookup on all fields.
104112
pub fn lookup_eq(&self, field_values: &[&[u8]]) -> Vec<&[u8]> {
105113
let key = Self::build_key(field_values);

‎nodedb/src/engine/kv/index/field.rs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,13 @@ impl KvFieldIndex {
8181
}
8282
}
8383

84+
/// Whether the index contains the (value, primary_key) pair.
85+
pub(crate) fn contains(&self, field_value: &[u8], primary_key: &[u8]) -> bool {
86+
self.tree
87+
.get(field_value)
88+
.is_some_and(|keys| keys.contains(primary_key))
89+
}
90+
8491
/// Exact-match lookup: find all primary keys where field == value.
8592
pub fn lookup_eq(&self, field_value: &[u8]) -> Vec<&[u8]> {
8693
self.tree

‎nodedb/src/engine/kv/index/set.rs‎

Lines changed: 140 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,18 @@ pub struct KvIndexSet {
2323
total_index_writes: u64,
2424
}
2525

26+
fn composite_vals<'a>(ci: &KvCompositeIndex, values: &'a [(&str, &[u8])]) -> Vec<&'a [u8]> {
27+
ci.fields()
28+
.iter()
29+
.filter_map(|f| {
30+
values
31+
.iter()
32+
.find(|(name, _)| *name == f.as_str())
33+
.map(|(_, v)| *v)
34+
})
35+
.collect()
36+
}
37+
2638
impl KvIndexSet {
2739
pub fn new() -> Self {
2840
Self {
@@ -155,62 +167,89 @@ impl KvIndexSet {
155167

156168
let mut writes = 0;
157169

158-
// Remove old single-field index entries (if this is an update).
170+
// Single-field indexes. Elide a no-op in-place update: when the old
171+
// indexed value equals the new one *and* the pair is already present,
172+
// remove+insert is pure churn. A row written before a backfill=false
173+
// registration is absent from the index; the next PUT with identical
174+
// bytes must insert it.
159175
if let Some(old_values) = old_field_values {
160176
for idx in &mut self.indexes {
161-
for &(field, value) in old_values {
162-
if field == idx.field() {
163-
idx.remove(value, primary_key);
177+
let f = idx.field();
178+
let new_val = field_values
179+
.iter()
180+
.find(|(field, _)| *field == f)
181+
.map(|(_, v)| *v);
182+
let old_val = old_values
183+
.iter()
184+
.find(|(field, _)| *field == f)
185+
.map(|(_, v)| *v);
186+
if new_val == old_val {
187+
if let Some(v) = new_val {
188+
if idx.contains(v, primary_key) {
189+
continue; // true no-op: already indexed
190+
}
191+
// old == new but absent (backfill=false gap): insert only —
192+
// there's no stale entry to remove, so don't count/perform one.
193+
idx.insert(v.to_vec(), primary_key.to_vec());
164194
writes += 1;
165195
}
196+
continue;
166197
}
167-
}
168-
}
169-
170-
// Insert new single-field index entries.
171-
for idx in &mut self.indexes {
172-
for &(field, value) in field_values {
173-
if field == idx.field() {
174-
idx.insert(value.to_vec(), primary_key.to_vec());
198+
if let Some(v) = old_val {
199+
idx.remove(v, primary_key);
200+
writes += 1;
201+
}
202+
if let Some(v) = new_val {
203+
idx.insert(v.to_vec(), primary_key.to_vec());
175204
writes += 1;
176205
}
177206
}
207+
} else {
208+
// Insert-only (new row): no old values to compare.
209+
for idx in &mut self.indexes {
210+
for &(field, value) in field_values {
211+
if field == idx.field() {
212+
idx.insert(value.to_vec(), primary_key.to_vec());
213+
writes += 1;
214+
}
215+
}
216+
}
178217
}
179218

180219
// Maintain composite indexes.
181220
for ci in &mut self.composite_indexes {
182-
// Remove old composite entry.
183-
if let Some(old_values) = old_field_values {
184-
let old_vals: Vec<&[u8]> = ci
185-
.fields()
186-
.iter()
187-
.filter_map(|f| {
188-
old_values
189-
.iter()
190-
.find(|(name, _)| *name == f.as_str())
191-
.map(|(_, v)| *v)
192-
})
193-
.collect();
194-
if old_vals.len() == ci.fields().len() {
195-
ci.remove(&old_vals, primary_key);
196-
writes += 1;
221+
// Composite entry, eliding the same no-op update case.
222+
match old_field_values {
223+
Some(old_values) => {
224+
let old_vals = composite_vals(ci, old_values);
225+
let new_vals = composite_vals(ci, field_values);
226+
if old_vals.len() == ci.fields().len() && new_vals.len() == ci.fields().len() {
227+
if old_vals == new_vals {
228+
if !ci.contains(&new_vals, primary_key) {
229+
ci.insert(&new_vals, primary_key.to_vec());
230+
writes += 1;
231+
}
232+
} else {
233+
ci.remove(&old_vals, primary_key);
234+
writes += 1;
235+
ci.insert(&new_vals, primary_key.to_vec());
236+
writes += 1;
237+
}
238+
} else if old_vals.len() == ci.fields().len() {
239+
ci.remove(&old_vals, primary_key);
240+
writes += 1;
241+
} else if new_vals.len() == ci.fields().len() {
242+
ci.insert(&new_vals, primary_key.to_vec());
243+
writes += 1;
244+
}
245+
}
246+
None => {
247+
let new_vals = composite_vals(ci, field_values);
248+
if new_vals.len() == ci.fields().len() {
249+
ci.insert(&new_vals, primary_key.to_vec());
250+
writes += 1;
251+
}
197252
}
198-
}
199-
200-
// Insert new composite entry.
201-
let new_vals: Vec<&[u8]> = ci
202-
.fields()
203-
.iter()
204-
.filter_map(|f| {
205-
field_values
206-
.iter()
207-
.find(|(name, _)| *name == f.as_str())
208-
.map(|(_, v)| *v)
209-
})
210-
.collect();
211-
if new_vals.len() == ci.fields().len() {
212-
ci.insert(&new_vals, primary_key.to_vec());
213-
writes += 1;
214253
}
215254
}
216255

@@ -334,6 +373,24 @@ mod tests {
334373
assert_eq!(set.lookup_eq("status", b"active").len(), 1);
335374
}
336375

376+
#[test]
377+
fn d6_identical_update_elides_index_writes() {
378+
// An in-place update that leaves the indexed value unchanged must not
379+
// rewrite the index when the pair is already present.
380+
let mut set = KvIndexSet::new();
381+
set.add_index("status", 0);
382+
set.on_put(b"k1", &[("status", b"active")], None);
383+
let writes = set.on_put(
384+
b"k1",
385+
&[("status", b"active")],
386+
Some(&[("status", b"active")]),
387+
);
388+
assert_eq!(
389+
writes, 0,
390+
"D6: identical update must elide index writes (got {writes})"
391+
);
392+
}
393+
337394
#[test]
338395
fn index_set_on_put_update_replaces_old() {
339396
let mut set = KvIndexSet::new();
@@ -420,6 +477,46 @@ mod tests {
420477
assert!(ci.lookup_eq(&[b"x", b"y"]).is_empty());
421478
}
422479

480+
#[test]
481+
fn d6_backfill_absent_pair_still_gets_inserted() {
482+
// old == new alone is not enough to elide — the pair must actually be
483+
// present. A row written before a backfill=false registration was never
484+
// indexed, so the next identical PUT has to insert it.
485+
let mut set = KvIndexSet::new();
486+
set.add_index("status", 0);
487+
// No prior on_put — simulates backfill=false: this row exists but
488+
// was never run through the indexer.
489+
let writes = set.on_put(
490+
b"k1",
491+
&[("status", b"active")],
492+
Some(&[("status", b"active")]), // old == new, but never indexed
493+
);
494+
assert_eq!(
495+
writes, 1,
496+
"backfill=false: absent pair must insert, got {writes}"
497+
);
498+
assert_eq!(set.lookup_eq("status", b"active"), vec![b"k1".as_slice()]);
499+
}
500+
501+
#[test]
502+
fn d6_composite_backfill_absent_pair_still_gets_inserted() {
503+
let mut set = KvIndexSet::new();
504+
set.add_composite_index(vec!["region".into(), "status".into()], vec![0, 1]);
505+
let writes = set.on_put(
506+
b"k1",
507+
&[("region", b"us-east"), ("status", b"active")],
508+
Some(&[("region", b"us-east"), ("status", b"active")]),
509+
);
510+
assert_eq!(
511+
writes, 1,
512+
"composite backfill=false: absent pair must insert"
513+
);
514+
let ci = set
515+
.get_composite_index(&["region".into(), "status".into()])
516+
.expect("composite index was registered");
517+
assert_eq!(ci.lookup_eq(&[b"us-east", b"active"]).len(), 1);
518+
}
519+
423520
/// The export accessors must see exactly the indexes that were registered —
424521
/// a checkpoint that iterated a partial view would publish rows whose index
425522
/// registrations are missing.

0 commit comments

Comments
 (0)