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

Fix shard ranges in seedtos3.sh #15

Merged
merged 1 commit into from
May 7, 2024
Merged

Conversation

darbha-ram
Copy link
Contributor

The shard_seeder tool (in opencbdc-tx) computes shard range as 256/#shards, i.e., rounds down the range. This script computes it as 255/#shards +1, i.e., rounds it up. This doesn't matter if num shards is a power of 2, but for other values, the tool and script differ, causing errors in seed generation. The quick fix in this commit is to calculate the range by rounding down, as the tool does, and to ensure that the last range is extended, if needed, to the 1-byte max or 255.

The shard_seeder tool (in opencbdc-tx) computes shard range
as 256/#shards, i.e., rounds down the range. This script computes
it as 255/#shards +1, i.e., rounds it up. This doesn't matter if
num shards is a power of 2, but for other values, the tool and script
differ, causing errors in seed generation. The quick fix in this
commit is to calculate the range by rounding down, as the tool
does, and to ensure that the last range is extended, if needed,
to the 1-byte max or 255.

Signed-off-by: Ram Darbha <[email protected]>
@HalosGhost
Copy link
Collaborator

concept ACK!

This has been a small thorn in our side for a while (it practically limits pre-seeded tests from the test controller to power-of-two logical shard counts).

We discussed this offline, and the longer-term fix is to unify all ancillary tooling (this script as well as the shard-seeder tool in -tx, and any others) to take the ranges listed in the specified configuration as authoritative (note: this might lead to an introduced error case if the full range of bytes is not covered).

Till then, this is an excellent quick-fix to allow testing of less-even shard topologies to go ahead unblocked.

Will try to test/approve/merge early next week!

@HalosGhost
Copy link
Collaborator

(I'm working to close opencbdc-tx#258 since we're actively testing it, before redeploying our TC to test this. Sorry for the delay!)

Copy link
Collaborator

@HalosGhost HalosGhost left a comment

Choose a reason for hiding this comment

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

Testing for #258 is a touch delayed, so I had a chance to redeploy the TC and test this without interrupting other things.

tACK! Brings the range-calculation in the TC's script inline with the one used by the shard-seeder tool (avoiding silly seed name-mismatching errors).

Merging!

@HalosGhost HalosGhost merged commit 4be5efe into mit-dci:trunk May 7, 2024
1 check passed
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.

2 participants