Repository navigation
Conversation
|
👋 Thanks for assigning @joostjager as a reviewer! |
d393f3e to
ce0d9cf
Compare
ajaysehwal
left a comment
ajaysehwal
left a comment
There was a problem hiding this comment.
@tnull , one gap in thorough test suite: there’s no test covering a sender that gets rate-limited once and then successfully retries after the bucket refills.
Not required for correctness — the existing tests already show that the rate-limited request is dropped cleanly without affecting other requests. But a small test using a short refill interval or deterministic bucket advancement would make the intended contract explicit: the request is dropped, and the sender is free to retry later rather than being blocked permanently.
Purely a documentation-via-test nice-to-have.
ce0d9cf to
264852e
Compare
| self.channel_manager.static_invoice_persisted(invoice_persisted_path); | ||
| }, | ||
| Err(e) => { | ||
| Err(StaticInvoiceStoreError::RateLimited) => { |
There was a problem hiding this comment.
Don't we want to skip the rate limiter once we are retrying persistence, so that we don't exit the retry loop because new requests came in?
There was a problem hiding this comment.
Added a fixup for this.
| } | ||
|
|
||
| #[tokio::test(flavor = "multi_thread", worker_threads = 2)] | ||
| async fn static_invoice_read_error_is_replayed() { |
There was a problem hiding this comment.
Quite a bit of test code. Can't AI come up with something more compact with still reasonable coverage?
Drop static invoice requests when the rate limit is reached instead of retrying them and delaying later events. Keep retrying storage errors and only acknowledge invoices after saving them. Fixes an issue reported by Ajay Sehwal. Co-Authored-By: HAL 9000
Storage retries consumed another rate-limit token on every attempt. Repeated failures could exhaust the bucket and silently drop an event that had already been admitted. Preserve admission across storage failures so requests can recover without losing their place in the event queue. Successful operations still consume quota, and regression tests cover retry recovery as well as silent rejection and later admission after refill. Co-Authored-By: HAL 9000
264852e to
0033e6b
Compare
4 participants
Drop static invoice requests when the rate limit is reached instead of retrying them and delaying later events. Keep retrying storage errors and only acknowledge invoices after saving them.
Fixes an issue reported by @ajaysehwal.