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

Algod: Simulation run with increased limits on logs #5247

Merged
merged 60 commits into from
Apr 24, 2023

Conversation

ahangsu
Copy link
Contributor

@ahangsu ahangsu commented Mar 31, 2023

Summary

Introduction of simulation run without limit on logs, issue #5230

This PR should proceed after introduction of #5253, which allowed sending request with raw body together with query params, and following things need to be done:

  • Currently, the log opcode related limitations (maxLogSize = 1024, maxLogCalls = 32) are hardcoded in binary.
    We might want to move them as part of consensus param? Except for consensus param, I can't think of a better location to let these constant reside.
    These constraints stay as is.
  • Introduce a set of simulation related log limits (corresponding to the consensus params above) in localConfig.go, which is per-node configuration.
  • On initializing a new EvalParam, swap in the 2 constraints for running these opcodes, and use tracer to override the constraints through local config for running simulation (conditioned that we are unlimiting log in endpoint call).

Test Plan

Test in e2e-script and restClient_test.go

@ahangsu ahangsu changed the title Simulation log unlimited Alogd: Simulation run without limit on logs Mar 31, 2023
@codecov
Copy link

codecov bot commented Mar 31, 2023

Codecov Report

Merging #5247 (a930b5a) into master (f4f5ec6) will increase coverage by 0.01%.
The diff coverage is 42.30%.

❗ Current head a930b5a differs from pull request most recent head 42ee635. Consider uploading reports for the commit 42ee635 to get more accurate results

@@            Coverage Diff             @@
##           master    #5247      +/-   ##
==========================================
+ Coverage   53.78%   53.79%   +0.01%     
==========================================
  Files         450      450              
  Lines       56201    56218      +17     
==========================================
+ Hits        30226    30245      +19     
- Misses      23626    23631       +5     
+ Partials     2349     2342       -7     
Impacted Files Coverage Δ
config/localTemplate.go 63.26% <ø> (ø)
daemon/algod/api/server/v2/handlers.go 0.82% <0.00%> (-0.01%) ⬇️
libgoal/libgoal.go 2.51% <0.00%> (ø)
node/follower_node.go 26.08% <ø> (ø)
node/node.go 4.14% <0.00%> (ø)
ledger/simulation/tracer.go 86.66% <20.00%> (-4.89%) ⬇️
cmd/goal/clerk.go 9.23% <50.00%> (+0.12%) ⬆️
data/transactions/logic/eval.go 90.17% <66.66%> (-0.08%) ⬇️
daemon/algod/api/server/v2/test/helpers.go 75.67% <100.00%> (ø)
ledger/simulation/simulator.go 83.82% <100.00%> (+0.24%) ⬆️

... and 11 files with indirect coverage changes

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

@ahangsu ahangsu changed the title Alogd: Simulation run without limit on logs Algod: Simulation run without limit on logs Apr 3, 2023
Copy link
Member

@jasonpaulos jasonpaulos left a comment

Choose a reason for hiding this comment

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

Looking good, I only have pretty minor comments

ledger/simulation/trace.go Outdated Show resolved Hide resolved
ledger/simulation/trace.go Outdated Show resolved Hide resolved
ledger/simulation/trace.go Outdated Show resolved Hide resolved
ledger/simulation/simulation_eval_test.go Show resolved Hide resolved
daemon/algod/api/algod.oas2.json Outdated Show resolved Hide resolved
ledger/simulation/simulation_eval_test.go Show resolved Hide resolved
ledger/simulation/simulator.go Show resolved Hide resolved
jasonpaulos
jasonpaulos previously approved these changes Apr 21, 2023
Copy link
Member

@jasonpaulos jasonpaulos left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for addressing all my concerns!

@algorand algorand deleted a comment from jasonpaulos Apr 24, 2023
Copy link
Contributor

@bbroder-algo bbroder-algo left a comment

Choose a reason for hiding this comment

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

"unlimited" usefulness

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