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

netgoal: fix large db generation #5445

Merged
merged 6 commits into from Jun 7, 2023

Conversation

AlgoAxel
Copy link
Contributor

@AlgoAxel AlgoAxel commented Jun 2, 2023

Introduces the following changes to support large-scale testing:

  • Reintroduction of Deterministic Account Creation during database generation in Netgoal, previously written here
  • Bugfix on the above code to resolve an issue where only one block worth of accounts could be created at maximum
  • Pacing added to netgoal generate to prevent OOM crashes if the in-memory database outpaces flushes to disk
  • Added TODO comments to two functions which I have overloaded locally (maybe worth rewriting those functions)
  • Updates BootstrappedScenario to use all of the above.

Testing

make install. This is a manual port of relevant fixes from a development branch. In that branch, these fixes are being used to produce large scale testing, and the changes introduced here were vital to unblocking issues in infrastructure.

time netgoal build -r "~/networks/deterministic" -n "deterministic" --recipe test/testdata/deployednettemplates/recipes/bootstrappedScenario/recipe.json --gen-db-files - watched this command with extra debug lines to observe the index for keypair being used and incremented as expected.

@codecov
Copy link

codecov bot commented Jun 2, 2023

Codecov Report

Merging #5445 (cda7408) into master (bad81ee) will decrease coverage by 1.23%.
The diff coverage is 7.69%.

@@            Coverage Diff             @@
##           master    #5445      +/-   ##
==========================================
- Coverage   55.40%   54.18%   -1.23%     
==========================================
  Files         447      447              
  Lines       63313    63328      +15     
==========================================
- Hits        35080    34312     -768     
- Misses      25842    26612     +770     
- Partials     2391     2404      +13     
Impacted Files Coverage Δ
netdeploy/remote/bootstrappedNetwork.go 75.00% <ø> (ø)
netdeploy/remote/deployedNetwork.go 18.58% <7.69%> (-0.31%) ⬇️

... and 71 files with indirect coverage changes

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

Copy link
Contributor

@algorandskiy algorandskiy left a comment

Choose a reason for hiding this comment

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

looks good, one suggestion about simplifying (?) keypair signature

netdeploy/remote/deployedNetwork.go Outdated Show resolved Hide resolved
netdeploy/remote/deployedNetwork.go Outdated Show resolved Hide resolved
netdeploy/remote/deployedNetwork_test.go Outdated Show resolved Hide resolved
netdeploy/remote/deployedNetwork.go Outdated Show resolved Hide resolved
@@ -159,6 +159,7 @@
],
"FeeSink": "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAY5HFKQ",
"RewardsPool": "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAY5HFKQ",
"RewardsPoolBalance": 125000000000000,
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious, was this required to get it going? Or did rerunning netgoal just generate this dirty change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did not add this in manually, it was modified by make if that's what you mean.

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we just remove this line then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, I have removed it.

* Use the netState structure to hold a counter for number of
  deterministic accounts created
* Two separate functions for keypair and deterministicKeypair
* Simplify delay loop in netdeploy generate
algorandskiy
algorandskiy previously approved these changes Jun 6, 2023
@algorandskiy algorandskiy merged commit 3fbc4b6 into algorand:master Jun 7, 2023
24 checks passed
@algorandskiy algorandskiy changed the title Fix: Large Database Generation netgoal: fix large db generation Jun 7, 2023
AlgoAxel added a commit to AlgoAxel/go-algorand that referenced this pull request Jul 5, 2023
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