Skip to content

SCRIPT: Updated script for multiple goroutines#5

Merged
maoueh merged 3 commits intomasterfrom
david-zhou/concurrency-2
Jul 10, 2025
Merged

SCRIPT: Updated script for multiple goroutines#5
maoueh merged 3 commits intomasterfrom
david-zhou/concurrency-2

Conversation

@Rampex1
Copy link
Contributor

@Rampex1 Rampex1 commented May 15, 2025

Changes Introduced

Configuration path to allow multiple goroutines concurrency

  • ./scripts/run_firehose_geth_dev.sh 3.0 prague (original config)
  • CONCURRENT_BLOCK_FLUSHING=10 ./scripts/run_firehose_geth_dev.sh 3.0 prague (with concurrency)

Notes

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the geth development script to enable configurable concurrency for block flushing by introducing a new environment variable.

  • Introduces validation for the CONCURRENT_BLOCK_FLUSHING environment variable.
  • Updates the JSON configuration for vmtrace to include the concurrentBlockFlushing parameter.
Comments suppressed due to low confidence (1)

scripts/wrapped_geth_dev.sh:46

  • Ensure that if forced_backward_compatibility is non-empty, it includes a leading comma so that the resulting JSON configuration is valid.
json_config="{"applyBackwardCompatibility":${backward_compatibility},"concurrentBlockFlushing":${concurrent_block_flushing}${forced_backward_compatibility}}"

Copy link
Contributor

@maoueh maoueh left a comment

Choose a reason for hiding this comment

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

Let's wait on other PR review to decide on a final name here.

Comment on lines +24 to +25
# CONCURRENT_BLOCK_FLUSHING=true ./scripts/run_firehose_geth_dev.sh 3.0 prague
concurrent_block_flushing="${CONCURRENT_BLOCK_FLUSHING:-0}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Now with a count, the variable is really not aligned anymore, we will need to find a better name as there is actually no concurrent flushing.

The value controls are many worker pick from the queue correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct. 0 means 0 workers, so no concurrency whereas 100 would mean 100 concurrent workers

@maoueh maoueh merged commit 7cbb971 into master Jul 10, 2025
@maoueh maoueh deleted the david-zhou/concurrency-2 branch July 10, 2025 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants