cli: prevent `demo` and `start` from working if TZ db not avail by knz · Pull Request #45680 · cockroachdb/cockroach

knz · 2020-03-04T00:01:43Z

Recommended by @rkruze .

Previously, unavailability of named time zones (issue #36864) was only
causing an error message to be printed on the terminal and the log
files. However, some users were not looking and would instead run
directly into silently incorrect results in SQL.

This change ensures a node doesn't even start when the TZ database is
not ready.

Example output:

ERROR: unable to load named timezones
HINT: See: https://github.com/cockroachdb/cockroach/issues/36864
--
Check that the time zone database is installed on your system, or
set the ZONEINFO environment variable to a Go time zone .zip archive.
Failed running "demo"

Release note (cli change): CockroachDB now refuses to start if named
time zones are not properly configured. It is possible to override
this behavior for testing purposes, with the understanding that doing
so will cause incorrect SQL results and other inconsistencies, using
the environment variable COCKROACH_INCONSISTENT_TIME_ZONES.

cockroach-teamcity · 2020-03-04T00:01:50Z

This change is

ajwerner

Code

Should we provide a mechanism to allow a user to ignore this problem if they know what they're doing. The case I worry about is somebody is trying to show off some specific feature of cockroach and encourages somebody else to download cockroach and try demo. That person doesn't have a TZ database for whatever reason but maybe for the context of this example, they don't care. Specifically I'm thinking of either an env var or flag that tells this change to just log like it used to.

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @ajwerner)

knz · 2020-03-04T00:08:47Z

Specifically I'm thinking of either an env var or flag that tells this change to just log like it used to.

I'm having a hard time of imagining a scenario where this would be desirable: the user who has been instructed to "simply download cockroach" but then "set this env var otherwise it doesn't work" is likely to keep the env var forever without understanding it and then complain to support that TZ operations don't work properly.

knz · 2020-03-04T00:20:18Z

(I am going to add the env var though)

knz · 2020-03-04T00:21:16Z

Done

ajwerner · 2020-03-04T00:24:31Z

heh I was about to say okay, that makes sense. Thanks for appeasing me!

Still

knz · 2020-03-04T01:00:26Z

bors r=ajwerner

craig · 2020-03-04T02:09:18Z

Build failed

GitHub CI (Cockroach)

knz · 2020-03-04T11:25:50Z

unrelated fluke it seems

bors r=ajwerner

knz · 2020-03-04T13:30:00Z

bors r=ajwerner

Previously, unavailability of named time zones (issue cockroachdb#36864) was only causing an error message to be printed on the terminal and the log files. However, some users were not looking and would instead run directly into silently incorrect results in SQL. This change ensures a node doesn't even start when the TZ database is not ready. Example output: ``` ERROR: unable to load named timezones HINT: See: cockroachdb#36864 -- Check that the time zone database is installed on your system, or set the ZONEINFO environment variable to a Go time zone .zip archive. Failed running "demo" ``` Release note (cli change): CockroachDB now refuses to start if named time zones are not properly configured. It is possible to override this behavior for testing purposes, with the understanding that doing so will cause incorrect SQL results and other inconsistencies, using the environment variable `COCKROACH_INCONSISTENT_TIME_ZONES`.

craig · 2020-03-04T14:26:35Z

Build failed (retrying...)

GitHub CI (Cockroach)

tbg · 2020-03-04T14:28:20Z

bors r-

craig · 2020-03-04T14:28:22Z

Canceled

awoods187 · 2020-03-17T02:22:05Z

@rkruze copying this over from slack:

Do you think Raphael’s fix here will actually cause problems for people? The current situation is that nodes will not boot if they don’t have the time zone DB installed, which might happen on e.g. limited docker containers.

ajwerner · 2020-03-17T02:30:53Z

We added a workaround if a user is willing to deal with not having the TZ db. You'll need to specify COCKROACH_INCONSISTENT_TIME_ZONES=t in the environment.

jordanlewis · 2020-03-17T02:48:30Z

Andrew, the (in my opinion reasonable) concern is that this is a pretty high friction workaround for something that people might conceivably run into when trying CockroachDB for the very first time. Also, pretending to be a user who is not a CockroachDB developer, the well-intentioned hints are still obtuse. Why would I know what a Go time zone .zip archive is? Also, the COCKROACH_INCONSISTENT_TIME_ZONES env variable isn't mentioned in the hint, and in the linked issue, it's only listed as a vague workaround without specific instructions.

Why would we disable demo from working in these circumstances? Why not let it start up, but then return errors or warnings in the off chance that people run time zone operations? This change seems like a large hammer to apply without a deprecation cycle.

ajwerner · 2020-03-17T02:51:47Z

I don't disagree. I'm cool with demo just logging in this case. @knz was hesistant offer an escape hatch.

I also buy the concern that the env var is not in the hint. Seems like another iteration here is in order.

This PR is a follow-up to cockroachdb#45680 to re-enable `cockroach demo` even if the tz db is not available. Demo should not force users into workarounds that are vague and poorly documented. Release justification: low risk, high benefit changes to existing functionality Release note: None

ajwerner · 2020-03-17T03:45:32Z

@jordanlewis see how you feel about #46178

knz · 2020-03-17T11:01:24Z

something that people might conceivably run into when trying CockroachDB for the very first time.

Citation needed on this. On MacOS and Linux, where the tzdata package is pre-installed, this error would never happen.

Also, pretending to be a user who is not a CockroachDB developer, the well-intentioned hints are still obtuse. Why would I know what a Go time zone .zip archive is?

This could be solved by documentation (we provide a download link on our site) and a hint in the error message.

ghost · 2020-09-02T16:29:28Z

I'm a user trying to learn and I can't get past this error. I've added zoneinfo.zip and updated path statement. Based on the comments above I've also tried: set COCKROACH_INCONSISTENT_TIME_ZONES=t and set COCKROACH_INCONSISTENT_TIME_ZONES=true.

It still does not work. Any help would be great!!! ;)

Also, what is GO language and how to install on windows:
"CockroachDB uses the Go language's idea "

This should be off by default, and turned on when people care about it.

PS C:\Users\q> set COCKROACH_INCONSISTENT_TIME_ZONES=t
PS C:\Users\q> cockroach start-single-node --insecure `

--listen-addr=localhost:26257 `
--http-addr=localhost:8080

WARNING: RUNNING IN INSECURE MODE!
- Your cluster is open for any client that can access localhost.
- Any user, even root, can log in without providing a password.
- Any user, connecting as root, can read or write any data in your cluster.
- There is no network encryption nor authentication, and thus no confidentiality.
Check out how to secure your cluster: https://www.cockroachlabs.com/docs/v20.2/secure-a-cluster.html
ERROR: ERROR: failed to initialize node: unable to load named timezones
HINT: See: https://go.crdb.dev/issue/36864
--
Check that the time zone database is installed on your system, or
set the ZONEINFO environment variable to a Go time zone .zip archive.

ERROR: failed to initialize node: unable to load named timezones
HINT: See: https://go.crdb.dev/issue/36864

Check that the time zone database is installed on your system, or
set the ZONEINFO environment variable to a Go time zone .zip archive.
Failed running "start-single-node"

knz requested a review from ajwerner March 4, 2020 00:01

knz requested a review from a team as a code owner March 4, 2020 00:01

ajwerner approved these changes Mar 4, 2020

View reviewed changes

knz force-pushed the 20200304-tz branch from 9e6dcff to 1c82161 Compare March 4, 2020 00:20

knz mentioned this pull request Mar 4, 2020

sql: stop relying on time.LoadLocation for time zone data #36864

Closed

knz force-pushed the 20200304-tz branch from 1c82161 to f82310d Compare March 4, 2020 00:23

knz force-pushed the 20200304-tz branch from f82310d to eca1053 Compare March 4, 2020 00:25

knz force-pushed the 20200304-tz branch from eca1053 to 3b3b8af Compare March 4, 2020 14:28

tbg merged commit 3b3b8af into cockroachdb:master Mar 4, 2020

knz deleted the 20200304-tz branch March 4, 2020 15:24

ajwerner mentioned this pull request Mar 17, 2020

cli: allow demo to work if TZ db not available #46178

Closed

bobvawter mentioned this pull request Jul 21, 2020

docs: Missing breaking change in 20.1 release notes about "missing" timezone data #51679

Closed

Conversation

knz commented Mar 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cockroach-teamcity commented Mar 4, 2020

Uh oh!

ajwerner left a comment

Choose a reason for hiding this comment

Uh oh!

knz commented Mar 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

knz commented Mar 4, 2020

Uh oh!

knz commented Mar 4, 2020

Uh oh!

ajwerner commented Mar 4, 2020 • edited by petermattis Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

knz commented Mar 4, 2020

Uh oh!

craig bot commented Mar 4, 2020

Build failed

Uh oh!

knz commented Mar 4, 2020

Uh oh!

knz commented Mar 4, 2020

Uh oh!

craig bot commented Mar 4, 2020

Build failed (retrying...)

Uh oh!

tbg commented Mar 4, 2020

Uh oh!

craig bot commented Mar 4, 2020

Canceled

Uh oh!

awoods187 commented Mar 17, 2020

Uh oh!

ajwerner commented Mar 17, 2020

Uh oh!

jordanlewis commented Mar 17, 2020

Uh oh!

ajwerner commented Mar 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ajwerner commented Mar 17, 2020

Uh oh!

knz commented Mar 17, 2020

Uh oh!

ghost commented Sep 2, 2020 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ERROR: failed to initialize node: unable to load named timezones HINT: See: https://go.crdb.dev/issue/36864

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

knz commented Mar 4, 2020 •

edited

Loading

knz commented Mar 4, 2020 •

edited

Loading

ajwerner commented Mar 4, 2020 •

edited by petermattis

Loading

ajwerner commented Mar 17, 2020 •

edited

Loading

ghost commented Sep 2, 2020 •

edited by ghost

Loading

ERROR: failed to initialize node: unable to load named timezones
HINT: See: https://go.crdb.dev/issue/36864