ENGKNOW-3657: Reduce Table Service dictionary memory (tunable TableCache + lazy content map)#129
Merged
Merged
Conversation
Diagnosis-first spec: local load harness + profiler to pinpoint the dominant memory consumer behind table-service OOMs under load. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… plan Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Use the same exclusive-lock write path as the real Table Service so concurrent writes serialize instead of racing on a shared temp file and silently losing inserts. Log caught op errors; shutdownNow on await timeout. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Cache table instances so read-side DictionaryEntries stay retained for the run (models a session/table cache), and measure post-GC retained-heap delta over a baseline. Replaces the peak-churn metric that failed to scale. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…driver Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
contentHashToLines, activeTags and deletedEntriesCount are only needed by the mutation/stats paths, not by select/filter reads. Building them lazily shrinks the long-lived cached dictionary copy for select-heavy workloads. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Keep the memory-diagnosis design spec and implementation plan as local-only files; not part of the PR. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Keep the memory findings and profiling runbook as local-only files; not part of the PR. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
getEntries()/isLoaded() read dataLoaded+rawLines without synchronization while a synchronized loader publishes them, so a concurrent reader could see dataLoaded==true with a stale rawLines (transient NPE/torn read). This is hit by concurrent selects sharing one cached GorDictionaryTable. Mark the lazily-published DictionaryEntries fields (and TableInfoBase.id) volatile so unsynchronized reads observe fully-constructed state; correct the class contract in the javadoc. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
getTableAccessOptimizer() built the optimizer via an unsynchronized null-check on a non-volatile field, so concurrent callers on a shared table could build/publish multiple instances unsafely. Use double-checked locking on a volatile field so concurrent readers share one safely-published optimizer. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
inferShouldBucketizeFromContent() lazily cached the content type in a non-volatile field with an unsynchronized null-check. Make the field volatile and compute via a local; the computation is idempotent so no lock is needed, and reads now observe a safely-published value. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
bragnarsson
approved these changes
Jul 9, 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
2 participants
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.
ENGKNOW-3657: Reduce Table Service dictionary memory
GOR-library side of the Table Service
selectmemory fix.Problem
Under concurrent
selectload, Table Service heap grows ~2GB→10GB. Root cause:selectre-parses a ~50MB / 300K-entry dictionary per call into its ownDictionaryEntries(all entry objects + indices), so N concurrent selects = N full copies.Changes in this PR
TableCachetunable + observable: max size / TTL from system propertiesgor.dictionary.cache.maxsize(default 500) andgor.dictionary.cache.ttl.hours(default 12);recordStats();stats()/estimatedSize()/cleanUp()accessors. Caffeine promotedimplementation→apiinmodelso the publicCacheStatsreturn type resolves for subclasses.DictionaryEntries:contentHashToLines,activeTags,deletedEntriesCountare now built lazily (only when the mutation/stats paths need them), not eagerly at load. The select/filter read path uses onlyrawLines+tagHashToLines, so a select-only dictionary keeps a smaller footprint.updateContentMap()is idempotent and resetsdeletedEntriesCountto stay correct across the incremental mutation path.Tests
UTestTableCache(tunable size + recorded stats) andUTestDictionaryEntriesLazy(read path does not build the content map; consumers do).org.gorpipe.gor.table.*suite: 169 pass, no regression.:gortools+:modeltest sources compile with the caffeine scope change.Merge order (cross-repo)
This PR must merge and be released before the companion
gdb-gor-servicesPR (which routesselectthrough this cache and bumpsGOR_VERSION).🤖 Generated with Claude Code