Skip to content

Stop rejecting queries by an estimated window, charge values by their size and size the default query pool from host memory - #1450

Merged
hanahmily merged 7 commits into
apache:mainfrom
hanahmily:fix-query-window-estimate
Oct 30, 2026
Merged

hanahmily merged 7 commits into
apache:mainfrom
hanahmily:fix-query-window-estimate

Conversation

@hanahmily

@hanahmily hanahmily commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Without a memory limit, stream, trace and property queries whose limit + offset exceeds 1792 are rejected with RESOURCE_EXHAUSTED before any data is read, so OAP's default LIMIT 10000 (apache/skywalking#13961) fails even on a stream holding a few rows. This removes that estimate. The 0.11.x backport is #1449.

What changed

  • Admission no longer estimates the result window. Without --allowed-bytes or a valid cgroup limit, the query budget uses its 64 MiB fallback pool and assigns each query 8 MiB. Admission assumed every requested row costs 4 KiB and rejected limit + offset above (8 MiB − 1 MiB) / 4 KiB = 1792, while limit = MaxUint32 was admitted as a scan. ErrQueryWindowPressure and the unused MaxWindow helper are removed.
  • The protections from fix(query): bound Property/Stream/Trace query allocations #1334 that close the preallocation problem stay. Preallocation is capped at 1,024 entries, limit + offset above 100,000 is rejected as an invalid argument, the pool still rejects queries once it is exhausted, and execution charges the blocks it decodes and the results it retains, so a query that really holds more than its allowance still fails.
  • Two preallocations sized by the request get the same cap: the sidx heap merge (by limit) and the measure TopN post-processor (by topN).

Large values and the default pool

OAP's ai-agent E2E also failed its size case with query memory pool exhausted when BanyanDB has no memory limit: OAP stores each agent session file as one body tag, and reading back one 1.5 MB file exceeded the allowance. Two causes, one commit each:

  • Charge tag and field values by their size. The merge operators charge every retained value through EstimatedRetainedBytes, which estimated a tag or field value as proto.Size × 8 + 256. The ×8 was meant for the object overhead of small values but also multiplied the payload, so a 1.5 MB value was charged 12 MB and one row exceeded the 8 MiB allowance on its own. A value is now charged its payload once plus a fixed 64-byte object overhead (and 16 bytes per string-array element, 8 per int-array element), through a type switch instead of proto.Size. One such row now peaks at about 1.5 MB of charge.
  • Size the default query pool from host memory. Without --allowed-bytes or a valid container memory limit, the query pool was a fixed 64 MiB, so each query got 8 MiB whatever the machine. It is now host memory × --allowed-percent (75% by default) ÷ 4, and each query an eighth of that (3 GiB pool and 384 MiB per query on a 16 GiB host). 64 MiB stays the fallback when host memory cannot be read. The memory protector itself stays disabled, so write backpressure and merge budgets are unchanged, and --allowed-bytes or a container limit still take precedence.
  • Test harness. Packages whose tests rely on the 64 MiB fallback pin it with SetHostMemoryForTest(0) in TestMain, so they keep proving enforcement on any machine. The distributed query_budget suite's SetNextQueryPool landed on the data node's embedded schema server rather than the data node, and only passed because of the ×8 charge; the new SetQueryPoolAfter(1, …) targets the data node. The stream budget case now uses one 2 MiB element to exceed its allowance, since sixteen 64 KiB elements fit once charged honestly.

End-to-end tests

BanyanDB's OAP E2E suites pinned OAP at b4a8811 (2026-04-14), before OAP's AI-agent storage existed, so nothing in CI stored multi-MB values or read them back with LIMIT 10000. Following the existing convention (each case is a copy of SkyWalking's, with BanyanDB taken from this repository's testing image):

  • SW_OAP_COMMIT moves to 5689aa8 (SkyWalking main), with the agent, Rover, Kubernetes chart and swctl pins synced to SkyWalking's script/env, and SW_AI_SESSIONIZER_COMMIT added.
  • The ai-agent case (AI agent conversations landed by the AI Sessionizer) is added and registered; BanyanDB runs without a memory limit, as in SkyWalking.
  • The storage, cluster, event, zipkin and trace-profiling cases are re-synced with SkyWalking at that commit (storage gains the inspect, TTL and config-dump checks; the cluster case keeps the TopN queries SkyWalking moved out of the shared file).
  • OAP no longer publishes a UI image, so the unused ui compose service is dropped and the Rover and lifecycle Helm installs set ui.enabled=false, as SkyWalking does.
  • install-yq.sh installs yq v4.54.1 when the yq present lacks tonumber, which the ai-agent verify script uses.
  • OAP 11 drops records older than their TTL as soon as it receives them unless TESTING_TTL=true, which SkyWalking's own TTL cases set; the lifecycle data generator, lifecycle file discovery and transfer export write data dated days ago, so their OAP sets it.
  • OAP 11 no longer seeds UI templates, so transfer export creates one through ui-management to keep its property export covered, and its expectations follow OAP 11's group options (sw_records gains a warm stage). Lifecycle file discovery enables the OAP admin port the synced storage checks use.

Verification

  • Protector tests cover the fallback pool admitting limit + offset of 1,792, 1,793, 10,000 and 100,000, the 100,000 ceiling, and pool exhaustion. New data-driven stream and trace cases run LIMIT 10000.
  • go test -race passes on the touched packages; the standalone and distributed query integration suites pass; make lint and make check pass.

The stream scan budget fix for apache/skywalking#14165 landed in #1433.

  • A stream test with OAP's schema shape (body, digest and the entity in a storage-only family, seq searchable), 1.5 MB bodies at one timestamp in separate parts: one copy passes under an 8 MiB allowance, five copies still fail with RESOURCE_EXHAUSTED (7.5 MB of real data), and five copies pass with a pool sized for a 16 GiB host.
  • Unit tests cover each value kind, a 1.5 MB value charged within 1% of its size, the host-sized pool, --allowed-percent, precedence of a configured limit, and the unreadable-host fallback.
  • go test -race passes on protector, stream, query, measure, trace, property and dquery, and on pkg/query/.... banyand/liaison/grpc passes apart from three registry specs and TestLoadSheddingIntegration, which fail the same way on unmodified main. The standalone and distributed query and query_budget integration suites pass. make lint and make check pass.
  • On the 0.11.x branch with the same two changes, SkyWalking main's ai-agent E2E passes all 21 cases locally; before them size failed.
  • Run locally against this branch with OAP 5689aa8: ai-agent 21/21, storage 79/79, cluster 78/78, log 4/4, event 4/4, zipkin 7/7, trace profiling 9/9, and FODC in Kind 3/3, lifecycle file discovery 78/78, lifecycle DNS discovery in Kind 8/8, transfer export 9/9. Rover on Istio, RBAC and native log were left to CI.

Without a memory limit the query budget falls back to a 64 MiB pool and
each query is assigned pool/8 = 8 MiB. AdmitContext estimated 4 KiB per
requested row before reading any data and rejected every query whose
limit + offset exceeded (assignment - 1 MiB) / 4 KiB = 1792 with
ErrQueryWindowPressure (RESOURCE_EXHAUSTED). A stream with 50 rows and
LIMIT 10000 was refused, which broke OAP, whose default LIMIT is 10000.

The estimate was the fourth layer added by apache#1334. The other three still
bound memory on their own: the 100,000 absolute window ceiling
(ErrQueryTooLarge), the capped preallocation hints, and the incremental
charges for decoded blocks and retained results (query.Charge and
ChargeResult). A limit says how many rows a caller will accept, not how
many exist, so it is not a memory estimate.

Remove the estimate, ErrQueryWindowPressure and the unused MaxWindow
helper. The fixed-overhead charge, the pool reservation and execution-time
charging are unchanged. Also cap the sidx merge and TopN post-processor
preallocations, which were sized by the request limit.

Add protector unit tests and stream and trace LIMIT 10000 cases.
@codecov-commenter

codecov-commenter commented Oct 29, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.51648% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.79%. Comparing base (3530dd9) to head (e47adbb).
⚠️ Report is 503 commits behind head on main.

Files with missing lines Patch % Lines
banyand/protector/query_budget.go 75.00% 6 Missing and 1 partial ⚠️
banyand/protector/test_hook.go 68.42% 6 Missing ⚠️
pkg/query/vectorized/retained_bytes.go 93.10% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #1450       +/-   ##
===========================================
+ Coverage   45.97%   62.79%   +16.81%     
===========================================
  Files         328     1006      +678     
  Lines       55505   148998    +93493     
===========================================
+ Hits        25520    93564    +68044     
- Misses      27909    46494    +18585     
- Partials     2076     8940     +6864     
Flag Coverage Δ
banyand 60.26% <79.03%> (?)
bydbctl 70.95% <ø> (?)
fodc 60.57% <ø> (?)
integration-distributed 67.09% <ø> (?)
integration-standalone 94.51% <ø> (?)
pkg 64.45% <93.10%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@hanahmily hanahmily added the bug Something isn't working label Oct 30, 2026
@hanahmily hanahmily added this to the 0.12.0 milestone Oct 30, 2026
wu-sheng and others added 2 commits October 30, 2026 10:56
The merge of main brought QueryFailure from apache#1433, which still maps
ErrQueryWindowPressure to RESOURCE_EXHAUSTED; this branch removes that error,
so the package no longer compiled. Only the pool exhaustion error remains.
A tag or field value was charged proto.Size*8+256, so a 1.5 MB value cost ~12 MB
against an 8 MiB per-query allowance. Charge the payload once plus a fixed
object overhead per value instead.

Also fix the distributed query budget test, whose pool pin landed on the data
node's embedded schema server, and give its fixture a value larger than the allowance.

(cherry picked from commit b2bf44f)
With no --allowed-bytes and no valid cgroup limit the protector limit is 0 and
the query pool was a fixed 64 MiB, so a query got 8 MiB regardless of the
machine. Size the pool from the host's total memory times the protector's
allowed percent, divided by the pool fraction. The protector itself stays
disabled. Unreadable host memory keeps the 64 MiB pool.

Budget tests that rely on the 64 MiB pool pin the host memory through a test
hook; add the OAP-shaped regression test for 1.5 MB bodies.

(cherry picked from commit 9b352f6)
@hanahmily hanahmily changed the title Stop rejecting queries by an estimated result window Stop rejecting queries by an estimated window, charge values by their size and size the default query pool from host memory Oct 30, 2026
- TESTING_TTL=true on the OAP of the lifecycle data generator, file discovery and transfer
  export cases: OAP 11 drops records older than their TTL on receipt, and these cases write
  data dated days ago.
- File discovery enables the OAP admin server (SW_ADMIN_SERVER, port 17128) and the inspect
  module for the synced storage checks.
- OAP 11 no longer seeds default UI templates into sw_property, so the transfer export setup
  creates one through ui-management (swctl admin ui-template create --id transfer-e2e), and
  the export checker and the property expectation pin that template instead of the General ones.
- Expectations follow OAP 11's group options: sw_records has one shard, the one-day hot TTL and
  a warm stage (nodeSelector type=warm) in the stream and schema-only exports, and the
  sw_metricsHour warm stage has 10d segments in the measure and schema-only exports.
@hanahmily
hanahmily merged commit 2d9f456 into apache:main Oct 30, 2026
54 checks passed
@hanahmily
hanahmily deleted the fix-query-window-estimate branch October 30, 2026 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants