Repository navigation
fix(util): Decode partition dates as UTC regardless of the host timezone - #8562
Open
RaphaelFakhri wants to merge 1 commit into
Open
RaphaelFakhri wants to merge 1 commit into
RaphaelFakhri wants to merge 1 commit into
Conversation
decode_part_str called .astimezone(UTC) on the naive datetime returned by strptime, which Python interprets as local time. On hosts east of UTC the Monday partition date became the previous Sunday, so cleanup dropped partitions a week early and partition-split lightweight deletes matched no rows.
| @pytest.fixture | ||
| def local_timezone(monkeypatch: pytest.MonkeyPatch) -> Generator[None]: | ||
| # A zone east of UTC, where local midnight falls on the previous UTC day. | ||
| monkeypatch.setenv("TZ", "Asia/Tokyo") |
Member
There was a problem hiding this comment.
I'd use a context manager here - something like
@pytest.fixture
def local_timezone(monkeypatch: pytest.MonkeyPatch) -> Generator[None]:
with monkeypatch.context() as m:
m.setenv("TZ", "Asia/Tokyo")
time.tzset()
yield
time.tzset()avoids calling monkeypatch.undo()
This branch has not been deployed
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.
decode_part_strparses the partition date withdatetime.strptime(...), which returns a naive datetime, and then calls.astimezone(UTC). Python treats a naive datetime passed toastimezone()as local time, so on a host whoseTZis east of UTC the decoded date moves to the previous day. For example, withTZ=Asia/Tokyothe partition(90,'2026-10-12')(a Monday) decodes as2026-10-11 15:00 UTC(a Sunday).Every caller assumes the decoded date is the partition's Monday:
snuba cleanup:filter_stale_partitionscomputes the partition's last day as the end of the week, which becomes the Sunday before the partition starts. Partitions are dropped 7 days early, while they still hold data inside the retention window._get_partition_datesreturns2026-10-11, and the split delete filters ontoMonday(timestamp) = '2026-10-11'. That condition matches no rows, so the delete removes nothing for that partition and the partition is still recorded as done.snuba optimize --before: the cutoff comparison is off by a week.This change uses
.replace(tzinfo=UTC)so the date is always the partition's UTC date. Hosts running withTZ=UTC(or a zone west of UTC) see no change.The new test in
tests/test_util.pysetsTZ=Asia/Tokyoand checks thatdecode_part_strreturns the Monday at midnight UTC. It fails before this change and passes after.Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.
Fixes #8561