fix(dbreads): use timezone-aware now() for stats windows - #1555
Draft
Harsh63870 wants to merge 2 commits into
Draft
fix(dbreads): use timezone-aware now() for stats windows#1555Harsh63870 wants to merge 2 commits into
Harsh63870 wants to merge 2 commits into
Conversation
Replace now() at time zone 'utc' comparisons against timestamptz columns so instance stats and rollout policy windows stay correct when the Postgres session timezone is not UTC. Also pin TimeZone=UTC on connect. Signed-off-by: Harsh63870 <harshvardhanpandey372@gmail.com>
Signed-off-by: Harsh63870 <harshvardhanpandey372@gmail.com>
Author
|
@ervcz , resolved the merge conflict in backend/pkg/api/api.go against upstream/main, upstream had relocated nowUTC to pkg/api/runtime, so this branch now keeps only withUTCSessionTimezone and updates the timezone regression test to use runtimeSvc / runtime.NewInstanceApplication, no changes to the original fix logic, just adapted to the new location... |
This file contains hidden or 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
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.
Summary
Fixes #1541.
Several reporting and rollout-policy queries compared
timestamptzcolumns (last_check_for_updates,last_update_granted_ts) tonow() at time zone 'utc'. That expression returns a timestamp without time zone. When Postgres compares it back totimestamptz, it converts using the session timezone, so the window shifts by the UTC offset whenever the DB session is not UTC.That skews:
GetGroupInstancesStats,GetGroupVersionBreakdown, app instance counts, instance list filters)GetGroupUpdatesStats→updates_in_progress/updates_timed_out/ period rate limits), which can pause updates early (safe mode) or hold instances too longTimeline queries in the same package already used plain
now(), so two reporting paths reading the same columns disagreed.Changes
now() at time zone 'utc'with timezone-awarenow()in:backend/pkg/api/internal/dbreads/applications.gobackend/pkg/api/internal/dbreads/instances.gobackend/pkg/api/internal/dbreads/groups.goTimeZone=UTCto the DB URL on connect when the operator has not already set a timezone (withUTCSessionTimezoneinapi.go). ExplicitTimeZone/timezone/options=-c TimeZone=...are left alone.TestGroupInstanceStatsStableAcrossSessionTimezones— same 1-day window returns 3 under UTC, America/New_York, and Asia/KolkataTestWithUTCSessionTimezone— DSN helper behaviorTest plan
cd backend && go test ./pkg/api/ -run 'TestWithUTCSessionTimezone|TestGroupInstanceStatsStableAcrossSessionTimezones'cd backend && go test ./pkg/api/ -run 'TestGetGroup|TestRegisterInstance|TestGetInstances'SET TIME ZONE 'Asia/Kolkata', confirm group instance totals match UTC before/afterNotes
Sample fixture SQL (
sample_data.sql) still usesnow() at time zone 'utc'for inserts only; those are not the comparison bug described in #1541 and were left unchanged to keep this PR focused.