Skip to content

Separate sequential-read from merge in StoredFieldsReader (allow different instances for read vs merge ) #16427

Description

@VijayanB

Description

StoredFieldsReader.getMergeInstance() is currently used for two conceptually different purposes, and there is no way for an implementation to tell them apart. I'd like to propose a distinct method (e.g. getSequentialInstance()) for the "read a contiguous run of documents efficiently" use case, separate from the existing "I am merging segments" use case, so that a StoredFieldsReader can return a different instance for read vs. merge.

Background: one method, two meanings

getMergeInstance() is documented as "an instance optimized for merging … may not be cloned." Its performance value (in Lucene90CompressingStoredFieldsReader) is that it decompresses each block once into a reused buffer and serves all docs in that block from it — ideal for reading adjacent doc ids.

That property is useful well beyond merging, which is why it is also used at read time.

How this is used in OpenSearch

OpenSearch fetches stored fields for a contiguous run of doc ids in several common paths — deep pagination, scroll, match_all with a large size, reindex, and source-heavy aggregations. When the docs to fetch are adjacent, OpenSearch calls getMergeInstance() to obtain the block-at-a-time reader and gets a significant speedup over the random-access path.

OpenSearch also wraps StoredFieldsReader to transform stored fields at read time. A concrete case is derived source, where large fields (e.g. vectors) are stripped from _source on write to save space and re-injected when _source is read back. In that model the desired behavior differs by call site:

  • Read — the wrapper must inject/transform so the user gets complete _source.
  • Merge — the wrapper must not inject/transform, so the stripped representation is copied through untouched and not re-materialized on disk.

The natural way to express this is:

  • getMergeInstance() → transformation off (merge path)
  • normal document() → transformation on (read path)

The problem: read and merge are indistinguishable

Because the read-time sequential path also goes through getMergeInstance(), a StoredFieldsReader implementation cannot use getMergeInstance() to mean "I am merging." Overriding it to disable transformation would also disable transformation on ordinary read-time sequential fetches, which need it. The two intents collapse onto the same method, so a wrapper that legitimately wants merge ≠ read behavior has no way to express it.

Why a separate method is justified (rather than reusing getMergeInstance())

  1. They are different contracts. "Optimize for merging" carries merge-specific semantics (may not be cloned, merge lifecycle, and — for wrapping readers — merge-copy behavior). "Optimize for sequential reads" is a read-time, user-facing path with different lifecycle and transformation expectations. A single method cannot honor both.

  2. It lets implementations decouple the two instances. With a distinct entry point, a StoredFieldsReader can return one instance tuned for sequential reads (transformation on) and another for merging (transformation off), overriding each independently.

  3. The optimization is worth keeping — so "just stop using it for reads" isn't free. Falling back to the random-access reader for sequential fetches is a real performance regression. The sequential-read use case needs its own sanctioned entry point, not removal.

Proposal

Add a distinct entry point on StoredFieldsReader for sequential/bulk read access, separate from getMergeInstance(), with its own documented threading/cloning/lifecycle contract. Implementations that don't care can default it to the current behavior; implementations that need to distinguish read from merge can override the two independently.

/**
 * Returns an instance optimized for sequential access of adjacent doc ids at
 * read time. Distinct from {@link #getMergeInstance()}, which signals merging,
 * so implementations can return a different instance for reads vs. merges.
 * The default implementation returns {@code this}.
 */
public StoredFieldsReader getSequentialInstance() {
    return this;
}

Prior art / related

This proposal is narrower: rather than a prefetch/range API, it only asks to disambiguate the existing capability so reads and merges can use different instances.

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