release-21.1: cli/demo: assorted backports #63535
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Before: ``` Connection parameters: (console) http://127.0.0.1:8080/demologin?password=demo63578&username=demo (sql) postgres://demo:demo63578@?host=%2Ftmp%2Fdemo916006859&port=26257 (sql/tcp) postgres://demo:[email protected]:26257?sslmode=require ``` After: ``` Connection parameters: (webui) http://127.0.0.1:8080/demologin?password=demo66299&username=demo (sql) postgres://demo:[email protected]:26257?sslmode=require (sql/unix) postgres://demo:demo66299@?host=%2Ftmp%2Fdemo869742428&port=26257 ``` Release note (cli change): The prefixes displayed before connection URLs when `cockroach demo` starts up have been updated to better align with the output of `cockroach start`.
The flag `--empty` was not, in fact, creating an empty cluster; the two base databases `defaultdb` and `postgres` are still created. Therefore the name of the flag was misleading. What the flag is really doing is disable the generation of the example database powered by the selected workload. Release note (cli change): The flag `--empty` for `cockroach demo` has been renamed to `--no-example-database`. The previous form of the flag is still recognized but is marked as deprecated. Additionally, the user can now set the env var `COCKROACH_NO_EXAMPLE_DATABASE` to obtain this behavior automatically in every new demo session.
Before this change, if you specified that the ports should be chosen by the system with `--http-port 0 --sql-port 0` then it would not work for multi-node demo. This fixes that. Release note: None
Previously, the --global flag to demo had some inconsistent behavior: some of the time, the actual "global" latencies were double what we would expect. The reason for this was that we were accidentally only introducing a delay on one of the sides of the connection, some of the time. The fix was to change a method to use a pointer receiver :) Thanks to @alf_the_n00b for noticing this one-character mistake. Release note (cli change): fix the artificial latencies introduced by the --global flag to demo.
Previously, it crashed due to not setting up the demo's rpc context to include the artificial latency map. Release note: None
This commit ensures that the server Start() method is properly sensitive to its context cancellation and possible async interruption by its stopper. Release note: None
The latency simulation code needs to be injected as a latency map while the servers are initialized, i.e. concurrently with server startup. The previous initialization code for `cockroach demo` to achieve this was exceedly difficult to understand and was, in fact, incorrect. This commit reworks this code by exposing the overall workings as a comment and then ensuring the structure of the comment follows the explanation. It also add logging. Additionally, this change ensures that the same initialization code is used regardless of whether latency simulation is requested or not. Release note: None Co-authored-by: Raphael 'kena' Poss <[email protected]> Co-authored-by: Oliver Tan <[email protected]>
In addition to the release note, reworded the add node (which already errored beforehand), and adding a clause in RestartNode. Note there is no code path that allows a restart, since shutdown is required before a node restart. Release note (cli change): No longer support `\demo add/shutdown` on cockroach demo.
Release note: None
Release note (cli change): Added a note when starting up a --global demo cluster that it is experimental.
|
This change is |
If the remote server shuts down while the connection is established, the delaying conn may not complete its handshake. Avoid a panic in that case. Release note: None
otan
approved these changes
Apr 13, 2021
There was a problem hiding this comment.
i'm assuming this is for v21.1.1, but happy for v21.1.0 if one deems it a ga blocker (not sure we have that)
Reviewed 2 of 2 files at r1, 10 of 12 files at r2, 1 of 1 files at r5, 5 of 8 files at r6, 2 of 4 files at r7, 2 of 3 files at r8, 1 of 2 files at r9, 1 of 1 files at r10, 1 of 1 files at r11.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @knz)
|
yes I can hold off merging this for now |
knz
changed the title
knz
deleted the
backport21.1-62307-62306-62391-62679-62704-63031-63057-63515
branch
This was referenced May 25, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport:
--empty->--no-example-database; add env var" (cli/demo:--empty->--no-example-database; add env var #62306)Please see individual PRs for details.
/cc @cockroachdb/release
/cc @jordanlewis