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

Recipe for network testing #1322

Merged
merged 22 commits into from
Sep 3, 2020

Conversation

egieseke
Copy link
Contributor

Summary

Added a new recipe for performing network disruption tests.

Test Plan

Testing using Algonet.

@@ -0,0 +1,12 @@
PARAMS=-w 100 -R 8 -N 20 -n 100 -H 10 --node-template node.json --relay-template relay.json --non-participating-node-template nonPartNode.json
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need such a big network to conduct a partitioning test ?
I don't really see how the large network would help us getting the answer "does the network recover after partitioning? and after how long ?"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can adjust the network to be smaller. The github tickets ask for all 3 scenarios.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's true. It's that I just don't want you to get delayed by "deployment issues" that aren't related to the actual test you're trying to conduct.. Having the feedback that it's broken on small network is more valuable than the fact the it's broken on a large network ( and would probably be easier to evaluate )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, we will start testing tomorrow.

@egieseke egieseke self-assigned this Jul 31, 2020
@egieseke egieseke added this to the Sprint 6 milestone Jul 31, 2020
@egieseke egieseke marked this pull request as ready for review July 31, 2020 00:16
Copy link
Contributor

@tsachiherman tsachiherman left a comment

Choose a reason for hiding this comment

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

few requests, looks good otherwise.


group := strings.TrimSpace(cloudHost.Group)
if group == "" {
hostSpec.Group = "default"
Copy link
Contributor

Choose a reason for hiding this comment

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

For backward compatibility, you might want to leave the default as an empty string.
This would force the cloudspec file consumer to populate it with "default" instead of expecting a non-empty string.
( i.e. currently, all cloud-spec files doesn't have this field. )

If you're going to break backward compatibility, please add versioning to the cloud spec file so we can detect that forward.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Leave default as an empty string.

Comment on lines 68 to 70
# for i in range(10):
# partition_name = get_partition(ranges)
# print(i, partition_name)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: delete deadcode

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed.

docker/build/Dockerfile-deploy Outdated Show resolved Hide resolved
"EnableMetrics": false,
"MetricsURI": "{{MetricsURI}}",
"ConfigJSONOverride": "{ \"TxPoolExponentialIncreaseFactor\": 1, \"DNSBootstrapID\": \"<network>.algodev.network\", \"DeadlockDetection\": -1, \"PeerPingPeriodSeconds\": 30, \"BaseLoggerDebugLevel\": 4, \"EnableProfiler\": true }",
"AltConfigs": [
Copy link

Choose a reason for hiding this comment

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

Nit: spacing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed extra white space.

@egieseke egieseke requested a review from btoll August 7, 2020 12:07
@egieseke egieseke requested a review from tsachiherman August 7, 2020 12:07
@ian-algorand ian-algorand modified the milestones: Sprint 6, Sprint 7 Aug 10, 2020
Copy link
Contributor

@onetechnical onetechnical left a comment

Choose a reason for hiding this comment

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

I can't give explicit approval here because I don't know this tool well enough...would take time to get familiar.

docker/build/Dockerfile-deploy Outdated Show resolved Hide resolved
@egieseke egieseke requested a review from onetechnical August 20, 2020 20:14
@tsachiherman tsachiherman dismissed their stale review August 21, 2020 00:41

Chanes rolled back

@algojohnlee algojohnlee merged commit 107e9c0 into algorand:master Sep 3, 2020
tsachiherman pushed a commit to tsachiherman/go-algorand that referenced this pull request Jul 7, 2021
Added a new recipe for performing network disruption tests.
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.

7 participants