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

rest api: Allow fast track transaction broadcasting via txHandler #5535

Merged
merged 4 commits into from Aug 8, 2023

Conversation

algorandskiy
Copy link
Contributor

@algorandskiy algorandskiy commented Jul 10, 2023

Summary

Added a new RawTransactionsAsync handler (enabled with ExperimentalAPI) that shortcuts to non-blocking txHandler call. This is useful for local pingpong high TPS testing. (Although real high TPS is still blocked by Echo's logging middleware, but this is a separate topic).

This handler is used in pingpong via libgoal.
Also set EnableExperimentalAPI=true for local network templates and scenario1s.

Test Plan

Tested locally with pingpong, 2.5 TPS observed out of 3k requested - here are the data for 2m run:

pingpong run -d ~/networks/two-fut/Primary --tps 3000 --numaccounts 50 --refresh 3700 --async --quiet --run 120
...
16550 sent, 3306.68/s (16550 total) (16600 sc 15112 sts)
13950 sent, 2731.90/s (30500 total) (30550 sc 30432 sts)
16150 sent, 3298.26/s (46650 total) (46700 sc 45121 sts)
14150 sent, 2755.04/s (60800 total) (60850 sc 60529 sts)
16250 sent, 3340.10/s (77050 total) (77100 sc 75125 sts)
13950 sent, 2760.98/s (91000 total) (91050 sc 90219 sts)

image

winder
winder previously requested changes Jul 10, 2023
Copy link
Contributor

@winder winder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be a new endpoint that isn't exposed to the public API / SDKs? I believe tagging it "experimental" will opt it out of the downstream code generation.

@algorandskiy algorandskiy dismissed winder’s stale review July 10, 2023 19:53

Fixed as requested

@algorandskiy algorandskiy requested a review from winder July 10, 2023 19:53
winder
winder previously approved these changes Jul 10, 2023
Copy link
Contributor

@winder winder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the change. LGTM.

daemon/algod/api/algod.oas2.json Show resolved Hide resolved
@algorandskiy algorandskiy marked this pull request as ready for review July 12, 2023 15:16
@codecov
Copy link

codecov bot commented Jul 12, 2023

Codecov Report

Merging #5535 (ad3c8d7) into master (5acab71) will decrease coverage by 0.01%.
Report is 1 commits behind head on master.
The diff coverage is 12.19%.

@@            Coverage Diff             @@
##           master    #5535      +/-   ##
==========================================
- Coverage   54.94%   54.93%   -0.01%     
==========================================
  Files         463      463              
  Lines       64476    64515      +39     
==========================================
+ Hits        35427    35444      +17     
- Misses      26668    26692      +24     
+ Partials     2381     2379       -2     
Files Changed Coverage Δ
daemon/algod/api/server/v2/handlers.go 0.81% <0.00%> (-0.01%) ⬇️
data/txHandler.go 71.73% <0.00%> (-1.40%) ⬇️
libgoal/transactions.go 0.00% <0.00%> (ø)
netdeploy/networkTemplate.go 42.85% <0.00%> (-0.27%) ⬇️
node/follower_node.go 34.10% <0.00%> (+1.54%) ⬆️
node/node.go 4.14% <0.00%> (-0.02%) ⬇️
shared/pingpong/config.go 0.00% <ø> (ø)
shared/pingpong/pingpong.go 0.00% <0.00%> (ø)
daemon/algod/api/server/v2/test/helpers.go 77.04% <100.00%> (+0.47%) ⬆️

... and 7 files with indirect coverage changes

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

@algorandskiy algorandskiy changed the title rest api: Allow fast track transaction broadcast rest api: Allow fast track transaction broadcasting via txHandler Jul 12, 2023
AlgoAxel
AlgoAxel previously approved these changes Jul 12, 2023
@algorandskiy
Copy link
Contributor Author

merged master and regenerated routes

data/txHandler.go Show resolved Hide resolved
node/node.go Show resolved Hide resolved
AlgoAxel
AlgoAxel previously approved these changes Jul 19, 2023
Copy link
Contributor

@AlgoAxel AlgoAxel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See cce's comment about the function naming being somewhat confusing, but this appears to be the same as when I previously approved.

winder
winder previously approved these changes Aug 1, 2023
@algorandskiy algorandskiy dismissed stale reviews from winder and AlgoAxel via d57fc52 August 4, 2023 17:38
@algorandskiy
Copy link
Contributor Author

rebased master

@algorandskiy algorandskiy merged commit 39ad943 into algorand:master Aug 8, 2023
21 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.

None yet

4 participants