Repository navigation
[C++] Bound retained memory during hot-device metadata lookups - #1030
Open
zhangxinyao88 wants to merge 2 commits into
Open
zhangxinyao88 wants to merge 2 commits into
zhangxinyao88 wants to merge 2 commits into
Conversation
Copy measurement index lookup results into the iterator/query arena, including intermediate nodes during multi-level traversal. Preserve the existing lookup entry point and keep cached node arenas unchanged. Cover repeated hot-device lookups, result-copy lifetime and an active iterator surviving cache eviction. Generated-by: OpenAI Codex
Keep column iterator destructors inside the block lifetime. MSVC ASan caught their reset_offset writes after revert_tsblock freed the vectors. The production metadata ownership fix remains unchanged. Generated-by: OpenAI Codex
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.
Repeated lookups of a hot device clone measurement names into the cached index node's arena. Releasing each scan iterator therefore leaves retained reader memory growing even with one cached device: the regression fixture grows from 1,072 to 84,688 accounted bytes after 10,000 lookups on develop.
Give lookup-result copies an explicit destination arena and use the query/iterator arena for measurement lookup, including every level of internal-index traversal. Keep the existing lookup signature and default behavior for other callers. Temporary traversal nodes can be destroyed safely, and cache eviction cannot invalidate parsed iterator metadata. The file format and read results are unchanged.
Fixes #1009.
Validation on macOS arm64, JDK 17, clang-format 17.0.6:
./mvnw -B -ntp -P with-cpp -Denable.lzma2=ON verifypasses with all configured codecs, ANTLR and SIMD enabled. CTest registers 1,180 cases: 1,167 passed, 3 skipped (external cross-language fixtures not configured), 10 disabled upstream performance/large-file cases, 0 failures. Spotless, Checkstyle and RAT pass.-fsanitize=address,undefined; the test binary imports ASan/UBSan handlers and loads the ASan runtime. Optional codecs/ANTLR/SIMD are disabled for this separate lifetime probe. Includes both hot-device index depths, returned names surviving source arena reset and a row read after cache eviction.git diff --checkpasses. No Java/Python suite, full sanitizer suite or external cross-language-fixture run is claimed.Validation correction (2026-10-22): the earlier local build labelled ASan had lost its sanitizer compiler flags during CMake configuration, so those earlier nine passes were plain Debug results. MSVC ASan then caught a use-after-free in this PR's eviction test: column iterators were destroyed after
revert_tsblock()freed their vectors. The follow-up commit scopes those iterators before block release. The full C++ Maven verification and the genuinely instrumented nine-case probe above were rerun successfully after that fix. The production metadata-ownership change is unchanged. Windows verification of the follow-up is still pending.CI at the original head also showed Linux setup failures before compilation (
clang-format-17absent; Python 3.9 unavailable for Ubuntu 26.04). Those provisioning failures are outside this patch. No complete green-CI claim is made.AI disclosure: OpenAI Codex generated this patch, tests, and description and performed the reported verification. No personal human review is claimed.