Add non-atomic ordered batch write method to KVStore traits#4379
Draft
joostjager wants to merge 4 commits intolightningdevkit:mainfrom
Draft
Add non-atomic ordered batch write method to KVStore traits#4379joostjager wants to merge 4 commits intolightningdevkit:mainfrom
joostjager wants to merge 4 commits intolightningdevkit:mainfrom
Conversation
Update MonitorUpdatingPersister and MonitorUpdatingPersisterAsync to queue persist operations in memory instead of writing immediately to disk. The Persist trait methods now return ChannelMonitorUpdateStatus:: InProgress and the actual writes happen when flush() is called. This fixes a race condition that could cause channel force closures: previously, if the node crashed after writing channel monitors but before writing the channel manager, the monitors would be ahead of the manager on restart. By deferring monitor writes until after the channel manager is persisted (via flush()), we ensure the manager is always at least as up-to-date as the monitors. The flush() method takes a count parameter specifying how many queued writes to flush. The background processor captures the queue size before persisting the channel manager, then flushes exactly that many writes afterward. This prevents flushing monitor updates that arrived after the manager state was captured. Key changes: - Add PendingWrite enum with FullMonitor and Update variants for queued writes - Add pending_writes queue to MonitorUpdatingPersisterAsyncInner - Add pending_write_count() and flush(count) to Persist trait and ChainMonitor - ChainMonitor::flush() calls channel_monitor_updated for each completed write - Stale update cleanup happens in flush() after full monitor is written - Call flush() in background processor after channel manager persistence Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
👋 Hi! I see this is a draft PR. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4379 +/- ##
==========================================
- Coverage 86.01% 85.96% -0.06%
==========================================
Files 156 156
Lines 102857 102924 +67
Branches 102857 102924 +67
==========================================
+ Hits 88474 88478 +4
- Misses 11876 11936 +60
- Partials 2507 2510 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Adds `write_batch` method to both `KVStoreSync` and `KVStore` traits with default implementations that delegate to the single `write` method. - `BatchWriteEntry`: struct containing namespace, key, and data - `BatchWriteResult`: returns successful write count and optional error - Writes execute sequentially in order; stops on first error - Existing implementations automatically inherit the default behavior Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Refactors flush() to batch all writes into a single write_batch() call instead of individual write() calls. On partial failure, failed and subsequent writes are re-queued at the front of the pending writes queue for retry on the next flush. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Extends Persist::flush() to accept channel_manager_bytes, allowing the channel manager to be written in the same write_batch() call as channel monitors. The channel manager is always written first in the batch to ensure proper ordering. This removes the separate channel manager persistence from the background processor and combines it with the monitor flush, reducing round trips. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
joostjager
force-pushed
the
batch-write
branch
from
3aa6003 to
6f20a00
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds
write_batchmethod to bothKVStoreSyncandKVStoretraits with default implementations that delegate to the singlewritemethod.BatchWriteEntry: struct containing namespace, key, and dataBatchWriteResult: returns successful write count and optional errorAnd show how it can be used in the background processor to write a complete batch.
Based on #4317