fix: rebuild the DFLogBuffer index cache without BinaryFormatter - #25
Open
userepo wants to merge 1 commit into
Open
fix: rebuild the DFLogBuffer index cache without BinaryFormatter#25userepo wants to merge 1 commit into
userepo wants to merge 1 commit into
Conversation
BinaryFormatter throws unconditionally on modern .NET, so saving the index cache crashed the open of any path-backed log at or over 300 MB, and loading silently never worked. The cache is now a versioned BinaryWriter format over the same gzip stream, stamped with the source file's length and write time so a stale cache for an edited log is rejected (the cache path only encodes the length, so a same-length in-place edit was previously undetectable). Writes go through a temp file and replace, so a torn write never lands at the cache path (also fixing the non-truncating File.OpenWrite). The load commits only after the whole cache reads back cleanly and resets the index collections on any failure, so a torn or foreign cache falls back to a fresh scan without leaving a half-loaded index behind. List lengths read from the cache - which lives in the shared temp directory and is untrusted - are bounded by the log's possible record count before any allocation. Neither direction can fail an open. The 300 MB threshold is unchanged but is now a named internal seam. DflogBufferCacheTests pins round-trip identity, stale-, corrupt- and implausible-length-cache rejection; every case fails against the old code. A real 433 MiB / 10.5M-record flight log that previously crashed the open now scans and saves in 5.7 s and reopens from the cache in 1.8 s.
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.
DFLogBuffercaches its index for large logs, but the cache is serializedwith
BinaryFormatter, which throws unconditionally on modern .NET.That makes it doubly broken on .NET10:
SaveCacheruns unguarded at the end of theinitial scan, so opening any path-backed log at or over 300 MB throws
PlatformNotSupportedExceptionout of the constructor — LogBrowse justreports "Failed to read log". (The
CancellationReadStreamwrappers inLogIndexService/OfflineMagFitServiceexist to dodge exactly this, butevery path-based consumer — LogBrowse curves/rows, expressions, FFT,
GeoRef — is exposed.)
Deserializesits in atry/catch, so every open of a big log pays the full scan.
The fix
The cache is reimplemented as a small versioned
BinaryWriterformat overthe same gzip stream: magic + version, the source file's length and
last-write time, then the raw index longs. Same feature, same location,
same 300 MB threshold (now a named internal constant) — but:
length, so a same-length in-place edit was previously undetectable; the
recorded write time catches it now.
(this also fixes the old non-truncating
File.OpenWrite, which couldleave trailing garbage after a shorter rewrite).
whole file reads back cleanly and resets the index collections on any
failure. It's important because the fallback rescan appends into them. List
lengths are bounded by the log's possible record count before any
allocation, since the cache lives in the shared temp directory and its
contents are untrusted. Neither direction can fail an open.
System.Runtime.Serialization.Formatters.Binaryis gone from the file.Verification
DflogBufferCacheTests(5 cases over a synthesized binary log): round-tripindex identity, same-length-edit rejection, garbage/truncated-cache
fallback, and implausible-length rejection. Every case fails against the
old code: the round-trip test's first open throws exactly like production.
Real-log proof with a 433 MiB / 10.5M-record flight log from public
archive (which crashes the open on master): first open scans and saves
in 5.7 s, reopening loads the cache in 1.8 s with an identical index -
the first time this cache has ever functioned on .NET10.
Full suite: 1538/1549 — the failures are the pre-existing
platform/environment set, unchanged from master.
Independent of the dflog offer from the log-download PR thread; this is a
standalone bug fix from master. (If that lands later, its native scanner
simply bypasses this cache and the managed fallback keeps benefiting.)