multi: make invoice loading async, only block callers when they wish to read the invoice state#168
Open
Roasbeef wants to merge 2 commits intolightninglabs:masterfrom
Open
multi: make invoice loading async, only block callers when they wish to read the invoice state#168Roasbeef wants to merge 2 commits intolightninglabs:masterfrom
Roasbeef wants to merge 2 commits intolightninglabs:masterfrom
Conversation
This will be used to simplify some of the logic the upcoming commit. We retain the existing condition variable usage, while also adding some methods that will be of use for the upcoming async background load.
This commit refactors the LND challenger's invoice handling mechanism to improve performance, reliability, and resource usage, especially for nodes with a large number of historical invoices. Previously, the challenger attempted to load all historical invoices in a single `ListInvoices` call during startup. This could lead to long startup times, high memory consumption, and potential timeouts or failures for nodes with extensive invoice history. Additionally, the state management relied on a mutex and condition variable, which could be complex to manage correctly. The key changes include: - **Concurrent Loading and Subscription:** The challenger now starts two background goroutines concurrently: one to load historical invoices and another to subscribe to new invoice updates using the latest known indices. - **Paginated Historical Loading:** Historical invoices are now fetched in batches using `ListInvoices` with `IndexOffset`. The batch size is configurable via the new `InvoiceBatchSize` config option (defaulting to 1000) and passed down from the main Aperture config. - **InvoiceStateStore:** A new `InvoiceStateStore` type is introduced to manage the invoice states map. This store handles thread-safe access, tracks the completion of the initial historical load, and provides a `WaitForState` method that correctly handles waiting for the initial load before checking for a specific invoice state. - **Improved Shutdown/Cancellation:** Context cancellation and handling of the quit signal are improved throughout the loading and subscription processes to ensure cleaner shutdowns. - **Refactored VerifyInvoiceStatus:** This method now delegates waiting logic to the `InvoiceStateStore`, simplifying the challenger code and ensuring it waits for the initial load if necessary.
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
1 participant
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.
In this PR, we refactor the LND challenger's invoice loading to be fully
asynchronous, so aperture can start serving requests while the historical
invoice set is still being populated in the background.
Previously,
LndChallenger.Startwould callListInvoiceswithNumMaxInvoices: math.MaxUint64, attempting to fetch the entire invoicehistory in a single RPC call. For nodes with a large number of historical
invoices (think 10+ minutes of loading on older nodes), this would block
startup entirely. The proxy couldn't serve any traffic until every last
invoice was loaded into memory.
We fix this by splitting the work into two concurrent goroutines that run
in the background: one paginates through the historical invoice set in
configurable batches (via the new
InvoiceBatchSizeconfig option,defaulting to 1000), while the other subscribes to real-time invoice
updates starting from the latest known indices. This way aperture is ready
to accept connections almost immediately after launch, and callers that
actually need to verify an invoice state will block only until the initial
load completes (with a configurable timeout).
The core abstraction enabling this is a new
InvoiceStateStoretype thatwraps the invoice states map with a
sync.Condfor signaling waiters. Ittracks whether the initial historical load has finished, and exposes a
WaitForStatemethod that first waits for the load to complete (if ithasn't yet), then waits for the specific invoice to reach the desired
state.
VerifyInvoiceStatusdelegates to this store, which keeps thechallenger code itself clean. The store also respects the quit channel
throughout, so shutdown during a long load is handled gracefully.
When
strictVerifyis false, we skip the entire loading and subscriptionprocess, since we don't need the invoice state map at all in that mode.
See each commit message for a detailed description w.r.t the incremental
changes.
Fixes #167