Skip to content
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.

Sign up for GitHub

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

Jump to bottom

cli/config: Warn if zoneinfo not available or up to date #23828

Closed
bdarnell opened this issue Mar 14, 2018 · 2 comments · Fixed by #38197
Closed

cli/config: Warn if zoneinfo not available or up to date #23828

bdarnell opened this issue Mar 14, 2018 · 2 comments · Fixed by #38197
Labels
A-monitoring C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)

Comments

@bdarnell
Copy link
Contributor

CockroachDB will use the system zoneinfo database if available for timezone computations (otherwise, the go standard library embeds a snapshot). Since the zoneinfo database is updated several times a year, it is better to get this data from the system (assuming the system keeps it up to date) than to rely on the internal snapshot. We should document this fact and encourage the use of system zoneinfo (the use of a docker image without zoneinfo is especially common among alpine users: #16053 (comment))
@knz knz changed the title cli: Warn if zoneinfo not available or up to date cli/config: Warn if zoneinfo not available or up to date Apr 11, 2018
@bdarnell bdarnell added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) A-monitoring labels Apr 26, 2018
@knz
Copy link
Contributor

knz commented Aug 7, 2018

There is no "internal snapshot". The go distribution ships a file zoneinfo.zip in $GOROOT but it is not embedded inside the cockroach binary so a prod system would not be able to use it.

We actually provide our own custom LoadLocation wrapper (timeutil/zoneinfo.go) which ensures that if Go's zoneinfo.zip is not found (a common occurrence), the error message is meaningful.

As a result, I would be surprised if cockroach worked properly in a docker image without a system zoneinfo database.

@bdarnell
Copy link
Contributor Author

bdarnell commented Aug 7, 2018

As a result, I would be surprised if cockroach worked properly in a docker image without a system zoneinfo database.

Yeah. It would be nice to try to resolve a time zone at startup to detect this and log an error instead of just waiting for the first query that uses a time zone.

Reporting the age of the zoneinfo database as a metric would be nice, but I'm not sure if we can easily get that information.

@knz knz mentioned this issue Jun 17, 2019
Merged
craig bot pushed a commit that referenced this issue Jun 17, 2019
@knz @nvanbenschoten
Merge #38197 #38214
c6ff147 
38197: cli: warn the user if the time zone database is unavailable r=knz a=knz

Fixes #23828.
(The part in that issue about the database not being up to date cannot be fixed using the Go standard library - this does not give access to the search path and the version number. A comprehensive fix needs to wait for #36864.)

Release note (cli change): CockroachDB will now print out an error
message and an informative hint if the time zone database is unusable.

38214: storage/tscache: save 4 bytes per encoded tscache value r=ajwerner a=nvanbenschoten

`unsafe.Sizeof(hlc.Timestamp{})` was including XXX_sizecache, which was bloating
the struct size even though it didn't have an effect on the encoded size of the
timestamps, which we control in `encodeValue`.

Even once we remove `XXX_sizecache` (#37706), it looks like `unsafe.Sizeof` will
still include padding which we don't want to capture in the `encodedTsSize` constant,
so this seems like the best approach. See https://play.golang.org/p/5swJSSbP6J4.

Release note: None

Co-authored-by: Raphael 'kena' Poss <[email protected]>
Co-authored-by: Nathan VanBenschoten <[email protected]>
@craig craig bot closed this as completed in #38197 Jun 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-monitoring C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

cli: warn the user if the time zone database is unavailable knz/cockroach
2 participants
@bdarnell @knz