Repository navigation
test(query): memory limit acceptance tests and measured figures (#549 PR 2) - #681
Merged
Merged
Conversation
…nting (#549) Ignored acceptance tests (1M aggregate groups; #575 cartesian k = 13, streamed in a few MiB), run nightly by smoke.yml without test-oracle, and an ignored engine test printing the meter's peak against peak RSS. Aggregate::finish built its sorted map of groups uncharged while the hash table and group rows were still alive: 1M groups metered 282 MiB against 651 MiB of RSS. The map is now charged until every row is built, and the table and rows go first (513 MiB RSS, 435 MiB metered). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019kY61Y11gcRD91Y6b7jbGT
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019kY61Y11gcRD91Y6b7jbGT
…sh (#549) Releasing the group charge after building the sorted map dropped the sum-distinct members, which live on in the map until finish_groups. They now move from the group charge to the sorted map's. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019kY61Y11gcRD91Y6b7jbGT
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
6 tasks
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.
#549 PR 2 (streaming engine 7/9 under #473): acceptance tests and measured figures, plan steps 11–13. PR 1 was #676.
What
tests/memory_limit_acceptance_test.rs(2 tests, ignored):[:find ?g (count ?e) :where [?e :g ?g]]returns 1M rows. Withquery_memory_limitat 32 MiB it fails withQRY-012namingAggregate, and the database still answers afterwards.QRY-012namingHashJoin.smoke.ymljobmemory-limit-acceptance:--profile bench, withouttest-oracle,--test-threads=1(peak RSS is per process).meter_peak_against_rss(ignored engine test): prints the meter's peak against peak RSS for both shapes. It opens the engine directly because the executor oracle would run out of memory on the product. It uses acfg(test)Minigraf::snapshot.docs/BENCHMARKS.mdgets the figures. CHANGELOG, TEST_COVERAGE, CLAUDE.md and README counts updated: 1725 tests (1711 passing, 14 ignored).Figures (dedicated Linux server, one NUMA node, 3 runs)
The executor held more than 5.7 GB on #575 at k = 13.
Fix found by measuring (deviation from the plan)
The first measurement of 1M groups gave a meter peak of 282 MiB against 651 MiB of RSS, outside spec §3.2's factor of two.
Aggregate::finishbuilt its sortedGroupsmap (each group's values twice, plus its accumulators) without charging it, while the hash table and group rows were still alive. Now:finisheduntil every row is built;sum-distinctmembers move from the group charge to the map's, so they are counted once and not dropped (Codex finding).Result: 435 MiB metered against about 545 MiB of RSS (1.25×). The real peak also dropped, from 651 to 513 MiB on a laptop. Two unit tests cover this; both fail without the fix.
Spec deviations (both in the plan)
:find ?v0reads the first pattern's value.Review
Codex found one issue:
sum-distinctmembers were uncharged duringfinish. Fixed. Its other finding, that the charges stay after a failedfinished.chargeinsidefinish, is by design:Stream::next_rowscloses the stream on any error, and that releases everyHeld. Codex's first pass confirmed this.#549 stays open until this merges, then gets closed by hand. Binding PRs follow. Refs #549, #575, #473.
🤖 Generated with Claude Code
https://claude.ai/code/session_019kY61Y11gcRD91Y6b7jbGT