From f3551bc19eb91095438dd5c41d399c1ebc177b60 Mon Sep 17 00:00:00 2001 From: bowenlan-amzn Date: Mon, 24 Feb 2025 16:35:17 -0800 Subject: [PATCH] Clean unused code Signed-off-by: bowenlan-amzn --- .../opensearch/bootstrap/BootstrapChecks.java | 6 +- .../java/org/opensearch/common/Rounding.java | 27 ++--- .../bucket/composite/CompositeAggregator.java | 15 +-- .../CompositeDocIdSetIterator.java | 112 ------------------ .../histogram/DateHistogramAggregator.java | 3 +- .../bucket/range/RangeAggregator.java | 35 ++---- 6 files changed, 33 insertions(+), 165 deletions(-) delete mode 100644 server/src/main/java/org/opensearch/search/aggregations/bucket/filterrewrite/CompositeDocIdSetIterator.java diff --git a/server/src/main/java/org/opensearch/bootstrap/BootstrapChecks.java b/server/src/main/java/org/opensearch/bootstrap/BootstrapChecks.java index b7d3d94015bf1..0e0b4e9be261a 100644 --- a/server/src/main/java/org/opensearch/bootstrap/BootstrapChecks.java +++ b/server/src/main/java/org/opensearch/bootstrap/BootstrapChecks.java @@ -712,9 +712,9 @@ static class AllPermissionCheck implements BootstrapCheck { @Override public final BootstrapCheckResult check(BootstrapContext context) { - // if (isAllPermissionGranted()) { - // return BootstrapCheck.BootstrapCheckResult.failure("granting the all permission effectively disables security"); - // } + if (isAllPermissionGranted()) { + return BootstrapCheck.BootstrapCheckResult.failure("granting the all permission effectively disables security"); + } return BootstrapCheckResult.success(); } diff --git a/server/src/main/java/org/opensearch/common/Rounding.java b/server/src/main/java/org/opensearch/common/Rounding.java index e653205b547c0..c6fa4915ad05a 100644 --- a/server/src/main/java/org/opensearch/common/Rounding.java +++ b/server/src/main/java/org/opensearch/common/Rounding.java @@ -38,6 +38,8 @@ import org.opensearch.common.LocalTimeOffset.Gap; import org.opensearch.common.LocalTimeOffset.Overlap; import org.opensearch.common.annotation.PublicApi; +import org.opensearch.common.round.Roundable; +import org.opensearch.common.round.RoundableFactory; import org.opensearch.common.time.DateUtils; import org.opensearch.common.unit.TimeValue; import org.opensearch.core.common.io.stream.StreamInput; @@ -60,7 +62,6 @@ import java.time.temporal.TemporalQueries; import java.time.zone.ZoneOffsetTransition; import java.time.zone.ZoneRules; -import java.util.Arrays; import java.util.List; import java.util.Locale; import java.util.Objects; @@ -454,7 +455,7 @@ protected Prepared maybeUseArray(long minUtcMillis, long maxUtcMillis, int max) values = ArrayUtil.grow(values, i + 1); values[i++] = rounded; } - return new ArrayRounding(values, i, this); + return new ArrayRounding(RoundableFactory.create(values, i), this); } } @@ -463,26 +464,17 @@ protected Prepared maybeUseArray(long minUtcMillis, long maxUtcMillis, int max) * pre-calculated round-down points to speed up lookups. */ private static class ArrayRounding implements Prepared { - private final long[] values; - private final int max; + private final Roundable roundable; private final Prepared delegate; - private ArrayRounding(long[] values, int max, Prepared delegate) { - this.values = values; - this.max = max; + public ArrayRounding(Roundable roundable, Prepared delegate) { + this.roundable = roundable; this.delegate = delegate; } @Override public long round(long utcMillis) { - assert values[0] <= utcMillis : utcMillis + " must be after " + values[0]; - int idx = Arrays.binarySearch(values, 0, max, utcMillis); - assert idx != -1 : "The insertion point is before the array! This should have tripped the assertion above."; - assert -1 - idx <= values.length : "This insertion point is after the end of the array."; - if (idx < 0) { - idx = -2 - idx; - } - return values[idx]; + return roundable.floor(utcMillis); } @Override @@ -732,10 +724,7 @@ private class FixedNotToMidnightRounding extends TimeUnitPreparedRounding { @Override public long round(long utcMillis) { - long localTime = offset.utcToLocalTime(utcMillis); - long roundedLocalTime = unit.roundFloor(localTime); - return offset.localToUtcInThisOffset(roundedLocalTime); - // return offset.localToUtcInThisOffset(unit.roundFloor(offset.utcToLocalTime(utcMillis))); + return offset.localToUtcInThisOffset(unit.roundFloor(offset.utcToLocalTime(utcMillis))); } @Override diff --git a/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/CompositeAggregator.java b/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/CompositeAggregator.java index b73a01cfd881f..bc9e824c87186 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/CompositeAggregator.java +++ b/server/src/main/java/org/opensearch/search/aggregations/bucket/composite/CompositeAggregator.java @@ -94,6 +94,7 @@ import java.util.stream.Collectors; import static org.opensearch.search.aggregations.MultiBucketConsumerService.MAX_BUCKET_SETTING; +import static org.opensearch.search.aggregations.bucket.filterrewrite.AggregatorBridge.segmentMatchAll; /** * Main aggregator that aggregates docs from multiple aggregations @@ -564,18 +565,18 @@ private void processLeafFromQuery(LeafReaderContext ctx, Sort indexSortPrefix) t // @Override // protected boolean tryPrecomputeAggregationForLeaf(LeafReaderContext ctx) throws IOException { - // finishLeaf(); // May need to wrap up previous leaf if it could not be precomputed - // return filterRewriteOptimizationContext.tryOptimize(ctx, this::incrementBucketDocCount, segmentMatchAll(context, ctx)); + // finishLeaf(); // May need to wrap up previous leaf if it could not be precomputed + // return filterRewriteOptimizationContext.tryOptimize(ctx, this::incrementBucketDocCount, segmentMatchAll(context, ctx)); // } @Override protected LeafBucketCollector getLeafCollector(LeafReaderContext ctx, LeafBucketCollector sub) throws IOException { boolean optimized = filterRewriteOptimizationContext.tryOptimize( - ctx, - this::incrementBucketDocCount, - segmentMatchAll(context, ctx), - collectableSubAggregators, - sub + ctx, + this::incrementBucketDocCount, + segmentMatchAll(context, ctx), + collectableSubAggregators, + sub ); if (optimized) throw new CollectionTerminatedException(); diff --git a/server/src/main/java/org/opensearch/search/aggregations/bucket/filterrewrite/CompositeDocIdSetIterator.java b/server/src/main/java/org/opensearch/search/aggregations/bucket/filterrewrite/CompositeDocIdSetIterator.java deleted file mode 100644 index 6f43fc3904f1f..0000000000000 --- a/server/src/main/java/org/opensearch/search/aggregations/bucket/filterrewrite/CompositeDocIdSetIterator.java +++ /dev/null @@ -1,112 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -package org.opensearch.search.aggregations.bucket.filterrewrite; - -import org.apache.lucene.search.DocIdSetIterator; - -import java.io.IOException; - -/** - * A composite iterator over multiple DocIdSetIterators where each document - * belongs to exactly one bucket within a single segment. - */ -public class CompositeDocIdSetIterator extends DocIdSetIterator { - private final DocIdSetIterator[] iterators; - - // Track active iterators to avoid scanning all - private final int[] activeIterators; // non-exhausted iterators to its bucket - private int numActiveIterators; // Number of non-exhausted iterators - - private int currentDoc = -1; - private int currentBucket = -1; - - public CompositeDocIdSetIterator(DocIdSetIterator[] iterators) { - this.iterators = iterators; - int numBuckets = iterators.length; - this.activeIterators = new int[numBuckets]; - this.numActiveIterators = 0; - - // Initialize active iterator tracking - for (int i = 0; i < numBuckets; i++) { - if (iterators[i] != null) { - activeIterators[numActiveIterators++] = i; - } - } - } - - @Override - public int docID() { - return currentDoc; - } - - public int getCurrentBucket() { - return currentBucket; - } - - @Override - public int nextDoc() throws IOException { - return advance(currentDoc + 1); - } - - @Override - public int advance(int target) throws IOException { - if (target == NO_MORE_DOCS || numActiveIterators == 0) { - currentDoc = NO_MORE_DOCS; - currentBucket = -1; - return NO_MORE_DOCS; - } - - int minDoc = NO_MORE_DOCS; - int minBucket = -1; - int remainingActive = 0; // Counter for non-exhausted iterators - - // Only check currently active iterators - for (int i = 0; i < numActiveIterators; i++) { - int bucket = activeIterators[i]; - DocIdSetIterator iterator = iterators[bucket]; - - int doc = iterator.docID(); - if (doc < target) { - doc = iterator.advance(target); - } - - if (doc == NO_MORE_DOCS) { - // Iterator is exhausted, don't include it in active set - continue; - } - - // Keep this iterator in our active set - activeIterators[remainingActive] = bucket; - remainingActive++; - - if (doc < minDoc) { - minDoc = doc; - minBucket = bucket; - } - } - - // Update count of active iterators - numActiveIterators = remainingActive; - - currentDoc = minDoc; - currentBucket = minBucket; - - return currentDoc; - } - - @Override - public long cost() { - long cost = 0; - for (int i = 0; i < numActiveIterators; i++) { - DocIdSetIterator iterator = iterators[activeIterators[i]]; - cost += iterator.cost(); - } - return cost; - } -} diff --git a/server/src/main/java/org/opensearch/search/aggregations/bucket/histogram/DateHistogramAggregator.java b/server/src/main/java/org/opensearch/search/aggregations/bucket/histogram/DateHistogramAggregator.java index 42f975a98a67a..ab6f86df66a91 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/bucket/histogram/DateHistogramAggregator.java +++ b/server/src/main/java/org/opensearch/search/aggregations/bucket/histogram/DateHistogramAggregator.java @@ -33,6 +33,7 @@ import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.index.SortedNumericDocValues; +import org.apache.lucene.search.CollectionTerminatedException; import org.apache.lucene.search.DocIdSetIterator; import org.apache.lucene.search.ScoreMode; import org.apache.lucene.util.CollectionUtil; @@ -196,7 +197,7 @@ protected boolean tryPrecomputeAggregationForLeaf(LeafReaderContext ctx) throws return true; } } - return filterRewriteOptimizationContext.tryOptimize(ctx, this::incrementBucketDocCount, segmentMatchAll(context, ctx)); + return false; } @Override diff --git a/server/src/main/java/org/opensearch/search/aggregations/bucket/range/RangeAggregator.java b/server/src/main/java/org/opensearch/search/aggregations/bucket/range/RangeAggregator.java index 0ff3ca683aaab..8588eb0a57d52 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/bucket/range/RangeAggregator.java +++ b/server/src/main/java/org/opensearch/search/aggregations/bucket/range/RangeAggregator.java @@ -31,9 +31,8 @@ package org.opensearch.search.aggregations.bucket.range; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; import org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.search.CollectionTerminatedException; import org.apache.lucene.search.ScoreMode; import org.opensearch.core.ParseField; import org.opensearch.core.common.io.stream.StreamInput; @@ -255,8 +254,6 @@ public boolean equals(Object obj) { private final FilterRewriteOptimizationContext filterRewriteOptimizationContext; - private final Logger logger = LogManager.getLogger(RangeAggregator.class); - public RangeAggregator( String name, AggregatorFactories factories, @@ -312,28 +309,20 @@ public ScoreMode scoreMode() { return super.scoreMode(); } - @Override - protected boolean tryPrecomputeAggregationForLeaf(LeafReaderContext ctx) throws IOException { - if (segmentMatchAll(context, ctx)) { - return filterRewriteOptimizationContext.tryOptimize(ctx, this::incrementBucketDocCount, false); - } - return false; - } + // @Override + // protected boolean tryPrecomputeAggregationForLeaf(LeafReaderContext ctx) throws IOException { + // if (segmentMatchAll(context, ctx)) { + // return filterRewriteOptimizationContext.tryOptimize(ctx, this::incrementBucketDocCount, false); + // } + // return false; + // } @Override public LeafBucketCollector getLeafCollector(LeafReaderContext ctx, final LeafBucketCollector sub) throws IOException { - - if (segmentMatchAll(context, ctx) - && filterRewriteOptimizationContext.tryOptimize( - ctx, - this::incrementBucketDocCount, - false, - collectableSubAggregators, - sub - )) { - throw new CollectionTerminatedException(); - } - + if (segmentMatchAll(context, ctx) + && filterRewriteOptimizationContext.tryOptimize(ctx, this::incrementBucketDocCount, false, collectableSubAggregators, sub)) { + throw new CollectionTerminatedException(); + } final SortedNumericDoubleValues values = valuesSource.doubleValues(ctx); return new LeafBucketCollectorBase(sub, values) {