Repository navigation
Lite counts each blocked process report, long query completion and system_health event once after a reset stored it twice - #4918
Merged
Conversation
…ent, long query completion, CPU sample and memory pressure event once after a 512 MB reset re-collects it The first collection cycle after the reset can read an empty watermark and fetch its fallback window again, so the archive and the hot table both held the same event. The views for these five tables now keep one row per exact identity, the earliest stored copy, and never collapse a row with no usable identity. The FinOps reserved-capacity CPU check now reads the archive view instead of the hot table.
An exact copy of a CPU sample changes no average, maximum or chart line, and a window over that table would cost every read, so cpu_utilization_stats keeps the plain union. Each of the four watermark reads now logs one WARN naming the read, the table, the server and the exception type when it fails and the collector falls back to its fallback window.
… a sweep pins every bare read FinOps' long-running jobs and file I/O checks, High Impact Queries and the Agent status header read their v_ views, so rows that archival moved to Parquet still count. A source sweep over every archivable table fails on any new bare read that is not on its list of reads that are bare on purpose.
…opped per read instead
…ad's own filter, and the collectors share their fallback window with that read
…dedupe-after-reset # Conflicts: # Lite/Analysis/AnomalyDetector.cs # Lite/Analysis/DrillDownCollector.Blocking.cs # Lite/Analysis/DuckDbFactCollector.Waits.cs # Lite/Services/LocalDataService.Overview.cs
Dev moved the blocking reads to event_time (#4909, #4913). A read that now windows on event_time calls StoredEventCopies without collectedFrom and keeps its event_time bounds in its filter: a stored copy keeps its first copy's event_time, so both are inside the window or both are outside it. The alert engine's read of blocked process reports still windows on collection_time, so it keeps collectedFrom. OnEventTime now swaps only the events CTE's own local-clock keys and window. Swapping every collection_time in the CTE would also rewrite the copy rule inside the blocking baseline's source, and that rule must stay on collection_time.
The event-baseline twin pin accepts a StoredEventCopies read as Lite's blocking event source, inside the OnEventTime wrapper, and pins its text. The consumed-timestamp census counts a StoredEventCopies.<Method>( call as a read of that method's table. The Lite sweep masks comments and string bodies with the shared source walker, as CommentFilterAdoptionTests requires, instead of dropping "//" lines.
A sweep fails when a call puts a collection_time lower bound in its where instead of passing it as collectedFrom, which would cut the look-back off. A long query completion with a NULL database, session or event sequence reads once, and completions that differ only there stay apart. The reader tests also cover the alert engine's collection_time read of blocked process reports, and the blocking baseline counts a stored copy once. The doc says a read on event_time needs no look-back, and states the look-back's clock assumption: a monitored server clock that is ahead of the collector's can put a first copy further back than the window.
The rule's key held each row's whole XML. Keying the XML part on md5_number holds 16 bytes per row instead. Every other part stays exact, and so do the CASE parts that keep a NULL or empty text apart. Two different reports would have to collide in 128 bits at one server and event time to merge, and a merge only hides a row from a read.
A window carries every row, its XML included, through the operator. At memory_limit 1GB it ran out of memory on 94,200 blocked process reports. The rule is now each identity's MIN(collection_time), grouped over the identity's keys alone and joined back to the rows with IS NOT DISTINCT FROM on every part. The XML part is DuckDB's 64-bit hash() of the text, named in one place. The parts that keep a row with no text apart test the raw text, because hash(NULL) is not NULL.
The comment said most of their readers filter on collection_time and that a window would run in the view. Most of them filter on event_time now, and the rule is a grouped minimum after each read's own filter.
…among the LF readers
Its StoredEventCopies census now reads that class through the LF reader, on an anchor that crosses the line break between a helper's => and its Read("v_ call.
…oves The two last-capture reads return MAX(collection_time), which a later batch's copies move on purpose; only the EXISTS read is one a copy cannot change.
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.
What users see
Before #4887, the first collection after Lite's 512 MB archive-and-reset read the collectors' 10-minute fallback window again. It stored blocked process reports, long query completions and system_health events that the archive already held. #4887 stopped new copies, but the copies already stored stay in the archive files.
Every read of these three tables counted such an event twice when both copies fell in its window. These reads serve:
A watermark read that fails also takes the fallback window, so it can still store a copy.
In the 30-day reads measured below, 7 of 942 blocked process reports and 9 of 5,189 system_health events of one type were copies.
Cause
PerformanceMonitor.Collectors/BlockedProcessReportCollector.cs:759,LongQueryCompletionsCollector.cs:230andSystemHealthEventsCollector.cs:159: a collector with no watermark reads back the fallback window.ArchiveService.ArchiveAllAndResetAsyncempties. So the first cycle after each reset took the fallback window and stored those events again.Lite/Services/RemoteCollectorService.cs:1480,:1579,:1659and:1721: a watermark read that fails also falls back. These catches logged nothing.Lite/Database/DuckDbInitializer.cs: each archive view for these three tables is a plain union of the live table and the archive files. So every read saw both copies.Changes
Lite/Database/StoredEventCopies.cs(new) is the one place that readsv_blocked_process_reports,v_long_query_completionsandv_system_health_events. Each read passes its own filter, and the helper drops the copies that a later batch stored, after that filter.DENSE_RANK() OVER (PARTITION BY identity ORDER BY collection_time) = 1. The helper groups the rows on the identity alone to find each identity'sMIN(collection_time). It joins that back to the rows withIS NOT DISTINCT FROMon every part of the identity, so a NULL part matches a NULL part.collection_timeon a whole run, strictly later than its previous run, so identical rows from one batch share it.server_id,event_timeand a 64-bithash()of the event's XML. A long query completion has no XML. Its identity is the server, database, event time, statement text, session and event sequence, all compared exactly.collection_timeto its key. They test the raw text, not its hash, becausehash(NULL)is not NULL.XmlKeyinStoredEventCopies.csis the only place the hash is named. Two different reports, or system_health events, that share a server and event time merge only if their XML hashes collide. The chance is about 2^-64 per pair. A collision only affects reads: it hides one row from one read and never deletes anything. Nothing stores the hash.memory_limit. There, a window over the identity ran out of memory. It did so with the XML compared exactly and with the XML keyed bymd5_number. A window carries every row, XML included, through the operator, while the grouped side holds only the keys.md5_numbergives a 128-bit key. On DuckDB 1.5.5, though, it read the XML at about 140 MB/s on one thread, 11 times slower thanhash(). The numbers are under Performance.event_time, and they need no look-back. A copy keeps its first copy'sevent_time, so both are inside the filter or both are outside it.collection_time: the two long query reads and the alert engine's form ofGetRecentBlockedProcessReportsAsync. Each passes its lower bound to the helper. The rule then also readsCollectorContext.EventFallbackWindowbefore that bound. So a copy stored inside the window, whose first copy was stored just before the window, is dropped too, and the first copy stays outside.event_time, is not ahead of the collector's. A server clock that is ahead can put a first copy further back. A copy stored just inside the window's start is then read.PerformanceMonitor.Collectors/CollectorContext.cs:70addsEventFallbackWindow(10 minutes). The three collectors and the helper use it, and no copy of the literal is left.collection_timeis not part of an event's identity, so a read'scollection_timefilter cannot run below that rule. The comment inDuckDbInitializer.csthat says so is brought up to date.Lite/Analysis/BaselineProvider.cs:OnEventTimemoves the blocking baseline's events CTE toevent_time. It swapped everycollection_timein that CTE, which also rewrote the helper's rule inside it, and the rule then kept every copy. It now swaps only the CTE's own keys and window.RemoteCollectorService.csnow log a WARN that names the read, the table, the server id and the exception type.{SCOPE}) on three blocking reads sits inside the helper's filter. The helper repeats that filter in both of its scans, and oneReplacecovers the whole statement.ArchivableTableBareReadSweepTestsin Lite advice, High Impact Queries and the Agent check read archived rows after the 512 MB archive step #4921, because all three are inArchiveService.ArchivableTables.Every read of the three views
There are 23 reads. 20 go through the helper, in 21 calls, because read 10 has two forms. The other 3 read the view directly.
Lite/Analysis/AnomalyDetector.cs:790current_blockingcount, byevent_timeLite/Analysis/BaselineProvider.cs:717event_timeLite/Analysis/BaselineProvider.cs:775event_timeLite/Analysis/DrillDownCollector.Blocking.cs:92event_timeLite/Analysis/DrillDownCollector.Blocking.cs:157event_timeLite/Analysis/DuckDbFactCollector.Waits.cs:254event_timeLite/Analysis/DuckDbFactCollector.Waits.cs:405event_timeLite/Services/LocalDataService.Blocking.cs:593GetAlertCountsAsyncblocking count, byevent_timeLite/Services/LocalDataService.Blocking.cs:599GetAlertCountsAsynclatestevent_timeLite/Services/LocalDataService.Blocking.cs:658and:659GetRecentBlockedProcessReportsAsync: the alert engine's form bycollection_time(:658), every other caller byevent_time(:659)Lite/Services/LocalDataService.Blocking.cs:863GetBlockingPairRowsAsync, byevent_timeLite/Services/LocalDataService.Blocking.cs:911GetBlockingSlicerDataAsync, byevent_timeLite/Services/LocalDataService.Blocking.cs:1019GetBlockingTrendAsync, byevent_timeLite/Services/LocalDataService.BlockingStats.cs:98GetBlockingDurationStatsAsync, byevent_timeLite/Services/LocalDataService.DailySummary.cs:56bprCTE, byevent_timeLite/Services/LocalDataService.LongQueries.cs:41GetRecentLongQueryCompletionsAsync, bycollection_timeLite/Services/LocalDataService.LongQueries.cs:70GetSlowestLongQueryCompletionsAsync, bycollection_timeget_long_query_completionsLite/Services/LocalDataService.Overview.cs:117GetServerSummaryAsyncblocking count, byevent_timeLite/Services/LocalDataService.SystemEvents.cs:329ReadSystemHealthEventXmlAsync, byevent_timeand typeLite/Services/LocalDataService.SystemEvents.cs:562CountSystemHealthEventsAsync, byevent_timeand typeLite/Services/LocalDataService.BlockingStats.cs:57HasAnyBlockingCaptureAsync,EXISTSwith no time windowLite/Services/LocalDataService.SystemEvents.cs:510GetLastSystemHealthCaptureAsync,MAX(collection_time)with no time windowLite/Services/LocalDataService.SystemEvents.cs:535GetLastSystemHealthCaptureOfTypeAsync, the same for one typeThe three direct reads have no time window, and they read every stored row on purpose:
MAX(collection_time), on purpose. That batch still read the session, so itscollection_timeis a true capture time.Lite.Tests/StoredEventCopiesSweepTests.csfails on any other read of the three views, and on a listed read that no longer exists. It also fails when a call puts acollection_timelower bound in its own filter instead of passing it to the helper. No read inLocalDataService.DatabaseStates.cstouches these views. The sources for that file are chosen in #4917.Performance
All numbers come from Lite's own engine, DuckDB.NET 1.5.5, at Lite's
memory_limitof 1 GB. The data is in an in-memory database, with views built the way Lite builds them. The helper's SQL comes from the built Lite assembly. Times are in milliseconds.The real archive files
These are real rows only: the real archive files, read through
read_parquet. The live tables are not read. Each window ends at the busiest server's last archived row, on the read's own clock. Before is the plain view read with the read's own filter, as on dev. After is the same read through the helper. Each figure is the median of 5 runs after a warm-up.current_blockingcountevent_timeThe 24-hour windows held 12 blocked process reports and 1,437 system_health events, with no copies. The 30-day windows held 940 to 942 blocked process reports with 7 copies, or 898 with 7 copies on the alert engine's
collection_timewindow. They held 5,189 system_health events with 9 copies. Reads 21 to 23 run the same SQL as before.No long query completion archive file exists on this machine, so reads 16 and 17 have no real-file numbers. They use the same helper as the other reads.
A plain
COUNT(*)never reads the XML column. The helper has to read and hash the XML in each of its two scans, and a blocked process report's XML averages 13,111 characters here. For the 30-day count of read 1:The cost grows with the rows and the XML inside the read's own window, not with the size of the archive.
100 times the busiest server's month
Real rows plus synthetic rows. The data is the busiest server's 942 real rows from the last 30 days and the look-back. Each has 99 synthetic copies, for 93,258 synthetic rows. Each synthetic copy shifts
event_timeby a few microseconds and appends its own number to the XML as a comment. So every copy is a distinct event with its own text, while the 7 stored copies inside each set stay copies. That makes 94,200 rows with about 1.2 GB of XML, in one Parquet file.Each figure is the median of 3 runs after a warm-up, with one form per process and no other test running.
md5_numberof the XMLmd5_numberof the XMLhash()of the XML (this change)Out of Memory Error: failed to pin block of size 256.0 KiB (953.6 MiB/953.6 MiB used).hash(), notmd5_number: at 20 times the month, there is 236 MB of XML. One pass over it took 1,718 ms withmd5_number, 157 ms withhash()and 111 ms withstrlen. DuckDB 1.4.4 took 605 ms for the samemd5_numberpass.hash()reads the whole text. On DuckDB 1.5.5, 20 texts gave 20 different hashes. One was a base text of 13,000 characters. 18 others each changed one character of it, at places from the first to the last. The last one added a character at the end.Pins
All of these run on DuckDB.NET 1.5.5.
Lite.Tests/StoredEventCopiesTests.cs, for each of the three tables:CollectorContext.EventFallbackWindowback, and that the helper's SQL looks back the same number of seconds.Lite.Tests/StoredEventCopiesTests.cs: a long query completion with a NULL database, session or event sequence reads once. Completions that differ only in which of those is NULL stay apart.Lite.Tests/StoredEventCopiesReaderTests.cs: an event's first copy is in an archive file and its later copy is in the live table. Six readers and the alert engine's form of read 10 each count the event once. A window that starts between the two copies shows neither, in both forms of read 10.Lite.Tests/StoredEventCopiesSweepTests.cs: the sweep described above.Lite.Tests/EventBaselineCoveredDaysTests.cs: a blocked process report stored again by a later batch counts once in the blocking baseline.Lite.Tests/WatermarkReadFailureLogTests.cs: each failed watermark read logs one WARN that names the read, the table, the server and the exception type.Darling/Darling.Tests/DarlingEventBaselineCoveredDaysTests.csandConsumedTimestampFrameDisciplineTests.csread the helper's calls in Lite's source.Red proofs
For each row, the product change was undone, the tests ran and failed, the file was restored, and
git statuswas clean.At
b7dd51435, before the merge with dev:collection_timelower bound in their own filter>=instead of=on the first batch'scollection_time)OnEventTimeswaps everycollection_timein the events CTE, as it did before=instead ofIS NOT DISTINCT FROMin the joinMAXinstead ofMINAt
2c797f05f, before the dev merge and the change of form:Tests
StoredEventCopiesSweepTestsand Lite advice, High Impact Queries and the Agent check read archived rows after the 512 MB archive step #4921'sArchivableTableBareReadSweepTests), and the classes that read these views. 129 tests, 0 failed, at2a0d4bd49, the merge with dev at3c5621b47. Neither sweep finds a bare read of the three tables.DarlingEventBaselineCoveredDaysTestsandConsumedTimestampFrameDisciplineTests: 22 tests, 0 failed, at2a0d4bd49.2a0d4bd49: 7,037 tests, 0 failed. After that head, the Lite source changed only in a doc comment.2a0d4bd49failed one Darling test:RepoFileAdoptionTests.TheLfReadingPins_AreExactlyTheOnesDeclaredHere.ConsumedTimestampFrameDisciplineTestsnow readsStoredEventCopies.csthrough the LF reader, and that test's list of LF readers did not name it.e80edd684adds it to the list.c6a4dcea7: DarlingRepoFileAdoptionTests2 tests andConsumedTimestampFrameDisciplineTests17 tests, 0 failed. LiteStoredEventCopiesTests,StoredEventCopiesSweepTestsandStoredEventCopiesReaderTests: 33 tests, 0 failed.c6a4dcea7: 19,704 tests, 0 failed, 1,265 skipped, 1 not run. The skipped tests need something this run did not have, almost always a live PostgreSQL. CI runs the live PostgreSQL ones.c6a4dcea7: 0 warnings.CHANGELOG
The changelog line is written from this entry at release, so
CHANGELOG.mdis not edited here.SECTION: Fixed
ENTRY: Splice into the [#4887] line, in place of "Duplicates stored by earlier resets are not removed.": Blocked process reports, long query completions and system_health events that an earlier reset stored twice now show and count once ([#4918]).
REF: [#4918]: #4918