Uh oh!
There was an error while loading. Please reload this page.
DRY up batched KVStore reads utility methods - #876
Conversation
👋 Thanks for assigning @joostjager as a reviewer! |
ldk-reviews-bot
commented
Apr 18, 2026
🔔 1st Reminder Hey @joostjager! This PR has been waiting for your review. |
ldk-reviews-bot
commented
Apr 20, 2026
🔔 2nd Reminder Hey @joostjager! This PR has been waiting for your review. |
Camillarhi
commented
Apr 20, 2026
LGTM |
Camillarhi
commented
Apr 21, 2026
This now needs a rebase, there's a merge conflict. |
ldk-reviews-bot
commented
Apr 22, 2026
🔔 3rd Reminder Hey @joostjager! This PR has been waiting for your review. |
joostjager
left a comment
There was a problem hiding this comment.
I am not sure a generic client-side read throttle is the right abstraction here.
I think the motivation is mainly VSS-specific? For local stores such as SQLite, this mostly limits task fan-out rather than real backend concurrency. Perhaps it should be implemented only in the VSS client. And perhaps longer term, a multi-key read would be helpful in the VSS protocol?
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
tnull
commented
Apr 24, 2026
Hmm, I tend to agree, though pre-existing.
Well, yes, presumably postgres could also benefit from it, though the store probably should switch to connection pooling anyways?
Yeah, maybe for now I should just switch to wrap |
joostjager
left a comment
There was a problem hiding this comment.
Yeah, maybe for now I should just switch to wrap VssStore in BatchingStore in this PR? WDYT?
I think this is a good move, and later expand either towards protocol-level multi reads for vss, or perhaps generalize for other backends if the need arises.
tnull
commented
Apr 28, 2026
Discussed offline, decided we're not going with the |
BatchingStoreKVStore reads utility methods5b8cebf to
22866abCompare
joostjager
left a comment
There was a problem hiding this comment.
Looks good, uncontentious. Rebase needed.
Uh oh!
There was an error while loading. Please reload this page.
The parallel `JoinSet`-based batching loop was duplicated across `read_payments` and `read_pending_payments`. Extract it into a generic `read_all_objects<T: Readable>` helper that callers invoke directly with the relevant namespace constants. Per-type log messages are preserved via `std::any::type_name::<T>()`. Co-Authored-By: HAL 9000
22866ab to
7aa48faComparetnull
commented
Apr 28, 2026
Rebased, sorry, forgot about that before. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.