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

tools: block generator #5245

Merged
merged 8 commits into from
Mar 31, 2023
Merged

Conversation

shiqizng
Copy link
Contributor

@shiqizng shiqizng commented Mar 30, 2023

adding block generator to /tools. the code is copied from an internal tools repo. Some fixes were needed to pass the CI.

@shiqizng shiqizng marked this pull request as draft March 30, 2023 18:03
@codecov
Copy link

codecov bot commented Mar 31, 2023

Codecov Report

Merging #5245 (1ce7969) into master (19862ba) will increase coverage by 0.06%.
The diff coverage is 60.04%.

@@            Coverage Diff             @@
##           master    #5245      +/-   ##
==========================================
+ Coverage   53.52%   53.59%   +0.06%     
==========================================
  Files         441      446       +5     
  Lines       55140    55633     +493     
==========================================
+ Hits        29513    29814     +301     
- Misses      23339    23515     +176     
- Partials     2288     2304      +16     
Impacted Files Coverage Δ
tools/block-generator/generator/server.go 26.15% <26.15%> (ø)
tools/block-generator/generator/generate.go 59.60% <59.60%> (ø)
tools/block-generator/generator/daemon.go 60.00% <60.00%> (ø)
...ols/block-generator/generator/make_transactions.go 100.00% <100.00%> (ø)
tools/block-generator/generator/utils.go 100.00% <100.00%> (ø)

... and 12 files with indirect coverage changes

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

@@ -68,3 +68,6 @@ index.html

# test summary
testresults.json

# block generator binary
tools/block-generator/block-generator
Copy link
Contributor Author

Choose a reason for hiding this comment

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

added this to gitignore.

@shiqizng shiqizng marked this pull request as ready for review March 31, 2023 15:28
Copy link
Contributor

@Eric-Warehime Eric-Warehime left a comment

Choose a reason for hiding this comment

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

I think we should merge as-is since the build is passing, and for the most part this is a straight port from the original. So I haven't gone through every bit of the logic--we can continue updating/changing that as needed when we start working on the logic updates we need.

Handler: mux,
Addr: addr,
Handler: mux,
ReadHeaderTimeout: 3 * time.Second,
Copy link
Contributor

@tzaffi tzaffi Mar 31, 2023

Choose a reason for hiding this comment

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

Is there any recovery in case of timeout? Or that will end up killing the run?

@@ -90,7 +90,7 @@ func (r *Args) run() error {
}

// This middleware allows us to lock the block endpoint
var freezeMutex sync.Mutex
var freezeMutex deadlock.Mutex
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious to understand the benefits of deadlock's Mutex over sync's in this context.

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe it's just for the deadlock detection feature, you can see it configured here: https://github.com/algorand/go-algorand/blob/master/cmd/algod/main.go#L269

Comment on lines +36 to +38
if err != nil {
return
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This segment seems redundant as err is now set and return is happening immediately after.

Copy link
Contributor

Choose a reason for hiding this comment

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

(not clear in this context, I'm referring to lines 36-38)

Copy link
Contributor

@tzaffi tzaffi left a comment

Choose a reason for hiding this comment

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

Basically appprovable, but I left a couple of questions.

@@ -0,0 +1,115 @@
# Block Generator

This tool is used for testing Indexer import performance. It does this by generating synthetic blocks which are sent by mocking the Algod REST API endpoints that Indexer uses.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This tool is used for testing Indexer import performance. It does this by generating synthetic blocks which are sent by mocking the Algod REST API endpoints that Indexer uses.
This tool is used for testing Conduit import performance. It does this by generating synthetic blocks which are sent by mocking the Algod REST API endpoints that Indexer uses.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's save these for the next PR

@winder winder changed the title tools: block generator tool tools: block generator Mar 31, 2023
@winder winder merged commit d27b674 into algorand:master Mar 31, 2023
@shiqizng shiqizng deleted the shiqi/block-generator branch April 3, 2023 15:28
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