Repository navigation
The self-hosted log-events live test drives its own events, so a log rotation cannot hide them - #4702
Merged
Merged
Conversation
…ight log rotation cannot hide them The test relied on a workflow step that ran the workload once, before the tests. PostgreSQL starts a new log file at every log_rotation_age boundary (a day by default, midnight in log_timezone), and the collector reads only the newest file's last 4 MB. When the rotation landed between that step and the read, the lock wait sat in a file the collector never reads, and the LockWait assertion failed. Error and Connection still passed because the UTC test that runs just before wrote a fresh one of each into the new file. The test now makes its own events after it starts, over unpooled connections: a caught SELECT 1/0, and a lock wait made of two sessions on one advisory key, held past deadlock_timeout. It polls the shipped collector query until each family shows a row written by one of those backends since the drive began, and drives again if the newest log file changes mid-poll. On the deadline (deadlock_timeout plus 30 s) it fails naming the missing families, the log settings and the newest log file at the start and now. The lock_wait page limit follows the target's own count instead of a fixed 5: a reused target adds two lock-wait lines per run.
…p ratchet does not read it as a store teardown LiveCleanupConversionRatchetTests sweeps every finally block in a live class for store teardown that skips LiveStoreCleanup. The drive helper's finally released a lock on the target, not store state, and tripped it. The same release now runs in a catch that rethrows; the success path is unchanged.
erikdarlingdata
marked this pull request as ready for review
September 29, 2026 01:29
This was referenced Sep 29, 2026
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.
Fixes a flake in
PgLogEventsLivePostgresTests.TheSelfHostedCollector_ReadsTheTargetsOwnLog_EndToEnd. Test code only. The product gap behind it is #4699 and is not touched here.Why
The test failed on the LockWait
Assert.Contains(PgLogEventsPipelineTests.cs:2821, "Filter not matched in collection") in the job "Darling PG tests (1)" of run 36500483761.build.yml(about lines 1088-1129) that runs the workload once, before the tests. That step ran from 23:59:16 to 23:59:33Z.log_rotation_ageboundary. The default is 1d andlog_timezoneis UTC, so it rotated at 00:00:00Z.PgServerLogTail.TailCteSql:ORDER BY modification DESC LIMIT 1). The lock wait sat in the old file, which the collector never reads.What changes
All of it is in
PgLogEventsLivePostgresTests.Pooling=falseconnections. That is a caughtSELECT 1/0for the Error family, and a lock wait: one session holdspg_advisory_xact_lock(<random key>)in an open transaction and a second blocks on the same key. Every session is its own backend, so each writes its connection and disconnection lines.pg_locksshows the waiter withgranted = false, keeps the lock pastdeadlock_timeout(read from the target), then releases it. The waiter then writes both lock-wait lines, "still waiting" and "acquired". A failed drive rolls back and never leaves the waiter blocked. That release is acatchthat rethrows, not afinally, becauseLiveCleanupConversionRatchetTestssweeps everyfinallyin a live class for store teardown.PgLogEventsCollector.BuildQueryandReadAsync, not a private query) until each family has a row written by one of this run's backend pids. A row must also be stamped no earlier than the drive began, read from the target's own clock, because the pid alone would match an older backend that had the same pid and Windows recycles pids fast. Rows left by the workflow step or by other tests cannot satisfy the poll.TailCteSqlchooses it) changes during the poll, the target is driven again, up to 3 drives in all.deadlock_timeoutplus 30 s, counted from the latest drive. On it,Assert.Failnames the missing families, the settingslog_lock_waits,deadlock_timeout,log_rotation_ageandlog_timezone, the driven pids, and the newest log file at the start and now.Assert.Containslines and the UTCAssert.Allare unchanged.lock_waitread at the end used a fixedlimitof 5. A reused target keeps every run's lines in its log file, and each run adds 2 lock-wait lines ("still waiting" and "acquired"). The tool setstruncatedwhen the window holds more rows than the limit, so"truncated": falsewould fail from the third run on the same file. The limit is nowMath.Max(5, <lock_wait count>). Measured on a reused target over consecutive runs:lock_wait= 2, 4, 6, 8. The test passed at 6 and 8.build.ymlis untouched. Its workload step is now redundant for this test and harmless.Test plan
Rig: PostgreSQL with
logging_collector = on,log_timezone = 'UTC',log_line_prefix = '%m %u@%d [%p] ',log_lock_waits = on,log_connections = on,log_disconnections = onanddeadlock_timeout = 100ms(the same as CI's log-target cluster), plus a separate store.PgLogEventsLivePostgresTests: 3 of 3 passed, repeatedly (about 1.4 to 1.9 s).SELECT pg_rotate_logfile(), then the class: the test failed with "Assert.Contains() Failure: Filter not matched in collection" at line 2819 (Error). In that run the UTC test had not run first. In the failed CI run it had, which is why only line 2821 failed there.PgLockWaitEventParser: failed after 30.1 s. "The target's log did not show this run's lock_wait event(s) within 30.1 s of the latest drive (1 drive(s)) ... Target settings: deadlock_timeout=100ms, log_lock_waits=on, log_rotation_age=1d, log_timezone=UTC. Newest log file at the start: ... now: ... The last read returned 31 event(s): checkpoint=2, connection=25, error=4."PgErrorEventParserrejecting every severity: same message namingerror. The last read returnedcheckpoint=2, connection=37, lock_wait=6.PgConnectionEventParser: same message namingconnection. The last read returnedcheckpoint=2, error=6, lock_wait=8.OccurredAtUtcre-kinded as Unspecified inPgLogEntryAssembler: the poll passes (the families are there), and the keptAssert.Allfails: "80 out of 80 items in the collection did not pass. Expected: Utc, Actual: Unspecified". This one fails on the kept assertion, not on the deadline message.Darling.Testssuite, once, against a fresh store database and both live targets, after mergingorigin/dev(already up to date): 16751 tests, 2 failed, 62 skipped.LiveCleanupConversionRatchetTests.NoLiveTestCleansUpOnItsOwnBodysConnectionfailed on this change's first commit. It sweeps everyfinallyin a live class for store teardown that skipsLiveStoreCleanup, and it flagged thefinallyin the new drive helper, which releases a lock on the target and touches no store state. The release now runs in acatchthat rethrows (the success path is unchanged). After the fix that class passes 15 of 15, andPgLogEventsLivePostgresTestspasses 3 of 3 on a fresh store database. The full suite was not re-run after this fix.TrendPayloadBudgetLiveTests.EveryDefaultAnswer_StaysNearTheBudget_AndTheLargestAnswerStaysUnderTheCapfailed with "get_pg_io_trend over 72h answered a error instead of data ... Exception while reading from stream". It fails the same way when run alone on a freshly created store database (59 s). It reads only the store, and this diff touches one log-events test class. It fails the same way on the same rig with an unmodifiedorigin/devbuild, and it passes in dev's CI, so the failure comes from the rig.Double-check
log_timezone = 'UTC'on the log target, as before. The UTC test asserts it.CHANGELOG
None: test-only change.