Skip to content

Commit cde6037

Browse files
ovitrifclaude
andcommitted
fix: skip redundant monitor write when update_id is equal
Use >= instead of > when comparing existing vs migrated update_id. When they are equal, the write is redundant — the store already has equivalent state. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 630b364 commit cde6037

File tree

1 file changed

+6
-44
lines changed

1 file changed

+6
-44
lines changed

src/builder.rs

Lines changed: 6 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1403,10 +1403,10 @@ where
14031403
) {
14041404
Ok((_, existing_monitor)) => {
14051405
let existing_update_id = existing_monitor.get_latest_update_id();
1406-
if existing_update_id > migrated_update_id {
1406+
if existing_update_id >= migrated_update_id {
14071407
log_warn!(
14081408
logger,
1409-
"Skipping migration for monitor {}: existing update_id {} is newer than migrated update_id {}",
1409+
"Skipping migration for monitor {}: existing update_id {} is newer than or equal to migrated update_id {}",
14101410
monitor_key,
14111411
existing_update_id,
14121412
migrated_update_id
@@ -2637,13 +2637,13 @@ mod tests {
26372637
}
26382638

26392639
#[test]
2640-
fn test_migration_skips_when_existing_is_newer() {
2640+
fn test_migration_skips_when_existing_is_newer_or_equal() {
26412641
let (monitor_bytes, monitor_key, _, seed) = create_test_monitor_bytes();
26422642
let (store, keys_manager, logger, runtime) = make_test_deps(&seed);
26432643

2644-
// Both existing and migrated have update_id=0. The function only skips
2645-
// when existing > migrated, so equal means it will write (not skip).
2646-
// This test verifies that the "equal" path succeeds.
2644+
// Pre-populate the store with valid monitor data (update_id=0).
2645+
// Migration data also has update_id=0. Since existing >= migrated,
2646+
// the function should skip the redundant write and succeed.
26472647
runtime
26482648
.block_on(KVStore::write(
26492649
&*store,
@@ -2664,44 +2664,6 @@ mod tests {
26642664
assert!(result.is_ok());
26652665
}
26662666

2667-
#[test]
2668-
fn test_migration_overwrites_when_existing_is_older() {
2669-
let (monitor_bytes, monitor_key, _, seed) = create_test_monitor_bytes();
2670-
let (store, keys_manager, logger, runtime) = make_test_deps(&seed);
2671-
2672-
// Write the monitor to the store first (same update_id=0).
2673-
runtime
2674-
.block_on(KVStore::write(
2675-
&*store,
2676-
CHANNEL_MONITOR_PERSISTENCE_PRIMARY_NAMESPACE,
2677-
CHANNEL_MONITOR_PERSISTENCE_SECONDARY_NAMESPACE,
2678-
&monitor_key,
2679-
monitor_bytes.clone(),
2680-
))
2681-
.unwrap();
2682-
2683-
// Migrate with the same bytes — existing_update_id (0) is NOT greater than
2684-
// migrated_update_id (0), so it overwrites.
2685-
let migration = ChannelDataMigration {
2686-
channel_manager: None,
2687-
channel_monitors: vec![monitor_bytes.clone()],
2688-
};
2689-
2690-
let result =
2691-
apply_channel_data_migration(&migration, &store, &keys_manager, &logger, &runtime);
2692-
assert!(result.is_ok());
2693-
2694-
// Verify the store still has the data.
2695-
let stored = runtime.block_on(KVStore::read(
2696-
&*store,
2697-
CHANNEL_MONITOR_PERSISTENCE_PRIMARY_NAMESPACE,
2698-
CHANNEL_MONITOR_PERSISTENCE_SECONDARY_NAMESPACE,
2699-
&monitor_key,
2700-
));
2701-
assert!(stored.is_ok());
2702-
assert_eq!(stored.unwrap(), monitor_bytes);
2703-
}
2704-
27052667
#[test]
27062668
fn test_migration_fails_on_corrupt_existing_data() {
27072669
let (monitor_bytes, monitor_key, _, seed) = create_test_monitor_bytes();

0 commit comments

Comments
 (0)