-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
demo: fix --global option #58466
demo: fix --global option #58466
Conversation
Still no tests for this, sorry. |
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.
Nice! Now that we handle the ArtificialLatencyMap
knob in grpcDialOptions
, do we still need to also handle the knob in grpcDialRaw
? Seems like the latter calls the former.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @ajstorm and @nvanbenschoten)
Tests?! Rename https://github.com/cockroachdb/cockroach/blob/master/pkg/cli/interactive_tests/test_demo_global.tcl.disabled to |
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.
I'm with Nathan - Nice!
Any chance you could add this to your PR? I wasn't able to test with the --global
flag when I built add node
, and realized in testing your change that the error message doesn't print out correctly.
index bebc3b0abf..27785de1b7 100644
--- a/pkg/cli/sql.go
+++ b/pkg/cli/sql.go
@@ -562,7 +562,7 @@ func (c *cliState) handleDemoAddNode(cmd []string, nextState, errState cliStateE
}
if demoCtx.simulateLatency {
- fmt.Printf("add command is not supported in --global configurations")
+ fmt.Printf("add command is not supported in --global configurations\n")
return nextState
}```
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @ajstorm)
The --global option broke somewhere during the 20.2 cycle. This patch revives it. --global simulates geo-latencies between nodes. Best used with `--global --geo-partitioned-replicas`. Release note (cli change): fix cockroach demo --global from crashing with "didn't get expected magic bytes header".
Release note: None
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.
Build failed: |
bors r+ |
Build failed: |
that's a flake bors r+ |
Build succeeded: |
The --global option broke somewhere during the 20.2 cycle. This patch
revives it. --global simulates geo-latencies between nodes. Best used
with
--global --geo-partitioned-replicas
.Release note (cli change): fix cockroach demo --global from crashing
with "didn't get expected magic bytes header".