Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ledger: support WaitWithCancel for unsuccessful WaitForBlock API calls #5814

Merged
merged 7 commits into from
Nov 6, 2023

Conversation

ohill
Copy link
Contributor

@ohill ohill commented Oct 30, 2023

Summary

The REST API WaitForBlockAfter call creates a notifier for each unique round requested, which remains present even if the HTTP request times out. This adds the ability to cancel and delete notifiers if no one wants them.

Test Plan

Added TestCancelWait

@ohill ohill marked this pull request as ready for review October 30, 2023 19:37
@codecov
Copy link

codecov bot commented Oct 30, 2023

Codecov Report

Merging #5814 (668f797) into master (40c16e5) will increase coverage by 1.10%.
Report is 16 commits behind head on master.
The diff coverage is 59.09%.

@@            Coverage Diff             @@
##           master    #5814      +/-   ##
==========================================
+ Coverage   54.45%   55.55%   +1.10%     
==========================================
  Files         475      475              
  Lines       66848    66860      +12     
==========================================
+ Hits        36399    37144     +745     
+ Misses      27903    27194     -709     
+ Partials     2546     2522      -24     
Files Coverage Δ
ledger/bulletin.go 94.64% <100.00%> (+2.33%) ⬆️
ledger/ledger.go 68.36% <0.00%> (+1.29%) ⬆️
daemon/algod/api/server/v2/handlers.go 0.77% <0.00%> (-0.01%) ⬇️

... and 84 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

algorandskiy
algorandskiy previously approved these changes Oct 30, 2023
jannotti
jannotti previously approved these changes Oct 31, 2023
@ohill ohill dismissed stale reviews from jannotti and algorandskiy via 668f797 October 31, 2023 19:46
@ohill
Copy link
Contributor Author

ohill commented Oct 31, 2023

Updated to remove unnecessary atomic CompareAndSwap (since notifier is only ever used under lock by bulletin). I think the original purpose of notifier's atomic operation was in case it would ever be used by other types besides bulletin, but it is only used by bulletin today.

@ohill ohill requested a review from winder November 6, 2023 15:51
@algorandskiy algorandskiy merged commit 5a2ef5e into algorand:master Nov 6, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants