-
Notifications
You must be signed in to change notification settings - Fork 453
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
Add setup for multi node/multi-machine testing #2044
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2044 +/- ##
========================================
+ Coverage 65.8% 72.4% +6.6%
========================================
Files 1001 1007 +6
Lines 85967 86460 +493
========================================
+ Hits 56582 62614 +6032
+ Misses 25300 19676 -5624
- Partials 4085 4170 +85
Continue to review full report at Codecov.
|
Great work, looking forward to testing our latest changes on HEAD vs last release (0.14.2). |
4dced99
to
89cf1b0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bunch of nits, but looks good generally and happy to approve if you'd prefer to deal with them async
scripts/vagrant/multi/README.md
Outdated
- kubernetes (using kind) | ||
- etcd (single node) | ||
- m3db operator | ||
- m3db node (single node) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Should this be multiple nodes?
scripts/vagrant/single/README.md
Outdated
@@ -0,0 +1,71 @@ | |||
# Vagrant | |||
|
|||
This allows you to run a Kubernetes environment on a single box using vagrant to provision a VM either local or in a cloud environment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: This runs a Kubernetes environment on a single box, using vagrant to provision a VM either locally, or in a cloud environment.
scripts/vagrant/single/README.md
Outdated
|
||
# GCP setup | ||
|
||
If you authorized with `gcloud` you can use `~/.ssh/google_compute_engine` as your SSH key. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: "After authorizing with gcloud
, use ~/.ssh/google_compute_engine
as the SSH key."
|
||
set -xe | ||
|
||
# Perform rollign restarts of db nodes forever. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: rolling
scripts/vagrant/multi/README.md
Outdated
# Vagrant | ||
|
||
This allows you to run a Kubernetes environment on multiple boxes using vagrant to provision VMs either locally or in a cloud environment. | ||
Let's us compare a feature branch to release by specifying a `FEATURE_DOCKER_IMAGE` env variable. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
This allows comparisons between a feature branch (specified by a `FEATURE_DOCKER_IMAGE` environment variable), and `latest`.
scripts/vagrant/multi/README.md
Outdated
|
||
# Requirements | ||
Setup vagrant azure provider via [docs](https://github.com/Azure/vagrant-azure). | ||
Or altertnatively set up google provider via [docs](https://github.com/mitchellh/vagrant-google). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: alternatively
scripts/vagrant/multi/Vagrantfile
Outdated
# For heavy benchmarks, use pre-emptible n1-standard-64: | ||
#google.machine_type = "n1-standard-64" | ||
#google.preemptible = true | ||
#google.auto_restart = false | ||
#google.on_host_maintenance = "TERMINATE" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we key this on an env variable?
fi | ||
|
||
# Create ingress rules if not already exists. | ||
MAYBE_M3COORDINATOR_INGRESS=$(gcloud --project=studious-saga-237001 compute firewall-rules list 2> /dev/null | tail -n +2 | awk '{ print $1 };' | grep default-allow-m3coordinator) || true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More of a question, what is studious-saga-237001
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the "Hosted Project" gcloud project, it seems to be the only one not attributed to a chronosphere environment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it ok to expose this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, needed to expose this for traffic to be allowed from the node running the benchmarker and the coordinators. It's for testing anyways.
358a450
to
5ed21c5
Compare
@@ -112,6 +112,9 @@ func newFileSystemSource(opts Options) bootstrap.Source { | |||
} | |||
s.newReaderPoolOpts.alloc = s.newReader | |||
|
|||
s.log.Info("newFileSystemSource BoostrapIndexNumProcessors", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Think this may have snuck in from the other PR
@@ -46,7 +46,7 @@ const ( | |||
// attempt to divide a mutable index block into when flushing the index | |||
// block. The fewer the blocks the better for searches, but the more memory | |||
// intensive it is to build at runtime. | |||
DefaultFlushIndexBlockNumSegments = 4 | |||
DefaultFlushIndexBlockNumSegments = 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Think this may have snuck in from the other PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What this PR does / why we need it:
Modifies
scripts/vagrant
to support both single and multi node testing. Multi node testing allows us to compare feature branches to release for both performance/stability testing.Special notes for your reviewer:
This also adds additional environment variables to promremotebench's config.
PROMREMOTEBENCH_SECONDARY_TARGET
PROMREMOTEBENCH_SECONDARY_QUERY_TARGET
Does this PR introduce a user-facing and/or backwards incompatible change?:
Does this PR require updating code package or user-facing documentation?: