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

Allow genesis tool to specify shards to track #1726

Merged
merged 5 commits into from Nov 20, 2019

Conversation

bowenwang1996
Copy link
Collaborator

Also fix the minor issue that genesis tool requires validator_key.json to exist.

@codecov
Copy link

codecov bot commented Nov 19, 2019

Codecov Report

Merging #1726 into staging will decrease coverage by 0.13%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           staging    #1726      +/-   ##
===========================================
- Coverage    80.75%   80.61%   -0.14%     
===========================================
  Files          132      132              
  Lines        25421    25352      -69     
===========================================
- Hits         20528    20437      -91     
- Misses        4893     4915      +22
Impacted Files Coverage Δ
near/src/runtime.rs 72.9% <ø> (+0.11%) ⬆️
chain/chain/src/test_utils.rs 77.26% <0%> (-7.51%) ⬇️
core/primitives/src/receipt.rs 59.09% <0%> (-6.82%) ⬇️
chain/client/tests/cross_shard_tx.rs 17.84% <0%> (-1.25%) ⬇️
chain/client/tests/catching_up.rs 8.82% <0%> (-1.09%) ⬇️
...enesis-tools/genesis-csv-to-json/src/csv_parser.rs 68.34% <0%> (-0.43%) ⬇️
near/tests/sync_state_nodes.rs 55.55% <0%> (-0.41%) ⬇️
runtime/runtime/src/state_viewer.rs 89.55% <0%> (-0.38%) ⬇️
chain/network/src/types.rs 77.55% <0%> (-0.21%) ⬇️
near/tests/track_shards.rs 94.59% <0%> (-0.15%) ⬇️
... and 18 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8528f20...60dd536. Read the comment docs.

.get_matches();

let home_dir = matches.value_of("home").map(|dir| Path::new(dir)).unwrap();
let chain_id = matches.value_of("chain-id").expect("Chain id is requried");
csv_to_json_configs::csv_to_json_configs(home_dir, chain_id.to_string());
let tracked_shards: HashSet<ShardId> = match matches.value_of("tracked-shards") {
Copy link
Contributor

Choose a reason for hiding this comment

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

How is this arg used? The usage of this command line tool is to allow a group of participants who share the same csv file to create the same genesis config. Are they all going to track the same set of shards? Will this set of shards include all shards existing, if not who is going to track the remaining shards?

Copy link
Contributor

Choose a reason for hiding this comment

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

Please add explanation to the arg's description.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Tracked shards are independent of genesis. They are specified by the node if the node wants to always track some shards. I'll add an explanation.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you also answer:

Are they all going to track the same set of shards? Will this set of shards include all shards existing, if not who is going to track the remaining shards?

Copy link
Contributor

Choose a reason for hiding this comment

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

And add this explanation to the arg description.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No. As I said tracked-shards is specified by the node. If a node wants to track some shards, it is their decision. It is not related in any way to how any other nodes make this decision.

Copy link
Contributor

Choose a reason for hiding this comment

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

How do people decide what shards to track? Let's say we have stake wars participants, how does participant Alice decide that she wants to track some set of shards, e.g. "1,5,6"? How do they know what is the total number of shards?

.get_matches();

let home_dir = matches.value_of("home").map(|dir| Path::new(dir)).unwrap();
let chain_id = matches.value_of("chain-id").expect("Chain id is requried");
csv_to_json_configs::csv_to_json_configs(home_dir, chain_id.to_string());
let tracked_shards: HashSet<ShardId> = match matches.value_of("tracked-shards") {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you also answer:

Are they all going to track the same set of shards? Will this set of shards include all shards existing, if not who is going to track the remaining shards?

.get_matches();

let home_dir = matches.value_of("home").map(|dir| Path::new(dir)).unwrap();
let chain_id = matches.value_of("chain-id").expect("Chain id is requried");
csv_to_json_configs::csv_to_json_configs(home_dir, chain_id.to_string());
let tracked_shards: HashSet<ShardId> = match matches.value_of("tracked-shards") {
Copy link
Contributor

Choose a reason for hiding this comment

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

How do people decide what shards to track? Let's say we have stake wars participants, how does participant Alice decide that she wants to track some set of shards, e.g. "1,5,6"? How do they know what is the total number of shards?

@bowenwang1996
Copy link
Collaborator Author

It is completely up to the discretion of the person running the node to track shards. If they want to track any shard for whatever reason, they can. I am just providing a convenient way for people to do it so that they don't have to modify config file after it is generated. Until we have dynamic resharding, the number of shards is fixed (set in genesis).

@nearprotocol-bulldozer nearprotocol-bulldozer bot merged commit e7b430e into staging Nov 20, 2019
@nearprotocol-bulldozer nearprotocol-bulldozer bot deleted the track-shards-for-genesis branch November 20, 2019 00:00
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.

None yet

2 participants