server: better error message for tsdump initialization#75880
server: better error message for tsdump initialization#75880craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
This change is |
|
How this looks on the command line: |
rimadeodhar
left a comment
There was a problem hiding this comment.
LG. Left some minor comments.
Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @liamgillies)
-- commits, line 10 at r1:
I think the release note can be mapped to cli change.
pkg/server/import_ts.go, line 134 at r1 (raw file):
"Then export the created file: export COCKROACH_DEBUG_TS_IMPORT_MAPPING_FILE=tsdump.gob.yaml\n \n" + "For more information, please visit " + "https://cockroachlabs.atlassian.net/wiki/spaces/CKB/pages/2168520776/Exporting+Internal+Timeseries+Detailed")
Remove these lines. We shouldn't reference internal wiki links for customer facing messages :).
Code quote:
"For more information, please visit " +
"https://cockroachlabs.atlassian.net/wiki/spaces/CKB/pages/2168520776/Exporting+Internal+Timeseries+Detailed")
dhartunian
left a comment
There was a problem hiding this comment.
pro tip: when pasting command line output in github (or any code/output anywhere), please use the three backticks before and after and paste as text so it gets formatted as code like this:
> grep stuff blah.go
output
etc
etc
it keeps the data searchable and quotable :)
typically we only use screenshots for UI
Reviewable status:
pkg/server/import_ts.go, line 127 at r1 (raw file):
if knobs.ImportTimeseriesMappingFile == "" { return errors.Errorf("need to specify COCKROACH_DEBUG_TS_IMPORT_MAPPING_FILE; it should point at " + "a YAML file that maps StoreID to NodeID. To create from a tsdump.gob file, run the following command:\n \n" +
This was a bit confusing for me to read because the command does not reference a tsdump.gob file below. Maybe this should say "To generate from the source cluster:" since it involves connecting via SQL.
pkg/server/import_ts.go, line 128 at r1 (raw file):
return errors.Errorf("need to specify COCKROACH_DEBUG_TS_IMPORT_MAPPING_FILE; it should point at " + "a YAML file that maps StoreID to NodeID. To create from a tsdump.gob file, run the following command:\n \n" + "roachprod sql $USER-ui:1 -- --format tsv -e \\\n \"select concat(store_id::string, ': ', node_id::string)" +
This example should use cockroach sql instead of roachprod if it's customer-facing. You should be able to try it out by running:
cockroach demo --nodes 3
and then running the sql command in a separate terminal. The sql command will require you to use the URL from the demo command output if you want to test it. Use the URL from the demo command output under (sql/unix) like this:
cockroach sql --url "<the url from demo>" <the rest of the sql command that was sent to roachprod before>
that should replicate the roachprod query.
Beforehand, the error message for tsdump initialization was unclear and didn't provide enough information to support engineers on how to fix it. To address this, the error message has been revamped with instructions and commands to get tsdump working. Release note (cli change): Added instructions to an error message when initializing tsdump.
92d0d6b to
a7a4c28
Compare
liamgillies
left a comment
There was a problem hiding this comment.
Reviewable status:
Previously, rimadeodhar (Rima Deodhar) wrote…
I think the release note can be mapped to cli change.
Done.
pkg/server/import_ts.go, line 127 at r1 (raw file):
Previously, dhartunian (David Hartunian) wrote…
This was a bit confusing for me to read because the command does not reference a
tsdump.gobfile below. Maybe this should say"To generate from the source cluster:"since it involves connecting via SQL.
Done.
pkg/server/import_ts.go, line 128 at r1 (raw file):
Previously, dhartunian (David Hartunian) wrote…
This example should use
cockroach sqlinstead ofroachprodif it's customer-facing. You should be able to try it out by running:
cockroach demo --nodes 3and then running the sql command in a separate terminal. The sql command will require you to use the URL from the demo command output if you want to test it. Use the URL from the demo command output under
(sql/unix)like this:cockroach sql --url "<the url from demo>" <the rest of the sql command that was sent to roachprod before>that should replicate the roachprod query.
Done.
pkg/server/import_ts.go, line 134 at r1 (raw file):
Previously, rimadeodhar (Rima Deodhar) wrote…
Remove these lines. We shouldn't reference internal wiki links for customer facing messages :).
Done.
dhartunian
left a comment
There was a problem hiding this comment.
Reviewable status:
rimadeodhar
left a comment
There was a problem hiding this comment.
Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status:
|
bors r+ |
75560: sql: allow non-ALL privileges to be granted WITH GRANT OPTION r=rafiss a=ecwall 75762: sql,roachpb: add plan hash correct value to persisted stats r=maryliag a=maryliag Previously, we didn't have the plan hash/gist values, so a dummy value was being used instead. Now that we have the value, this commit uses those values to be corrected stored. The Plan Hash is saved on its own column and is part of a statement key. A plan gist is a string saved in the metadata and can later on converted back into a logical plan. Partially addresses #72129 Release note (sql change): Saving plan hash/gist to the Statements persisted stats. 75880: server: better error message for tsdump initialization r=liamgillies a=liamgillies Beforehand, the error message for tsdump initialization was unclear and didn't provide enough information to support engineers on how to fix it. To address this, the error message has been revamped with instructions and commands to get tsdump working. Release note (cli change): Added instructions to an error message when initializing tsdump. 76112: sql: Replace `WITH BUCKET_COUNT` with new `bucket_count` storage param. r=chengxiong-ruan a=chengxiong-ruan a follow up of pr #76068 Release note (sql change): We have add support for the new `bucket_count` storage param syntax. We prefer it over the old `WITH BUCKET_COUNT=xxx` syntax. With this change, crdb outputs the new syntax for `SHOW CREATE`. Though for the AST tree formatting, we still respect the old syntax if user used it. 76129: geo: add pgerror codes for errors.Newf r=rafiss a=otan Release note (sql change): Added PG error codes to the majority of spatial related functions. 76242: sql/logictest: fix flakey new_schema_changer r=ajwerner a=ajwerner IDs are not deterministic due to the non-transactional nature of the descriptor ID allocator sequence. The ID wasn't really adding value to the test anyway but rather the name was interesting. Fixes #76237 Release note: None Co-authored-by: Evan Wall <wall@cockroachlabs.com> Co-authored-by: Marylia Gutierrez <marylia@cockroachlabs.com> Co-authored-by: Liam Gillies <liam.gillies@cockroachlabs.com> Co-authored-by: Chengxiong Ruan <chengxiongruan@gmail.com> Co-authored-by: Oliver Tan <otan@cockroachlabs.com> Co-authored-by: Andrew Werner <awerner32@gmail.com>
|
Build failed (retrying...): |
|
Build failed (retrying...): |
|
Build succeeded: |
Beforehand, the error message for tsdump initialization was
unclear and didn't provide enough information to support
engineers on how to fix it. To address this, the error
message has been revamped with instructions and commands
to get tsdump working.
Release note (cli change): Added instructions to an error
message when initializing tsdump.