Avoid PIT context exhaustion by pruning indices that cannot match

[dai-chen]

Description

Prunes a wildcard index expression down to the indices that can match the query's @timestamp range before the PIT is created, so a scan over many time-bucketed indices no longer opens a reader context per shard of every matching index and exhausts search.max_open_pit_context.

• Approach: the probe is _field_caps with index_filter — Option A of the approaches weighed on the issue — driven by the resolved filter already sitting in the pushdown context.
• Notes
  • This is the PIT half of Approach 1; the schema-conflict half of that comment is not addressed here.
  • One decision is left open for review: the candidate list is capped at 50 rather than tied to search.max_open_pit_context, so an expression matching more indices than the cap falls back to the wildcard and opens every context.

Example

Three monthly indices, one shard each, one document each. max_result_window is 2 so that head 5 exceeds it and the query takes the PIT path, which is where pruning runs.

No reader contexts opened yet:

curl -s 'localhost:9200/demo-2026-*/_stats/search?filter_path=indices.*.total.search.point_in_time_total'

{"indices": {
  "demo-2026-01": {"total": {"search": {"point_in_time_total": 0}}},
  "demo-2026-02": {"total": {"search": {"point_in_time_total": 0}}},
  "demo-2026-03": {"total": {"search": {"point_in_time_total": 0}}}}}

Query a range only February can satisfy, with pruning at its default of false:

curl -s -X POST localhost:9200/_plugins/_ppl -H 'Content-Type: application/json' -d '{
  "query": "source=demo-2026-* | where `@timestamp` >= '\''2026-02-01 00:00:00'\'' and `@timestamp` <= '\''2026-02-28 23:59:59'\'' | fields body | head 5"}'

{"datarows": [["february"]], "total": 1, "size": 1}

All three indices opened a reader context, including the two that cannot match:

{"indices": {
  "demo-2026-01": {"total": {"search": {"point_in_time_total": 1}}},
  "demo-2026-02": {"total": {"search": {"point_in_time_total": 1}}},
  "demo-2026-03": {"total": {"search": {"point_in_time_total": 1}}}}}

Now recreate the indices to reset the counters, and enable pruning:

curl -s -X PUT localhost:9200/_plugins/_query/settings -H 'Content-Type: application/json' -d '{
  "transient": {"plugins.query.pruning.enabled": true}}'

The same query returns the same row:

{"datarows": [["february"]], "total": 1, "size": 1}

But only February was read — three reader contexts down to one:

{"indices": {
  "demo-2026-01": {"total": {"search": {"point_in_time_total": 0}}},
  "demo-2026-02": {"total": {"search": {"point_in_time_total": 1}}},
  "demo-2026-03": {"total": {"search": {"point_in_time_total": 0}}}}}

Related Issues

Part of #5698

Check List

• New functionality includes testing.
• New functionality has been documented.
• New functionality has javadoc added.
• New functionality has a user manual doc added.
• New PPL command checklist all confirmed.
• API changes companion pull request created.
• Commits are signed per the DCO using --signoff or -s.
• Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[github-actions]

PR Code Analyzer ❗

AI-powered 'Code-Diff-Analyzer' found issues on commit b25d8b7.

Path	Line	Severity	Description
opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilder.java	172	low	Index names (which may encode tenant, environment, or data sensitivity context) are logged at INFO level via log.info(). In a shared or multi-tenant cluster, these log entries are accessible to anyone with log file access and could leak index naming conventions. This is common operational logging and not evidence of malicious intent, but warrants a review of the log access policy.

The table above displays the top 10 most important findings.

Total: 1 | Critical: 0 | High: 0 | Medium: 0 | Low: 1

---

Pull Requests Author(s): Please update your Pull Request according to the report above.

Repository Maintainer(s): You can bypass diff analyzer by adding label skip-diff-analyzer after reviewing the changes carefully, then re-run failed actions. To re-enable the analyzer, remove the label, then re-run all actions.

---

⚠️ Note: The Code-Diff-Analyzer helps protect against potentially harmful code patterns. Please ensure you have thoroughly reviewed the changes beforehand.

Thanks.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 1bbddad)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Possible Issue The probeMatching method calls actionGet(PROBE_TIMEOUT) which can throw a timeout exception if the probe exceeds 5 seconds. This exception is caught in the prune method and logged as a warning, causing the query to fall back to the full wildcard expression. However, under heavy cluster load or with many shards, this timeout may be too short, leading to frequent fallbacks and defeating the purpose of pruning. The 5-second timeout may not be sufficient for clusters with hundreds of shards or high query load. String[] probeMatching(QueryBuilder filter) { FieldCapabilitiesRequest request = new FieldCapabilitiesRequest() .indices(indexName.getIndexNames()) .fields(IMPLICIT_FIELD_TIMESTAMP) .indexFilter(filter) // Must expand as the search will, or candidates describe a different index set. .indicesOptions(DEFAULT_INDICES_OPTIONS); return node.fieldCaps(request).actionGet(PROBE_TIMEOUT).getIndices(); Possible Issue The prunedIndexName field is memoized and reused across multiple build() calls on the same builder instance. If the builder is reused with different filters or index names (which is possible since the builder is mutable), the stale memoized value will be used instead of re-pruning with the new parameters. This can cause queries to read the wrong set of indices. /** Memoized because build() runs once per scan on the calling thread. */ @EqualsAndHashCode.Exclude @ToString.Exclude private OpenSearchRequest.IndexName prunedIndexName;

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 1bbddad

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Prevent stale memoization across queries The memoized prunedIndexName is never reset between different queries on the same builder instance. If the builder is reused with different filters or index names, stale pruning results will be incorrectly applied, leading to wrong indices being queried. opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilder.java [172-183] if (prunedIndexName == null) { - // Only the node client can issue the pruning probes, so the REST client never prunes. prunedIndexName = client .getNodeClient() .map( nodeClient -> new IndexPruner(nodeClient).prune(indexName, sourceBuilder.query())) .orElse(indexName); log.info( "Index expression has {} names after pruning", prunedIndexName.getIndexNames().length); log.debug("Pruned index expression from {} to {}", indexName, prunedIndexName); +} else if (!prunedIndexName.equals(indexName)) { + log.warn("Builder reused with different index name; pruning may be stale"); } Suggestion importance[1-10]: 3 __ Why: The concern about builder reuse is valid, but the suggested fix only adds a warning without actually preventing the stale memoization issue. The comment at line 79 states "Memoized because build() runs once per scan on the calling thread," suggesting the builder is not intended for reuse, making this a minor concern.	Low

---

Previous suggestions

Suggestions up to commit 8a8c4c4

Category	Suggestion	Impact
General	Handle probe timeout explicitly The actionGet(PROBE_TIMEOUT) call blocks the calling thread until the probe completes or times out. If the probe takes longer than 5 seconds, this will throw an exception. Consider handling the timeout exception explicitly to provide better error context and ensure graceful fallback to the unpruned expression. opensearch/src/main/java/org/opensearch/sql/opensearch/request/IndexPruner.java [135-144] String[] probeMatching(QueryBuilder filter) { FieldCapabilitiesRequest request = new FieldCapabilitiesRequest() .indices(indexName.getIndexNames()) .fields(IMPLICIT_FIELD_TIMESTAMP) .indexFilter(filter) - // Must expand as the search will, or candidates describe a different index set. .indicesOptions(DEFAULT_INDICES_OPTIONS); - return node.fieldCaps(request).actionGet(PROBE_TIMEOUT).getIndices(); + try { + return node.fieldCaps(request).actionGet(PROBE_TIMEOUT).getIndices(); + } catch (Exception e) { + log.warn("Field capabilities probe timed out or failed", e); + throw e; + } } Suggestion importance[1-10]: 3 __ Why: The suggestion adds explicit exception handling for the probe timeout, but the prune() method at line 54 already has a catch-all exception handler that logs and falls back gracefully. Adding another try-catch here would be redundant and doesn't improve error handling meaningfully.	Low

Suggestions up to commit 146b86f

Category	Suggestion	Impact
Possible issue	Validate builder reuse assumptions The memoization of prunedIndexName assumes the index expression and filter remain constant across multiple build() calls. If the builder is reused with different filters or indices, stale pruning results will be used. Consider clearing prunedIndexName when filters or indices change, or document that the builder should not be reused. opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilder.java [172-183] if (prunedIndexName == null) { prunedIndexName = client .getNodeClient() .map( nodeClient -> new IndexPruner(nodeClient).prune(indexName, sourceBuilder.query())) .orElse(indexName); log.info( "Index expression has {} names after pruning", prunedIndexName.getIndexNames().length); log.debug("Pruned index expression from {} to {}", indexName, prunedIndexName); +} else if (!indexName.equals(this.indexName)) { + throw new IllegalStateException("Builder cannot be reused with different index expressions"); } Suggestion importance[1-10]: 7 __ Why: This identifies a potential issue where memoization could cause stale results if the builder is reused with different parameters. The suggested validation check would catch this misuse, though the improved_code references this.indexName which doesn't exist in the current context. The concern is valid and important for correctness.	Medium
General	Avoid catching all exception types Catching all exceptions masks critical errors that should propagate, such as security exceptions or cluster state issues. Consider catching only specific expected exceptions (like timeout or resolution failures) and allowing critical errors to propagate so they can be properly handled upstream. opensearch/src/main/java/org/opensearch/sql/opensearch/request/IndexPruner.java [72-76] -} catch (Exception e) { +} catch (RuntimeException e) { log.warn("Index pruning failed; querying the full index expression", e); } return indexName; Suggestion importance[1-10]: 6 __ Why: The suggestion to catch RuntimeException instead of Exception is a valid improvement for better error handling. However, the improved_code still catches a broad exception type and doesn't fully address the concern about masking critical errors. The change provides marginal improvement in exception handling specificity.	Low
	Make probe timeout configurable The hardcoded 5-second timeout may be insufficient for large clusters or slow networks, potentially causing pruning to fail and fall back to querying all indices. Consider making this timeout configurable through settings to allow administrators to tune it based on their cluster characteristics. opensearch/src/main/java/org/opensearch/sql/opensearch/request/IndexPruner.java [40] -private static final TimeValue PROBE_TIMEOUT = TimeValue.timeValueSeconds(5); +private final TimeValue probeTimeout; +public IndexPruner(NodeClient node, TimeValue probeTimeout) { + this.node = node; + this.probeTimeout = probeTimeout; +} + Suggestion importance[1-10]: 4 __ Why: While making the timeout configurable could improve flexibility for different cluster environments, the current 5-second timeout is reasonable for most cases. The suggestion changes the constructor signature which would require updates throughout the codebase, making it a moderate improvement rather than a critical fix.	Low

Suggestions up to commit bd28765

Category	Suggestion	Impact
Possible issue	Add null safety checks The probeMatching method does not handle potential null responses from node.fieldCaps() or actionGet(). If the field capabilities response is null or the indices array is null, this will cause a NullPointerException. Add null checks before accessing the indices array. opensearch/src/main/java/org/opensearch/sql/opensearch/request/IndexPruner.java [135-144] String[] probeMatching(QueryBuilder filter) { FieldCapabilitiesRequest request = new FieldCapabilitiesRequest() .indices(indexName.getIndexNames()) .fields(IMPLICIT_FIELD_TIMESTAMP) .indexFilter(filter) // Must expand as the search will, or candidates describe a different index set. .indicesOptions(DEFAULT_INDICES_OPTIONS); - return node.fieldCaps(request).actionGet(PROBE_TIMEOUT).getIndices(); + FieldCapabilitiesResponse response = node.fieldCaps(request).actionGet(PROBE_TIMEOUT); + return response != null && response.getIndices() != null ? response.getIndices() : new String[0]; } Suggestion importance[1-10]: 3 __ Why: While adding null checks is generally good practice, the OpenSearch client APIs typically don't return null responses. The actionGet() method would throw an exception on failure rather than return null. The suggestion adds defensive code that may not be necessary given the API contract.	Low
	Validate resolution response before caching The resolved() method caches the result but does not handle the case where node.execute() or actionGet() returns null. If the resolution fails and returns null, subsequent calls will return null without retrying, potentially causing NullPointerExceptions in calling code that expects a valid response. opensearch/src/main/java/org/opensearch/sql/opensearch/request/IndexPruner.java [158-165] private ResolveIndexAction.Response resolved() { if (resolved == null) { ResolveIndexAction.Request request = new ResolveIndexAction.Request(indexName.getIndexNames(), DEFAULT_INDICES_OPTIONS); - resolved = node.execute(ResolveIndexAction.INSTANCE, request).actionGet(PROBE_TIMEOUT); + ResolveIndexAction.Response response = node.execute(ResolveIndexAction.INSTANCE, request).actionGet(PROBE_TIMEOUT); + if (response == null) { + throw new IllegalStateException("Failed to resolve index expression"); + } + resolved = response; } return resolved; } Suggestion importance[1-10]: 3 __ Why: Similar to suggestion 1, the OpenSearch execute() and actionGet() methods throw exceptions on failure rather than returning null. The existing code at line 72 already has a catch block that handles exceptions from pruning operations, making this additional null check redundant.	Low
General	Guard logging against null values The method accesses prunedIndexName.getIndexNames().length in the log statement without checking if prunedIndexName or its index names array is null. If pruning returns a null IndexName or the array is null, this will cause a NullPointerException during logging. opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilder.java [166-185] private OpenSearchRequest.IndexName pruneIndexName( OpenSearchRequest.IndexName indexName, OpenSearchClient client) { if (!Boolean.TRUE.equals(settings.getSettingValue(Settings.Key.QUERY_PRUNING_ENABLED))) { return indexName; } if (prunedIndexName == null) { // Only the node client can issue the pruning probes, so the REST client never prunes. prunedIndexName = client .getNodeClient() .map( nodeClient -> new IndexPruner(nodeClient).prune(indexName, sourceBuilder.query())) .orElse(indexName); - log.info( - "Index expression has {} names after pruning", prunedIndexName.getIndexNames().length); - log.debug("Pruned index expression from {} to {}", indexName, prunedIndexName); + if (prunedIndexName != null && prunedIndexName.getIndexNames() != null) { + log.info( + "Index expression has {} names after pruning", prunedIndexName.getIndexNames().length); + log.debug("Pruned index expression from {} to {}", indexName, prunedIndexName); + } } return prunedIndexName; } Suggestion importance[1-10]: 2 __ Why: The prune() method in IndexPruner always returns a non-null IndexName (either pruned or the original), as seen in line 75 where it falls back to indexName. The getIndexNames() method also returns a non-null array based on the IndexName constructor. This null check is unnecessary given the code structure.	Low

Suggestions up to commit d447702

Category	Suggestion	Impact
Possible issue	Add thread-safe memoization The memoization of prunedIndexName is not thread-safe. If build() is called concurrently from multiple threads, race conditions could cause pruning to execute multiple times or return inconsistent results. Synchronize access or use a thread-safe initialization pattern. opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilder.java [172-183] if (prunedIndexName == null) { - prunedIndexName = - client - .getNodeClient() - .map( - nodeClient -> new IndexPruner(nodeClient).prune(indexName, sourceBuilder.query())) - .orElse(indexName); - log.info( - "Index expression has {} names after pruning", prunedIndexName.getIndexNames().length); - log.debug("Pruned index expression from {} to {}", indexName, prunedIndexName); + synchronized (this) { + if (prunedIndexName == null) { + prunedIndexName = + client + .getNodeClient() + .map( + nodeClient -> new IndexPruner(nodeClient).prune(indexName, sourceBuilder.query())) + .orElse(indexName); + log.info( + "Index expression has {} names after pruning", prunedIndexName.getIndexNames().length); + log.debug("Pruned index expression from {} to {}", indexName, prunedIndexName); + } + } } return prunedIndexName; Suggestion importance[1-10]: 8 __ Why: The memoization pattern is not thread-safe and could lead to race conditions if build() is called concurrently. The double-checked locking pattern suggested would prevent multiple pruning executions and ensure consistency.	Medium
	Avoid catching critical JVM errors Catching all exceptions masks critical errors like OutOfMemoryError or StackOverflowError that should propagate. Catch only specific exceptions related to pruning operations (e.g., transport exceptions, timeout exceptions) to avoid suppressing JVM-level errors. opensearch/src/main/java/org/opensearch/sql/opensearch/request/IndexPruner.java [65-68] -} catch (Exception e) { +} catch (RuntimeException e) { log.warn("Index pruning failed; querying the full index expression", e); } return indexName; Suggestion importance[1-10]: 7 __ Why: Catching Exception can mask critical JVM errors like OutOfMemoryError. Changing to RuntimeException is safer, though the impact is moderate since pruning failures already fall back gracefully to the original index expression.	Medium
General	Make max pruned indices configurable The hardcoded limit of 50 indices may be too restrictive for large-scale deployments with many time-based indices. Consider making this configurable through the Settings system to allow administrators to tune it based on their cluster's PIT context capacity and query patterns. opensearch/src/main/java/org/opensearch/sql/opensearch/request/IndexPruner.java [37] -private static final int MAX_PRUNED_INDICES = 50; +private final int maxPrunedIndices; +public IndexPruner(NodeClient node, int maxPrunedIndices) { + this.node = node; + this.maxPrunedIndices = maxPrunedIndices; +} + Suggestion importance[1-10]: 5 __ Why: Making MAX_PRUNED_INDICES configurable could be useful for large deployments, but the suggestion changes the constructor signature which would require updates throughout the codebase. The current hardcoded value is reasonable for the experimental feature.	Low

Suggestions up to commit b25d8b7

Category	Suggestion	Impact
General	Remove premature logging statement The first log statement reveals the memoized value before it's computed, which will always log null on the first call. Move the first log after the pruning logic or remove it to avoid logging misleading information. opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilder.java [172-182] -log.info("Index pruning is enabled, pruned index name so far: {}", prunedIndexName); if (prunedIndexName == null) { // Only the node client can issue the pruning probes, so the REST client never prunes. prunedIndexName = client .getNodeClient() .map( nodeClient -> new IndexPruner(nodeClient).prune(indexName, sourceBuilder.query())) .orElse(indexName); log.info("Pruned index expression from {} to {}", indexName, prunedIndexName); } Suggestion importance[1-10]: 7 __ Why: The first log statement at line 172 will always log null on the first call since prunedIndexName hasn't been computed yet, making it misleading. Removing it improves log clarity without affecting functionality.	Medium
	Avoid catching all exception types Catching all exceptions masks critical errors that should fail fast, such as security exceptions or cluster state issues. Consider catching only specific expected exceptions (like timeout or transport exceptions) and letting security or authentication failures propagate to the caller. opensearch/src/main/java/org/opensearch/sql/opensearch/request/IndexPruner.java [65-68] -} catch (Exception e) { +} catch (TimeoutException | TransportException e) { log.warn("Index pruning failed; querying the full index expression", e); } return indexName; Suggestion importance[1-10]: 6 __ Why: Catching specific exceptions instead of all Exception types is generally better practice. However, the current implementation's fallback behavior (querying the full expression) is safe for all failure types, and the suggested specific exceptions may not cover all legitimate pruning failures.	Low
	Make max pruned indices configurable The hardcoded limit of 50 indices may be too restrictive for large-scale deployments with many time-based indices. Consider making this configurable through the Settings system to allow administrators to tune it based on their cluster's PIT context limits and query patterns. opensearch/src/main/java/org/opensearch/sql/opensearch/request/IndexPruner.java [37] -private static final int MAX_PRUNED_INDICES = 50; +private final int maxPrunedIndices; +public IndexPruner(NodeClient node, int maxPrunedIndices) { + this.node = node; + this.maxPrunedIndices = maxPrunedIndices; +} + Suggestion importance[1-10]: 5 __ Why: Making MAX_PRUNED_INDICES configurable could improve flexibility for different cluster sizes. However, the suggestion changes the constructor signature which would require updates throughout the codebase, and the current hardcoded value may be sufficient for most use cases.	Low

[github-actions]

Persistent review updated to latest commit d447702

[github-actions]

Persistent review updated to latest commit bd28765

[github-actions]

Persistent review updated to latest commit 146b86f

[github-actions]

Persistent review updated to latest commit 8a8c4c4

[github-actions]

Persistent review updated to latest commit 1bbddad