Skip to content

feat(util): clarify Accountable ownership semantics and fix inconsistent ramBytesUsed() #16113

Description

@salvatorecampagna

Goal

Clarify the Accountable.ramBytesUsed() ownership semantics first, so that any subsequent work on missing or inconsistent implementations has a single rule to apply. The contract is the prerequisite; revisiting implementations is downstream and out of scope here.

Java has no synchronous release event for memory; ramBytesUsed() is a snapshot of currently held bytes, with close() as the one synchronous release signal the language offers for resources that have one.

Problem

Accountable.ramBytesUsed() is specified as "an estimate of the bytes used by this object" without saying whether bytes from referenced objects are included. In practice, implementations in the Lucene codebase vary: some count everything reachable from the object, including borrowed/shared state passed in via the constructor; some count only what they allocated; a few have explicit comments noting that a referenced object "should be accounted separately."

When external code sums ramBytesUsed() across multiple Accountable instances that share state, the same memory can be counted more than once.

getChildResources() is conventionally a diagnostic accessor for printing and inspection. It does not advertise itself as a deduplicated ownership tree, and consumers should not assume that summing over it yields a well-defined per-node total.

Examples of divergence

These are not bug reports. They show that today's contract admits multiple defensible choices, which is what the proposal is meant to resolve.

GlobalOrdinalsQuery counts a shared OrdinalMap received through its constructor:

// lucene/join/src/java/org/apache/lucene/search/join/GlobalOrdinalsQuery.java
this.ramBytesUsed = BASE_RAM_BYTES + ... + RamUsageEstimator.sizeOfObject(this.globalOrds) + ...

The OrdinalMap is cached on the top reader context and shared across queries that target the same join field on the same index. Two such queries each include its full size, so a consumer summing across queries sees the same bytes more than once. This is a defensible local choice under today's contract; it just isn't the only one.

GlobalOrdinalsWithScoreQuery in lucene/join/src/java/org/apache/lucene/search/join/GlobalOrdinalsWithScoreQuery.java has the same pattern at the equivalent site.

Lucene99HnswVectorsWriter.FieldWriter includes flatFieldVectorsWriter.ramBytesUsed() in its own count. The outer writer's per-field loop comments that the field tracks the delegate's usage and the outer omits the delegate from its own count to compensate. The pair sums to the right total today; the inner and outer stay in step through a comment rather than through the contract. A clarified contract would make where the bytes are attributed explicit without changing the total.

Downstream impact

Embedders (Elasticsearch, Solr, OpenSearch, custom indexers) build memory inventories by summing ramBytesUsed() across the Accountables they hold open. When two Accountables that share state both report the shared size, the totals inflate, and downstream consumers cannot tell from the API which value reflects actual heap. Capacity planning and circuit breaker thresholds drift, diagnostics become forensic since the API alone cannot answer why a reader reports N GB on an M GB heap, and per shard or per segment inventories cannot produce useful totals without a deduplication layer.

Proposal

  1. Clarify Accountable's javadoc with an ownership-oriented convention. Define "owned" as memory allocated for this object's lifetime, or released by its close() if applicable. Inputs received via constructors or factories — borrowed, wrapped, sliced, or shared — are reported only to the extent they are owned, not by their deep content. The reference slot itself is already covered by the holder's shallow size. State that getChildResources() is a diagnostic accessor and not a deduplicated ownership tree; summing over it is not sum-safe in general. With each implementation reporting only its owned slice, summing ramBytesUsed() across Accountable instances becomes a defined operation: each byte is reported by at most one holder, so the sum is meaningful without a deduplication layer. The convention text in the companion PR makes three precision points explicit: ramBytesUsed() reports owned heap bytes (mmap regions and direct buffers are out of scope); you own what you close(), regardless of who allocated it; a clone reports the bytes it allocated for its own iteration state, not the master's.

  2. Revisit individual implementations only as follow-up. Once the convention is settled, classes that diverge can be evaluated case-by-case in separate PRs, weighing local design intent against the shared rule. This issue does not propose specific changes; the examples above are illustrative.

Backward compatibility

The proposal does not change the Accountable interface contract or signature. Fixes to specific classes will change the numbers those classes report, and ordering matters: undercounting is operationally more dangerous than overcounting because it lets capacity planners and circuit breakers assume headroom that does not exist. The codebase already undercounts on average since many classes do not implement Accountable, so adding the interface to classes that report nothing today is always safe. The real risk is migrating a class that currently includes borrowed state before that state's owning Accountable exists, which turns a known overcount into a fresh undercount; such migration should wait.

Accountable.getChildResources() is unchanged. Existing patterns where a parent aggregates child ramBytesUsed() while also exposing those children via getChildResources() (DocumentsWriterPerThread, IndexingChain, similar tests) continue to work as today.

Related issues

  • LUCENE-8855 (2019, fixed in 8.2): "Add Accountable to some Query implementations". Direct prior evidence that the same ambiguity this issue addresses already shaped API decisions: Adrien Grand declined to add Accountable to BytesRef and IntsRef because they can represent slices of arrays and the contract did not say whether borrowed deep content should be counted. The community also chose UNKNOWN_DEFAULT_RAM_BYTES_USED = 256 to overcount unknown query types, codifying the operational asymmetry this issue's Backward compatibility section names.
  • apache/lucene#15097 (fixed by #15124): "Incorrect size estimation of BooleanQuery in LRUQueryCache causing heap exhaustion". Operational evidence that query size estimation affects cache budgets in production. The fix replaced a 1024 byte default with RamUsageEstimator.sizeOf(query), a generic overcounting walker, consistent with the LUCENE-8855 preference; BooleanQuery still does not implement Accountable.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions