diff --git a/.github/workflows/custom_CI.yml b/.github/workflows/custom_CI.yml new file mode 100644 index 0000000000000..dd072c09a9233 --- /dev/null +++ b/.github/workflows/custom_CI.yml @@ -0,0 +1,46 @@ +name: Custom CI + +on: + push: + branches: [main] + pull_request: + branches: [main] + +jobs: + build-and-test: + runs-on: ubuntu-latest + + steps: + # 1. Checkout source + - name: Checkout repository + uses: actions/checkout@v4 + + # 2. Set up JDK 21 + - name: Set up JDK 21 + uses: actions/setup-java@v3 + with: + distribution: temurin + java-version: 21 + cache: gradle + + # 3. Make gradlew executable + - name: Grant execute permission for gradlew + run: chmod +x gradlew + + # 4. Build local distribution + - name: Build local distribution + run: ./gradlew localDistro --stacktrace --no-daemon + + # 5. Run unit tests + - name: Run server tests + run: ./gradlew :server:test --stacktrace --no-daemon + + # 6. Upload all test reports + - name: Archive test reports + if: always() + uses: actions/upload-artifact@v4 + with: + name: test-reports-java-21 + path: | + **/build/test-results/**/* + **/build/reports/tests/**/* \ No newline at end of file diff --git a/BUILDING.md b/BUILDING.md index 1afe6a5e833b4..846e1dcfc242c 100644 --- a/BUILDING.md +++ b/BUILDING.md @@ -4,7 +4,7 @@ Building Elasticsearch with Gradle Elasticsearch is built using the [Gradle](https://gradle.org/) open source build tools. This document provides a general guidelines for using and working on the Elasticsearch build logic. - +test ## Build logic organisation The Elasticsearch project contains 3 build-related projects that are included into the Elasticsearch build as a [composite build](https://docs.gradle.org/current/userguide/composite_builds.html). diff --git a/JenkinsFile b/JenkinsFile new file mode 100644 index 0000000000000..72914a07f7357 --- /dev/null +++ b/JenkinsFile @@ -0,0 +1,39 @@ +pipeline { + agent any + + tools { + jdk 'jdk_21' + } + + options { + skipDefaultCheckout() + } + + stages { + stage('Checkout repository') { + steps { + git branch: 'main', + url: 'https://github.com/Jingjia01/elasticsearch.git' + } + } + + stage('Build local distribution') { + steps { + bat 'gradlew.bat localDistro --stacktrace --no-daemon' + } + } + + stage('Run server tests') { + steps { + bat 'gradlew.bat :server:test --stacktrace --no-daemon' + } + } + } + + post { + always { + junit '**/build/test-results/**/*.xml' + archiveArtifacts artifacts: '**/build/reports/tests/**/*', allowEmptyArchive: true + } + } +} diff --git a/docs/refactor-zero-bucket-notes.md b/docs/refactor-zero-bucket-notes.md new file mode 100644 index 0000000000000..63581e1261c42 --- /dev/null +++ b/docs/refactor-zero-bucket-notes.md @@ -0,0 +1,54 @@ +# Refactor Notes: ZeroBucket (Task 1) + +## Summary + +Converted implicit lazy loading (sentinel values Long.MAX_VALUE / Double.NaN) into explicit state flags (indexComputed, thresholdComputed). Added static factories (fromThreshold, fromIndexAndScale) while retaining the original public constructor (deprecated) and the original APIs (minimalEmpty, minimalWithCount, merge, collapseOverlappingBuckets\*, compareZeroThreshold). Added equals/hashCode/toString for value semantics and stronger testability. + +## Key Changes + +| Aspect | Before | After | +| ----------------- | --------------------------------------- | ------------------------------------------------------------------------------- | +| Lazy tracking | Sentinels (Long.MAX_VALUE / Double.NaN) | Booleans (indexComputed / thresholdComputed) | +| Constructors | Public constructor only | + fromThreshold / fromIndexAndScale (constructor kept, deprecated) | +| API compatibility | merge, collapse\*, minimalWithCount | Preserved (restored where removed) | +| Value semantics | None | equals, hashCode, toString | +| Tests | None for ZeroBucket | Added ZeroBucketTests (lazy, merge, minimal, invalid input, collapse, equality) | + +## Rationale + +Explicit flags improve readability and reduce mental overhead. Factories clarify intent (threshold-driven vs index-driven). Value semantics simplify reasoning in future merging logic and aid debugging (toString includes internal state). + +## Risks & Mitigation + +| Risk | Mitigation | +| ---------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------- | +| Off-by-one index computation regression | Preserved original +1 logic in index() lazy path | +| Breaking callers using removed methods | Restored original methods (merge, minimalWithCount, collapseOverlappingBuckets\*, compareZeroThreshold) | +| Hidden performance cost from forcing both computations in equals | Acceptable; equality rarely on hot path; keeps simplicity | + +## Tests Added + +- testFromThresholdLazyIndex +- testFromIndexLazyThreshold +- testMinimalEmptySingleton +- testMinimalWithCount +- testMergeKeepsLargerThreshold +- testInvalidNegativeThreshold +- testEqualityAndHashStable +- testCollapseNoOverlapReturnsSame +- testToStringContainsKeyFields + +All passed with: +`./gradlew :libs:exponential-histogram:test --tests "*ZeroBucketTests"` + +## Evidence Pointers + +- Commit(s): refactor(zerobucket)... (and fix if needed) +- PR: refactor: ZeroBucket explicit lazy state & value semantics (Task 1) +- Build screenshot: ZeroBucketTests BUILD SUCCESSFUL +- Diff: removal of sentinel logic / introduction of flags & factories + +## Future Follow-up (Not in Task 1) + +- Remove deprecated constructor after downstream code migrates to factories. +- Consider benchmarking overhead of toString in large diagnostics. diff --git a/gradle.properties b/gradle.properties index 7c781d859cea6..f0e0bf847246e 100644 --- a/gradle.properties +++ b/gradle.properties @@ -22,3 +22,6 @@ org.gradle.java.installations.fromEnv=RUNTIME_JAVA_HOME # if configuration cache enabled then enable parallel support too org.gradle.configuration-cache.parallel=true + +org.gradle.java.installations.auto-download=true +org.gradle.java.installations.paths=C:/jenkins/jdks \ No newline at end of file diff --git a/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/DownscaleStats.java b/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/DownscaleStats.java index 4cf8f6f89d18f..adb46848252ec 100644 --- a/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/DownscaleStats.java +++ b/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/DownscaleStats.java @@ -29,11 +29,50 @@ import static org.elasticsearch.exponentialhistogram.ExponentialHistogram.MAX_INDEX_BITS; import static org.elasticsearch.exponentialhistogram.ExponentialHistogram.MIN_INDEX; +/** + * Interface for collecting downscale data by adding bucket pairs and resetting state. + */ +interface DownscaleDataCollector { + /** + * Resets the data structure to its initial state. + */ + void reset(); + + /** + * Adds a pair of neighboring bucket indices to track for potential merging. + * + * @param previousBucketIndex the index of the previous bucket + * @param currentBucketIndex the index of the current bucket + */ + void add(long previousBucketIndex, long currentBucketIndex); +} + +/** + * Interface for querying downscale statistics and computing scale reductions. + */ +interface DownscaleQueryProvider { + /** + * Returns the number of buckets that will be merged after applying the given scale reduction. + * + * @param reduction the scale reduction factor + * @return the number of buckets that will be merged + */ + int getCollapsedBucketCountAfterScaleReduction(int reduction); + + /** + * Returns the required scale reduction to reduce the number of buckets by at least the given amount. + * + * @param desiredCollapsedBucketCount the target number of buckets to collapse + * @return the required scale reduction + */ + int getRequiredScaleReductionToReduceBucketCountBy(int desiredCollapsedBucketCount); +} + /** * A data structure for efficiently computing the required scale reduction for a histogram to reach a target number of buckets. * This works by examining pairs of neighboring buckets and determining at which scale reduction they would merge into a single bucket. */ -class DownscaleStats { +class DownscaleStats implements DownscaleDataCollector, DownscaleQueryProvider { static final long SIZE = RamUsageEstimator.shallowSizeOf(DownscaleStats.class) + RamEstimationUtil.estimateIntArray(MAX_INDEX_BITS); @@ -44,7 +83,8 @@ class DownscaleStats { /** * Resets the data structure to its initial state. */ - void reset() { + @Override + public void reset() { Arrays.fill(collapsedBucketCount, 0); } @@ -54,7 +94,8 @@ void reset() { * @param previousBucketIndex the index of the previous bucket * @param currentBucketIndex the index of the current bucket */ - void add(long previousBucketIndex, long currentBucketIndex) { + @Override + public void add(long previousBucketIndex, long currentBucketIndex) { assert currentBucketIndex > previousBucketIndex; assert previousBucketIndex >= MIN_INDEX && previousBucketIndex <= MAX_INDEX; assert currentBucketIndex <= MAX_INDEX; @@ -84,7 +125,8 @@ void add(long previousBucketIndex, long currentBucketIndex) { * @param reduction the scale reduction factor * @return the number of buckets that will be merged */ - int getCollapsedBucketCountAfterScaleReduction(int reduction) { + @Override + public int getCollapsedBucketCountAfterScaleReduction(int reduction) { assert reduction >= 0 && reduction <= MAX_INDEX_BITS; int totalCollapsed = 0; for (int i = 0; i < reduction; i++) { @@ -99,7 +141,8 @@ int getCollapsedBucketCountAfterScaleReduction(int reduction) { * @param desiredCollapsedBucketCount the target number of buckets to collapse * @return the required scale reduction */ - int getRequiredScaleReductionToReduceBucketCountBy(int desiredCollapsedBucketCount) { + @Override + public int getRequiredScaleReductionToReduceBucketCountBy(int desiredCollapsedBucketCount) { assert desiredCollapsedBucketCount >= 0; if (desiredCollapsedBucketCount == 0) { return 0; diff --git a/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ZeroBucket.java b/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ZeroBucket.java index 6a9d24e87c0e1..9c7fb36393cab 100644 --- a/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ZeroBucket.java +++ b/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ZeroBucket.java @@ -34,66 +34,100 @@ /** * Represents the bucket for values around zero in an exponential histogram. * The range of this bucket is {@code [-zeroThreshold, +zeroThreshold]}. - * To allow efficient comparison with bucket boundaries, this class internally - * represents the zero threshold as a exponential histogram bucket index with a scale, - * computed via {@link ExponentialScaleUtils#computeIndex(double, int)}. + * + * Refactor (Task 1): + * - Added static factories (fromThreshold, fromIndexAndScale) while keeping + * original public constructor + * - Introduced explicit lazy flags (indexComputed, thresholdComputed) instead + * of sentinels + * - Added value semantics (equals, hashCode, toString) + * - Preserved original API: minimalEmpty, minimalWithCount, merge, + * collapseOverlappingBuckets[ForAll], compareZeroThreshold */ public final class ZeroBucket { public static final long SHALLOW_SIZE = RamUsageEstimator.shallowSizeOfInstance(ZeroBucket.class); - /** - * The exponential histogram scale used for {@link #index} - */ private final int scale; - - /** - * The exponential histogram bucket index whose upper boundary corresponds to the zero threshold. - * Might be computed lazily from {@link #realThreshold}, uses {@link Long#MAX_VALUE} as placeholder in this case. - */ private long index; - - /** - * Might be computed lazily from {@link #realThreshold}, uses {@link Double#NaN} as placeholder in this case. - */ private double realThreshold; - private final long count; - // A singleton for an empty zero bucket with the smallest possible threshold. - private static final ZeroBucket MINIMAL_EMPTY = new ZeroBucket(MIN_INDEX, MIN_SCALE, 0); + + // Explicit lazy flags + private boolean indexComputed; + private boolean thresholdComputed; + + // Original minimal empty singleton (index known, threshold lazy) + private static final ZeroBucket MINIMAL_EMPTY = new ZeroBucket( + MIN_INDEX, + MIN_SCALE, + 0L, + true, + false, + Double.NaN); + + /* + * ===================== Original Public Constructor (kept) + * ===================== + */ /** - * Creates a new zero bucket with a specific threshold and count. - * - * @param zeroThreshold The threshold defining the bucket's range [-zeroThreshold, +zeroThreshold]. - * @param count The number of values in the bucket. + * Original public constructor (threshold authoritative). Kept for source + * compatibility. + * Deprecated in favor of {@link #fromThreshold(double, long)}. */ + @Deprecated public ZeroBucket(double zeroThreshold, long count) { - assert zeroThreshold >= 0.0 : "zeroThreshold must not be negative"; - this.index = Long.MAX_VALUE; // compute lazily when needed + if (zeroThreshold < 0.0) { + throw new IllegalArgumentException("zeroThreshold must be >= 0 (was " + zeroThreshold + ")"); + } this.scale = MAX_SCALE; - this.realThreshold = zeroThreshold; this.count = count; + this.realThreshold = zeroThreshold; + this.index = 0L; // placeholder until computed + this.indexComputed = false; + this.thresholdComputed = true; } - private ZeroBucket(long index, int scale, long count) { - assert index >= MIN_INDEX && index <= MAX_INDEX : "index must be in range [" + MIN_INDEX + ", " + MAX_INDEX + "]"; - assert scale >= MIN_SCALE && scale <= MAX_SCALE : "scale must be in range [" + MIN_SCALE + ", " + MAX_SCALE + "]"; - this.index = index; - this.scale = scale; - this.realThreshold = Double.NaN; // compute lazily when needed - this.count = count; + /* ===================== Factory Methods ===================== */ + + /** + * Create a ZeroBucket from an explicit threshold (index lazy). + */ + public static ZeroBucket fromThreshold(double zeroThreshold, long count) { + if (zeroThreshold < 0.0) { + throw new IllegalArgumentException("zeroThreshold must be >= 0 (was " + zeroThreshold + ")"); + } + return new ZeroBucket( + 0L, + MAX_SCALE, + count, + false, + true, + zeroThreshold); } - private ZeroBucket(double realThreshold, long index, int scale, long count) { - this.realThreshold = realThreshold; - this.index = index; - this.scale = scale; - this.count = count; + /** + * Create a ZeroBucket from an index + scale (threshold lazy). + */ + public static ZeroBucket fromIndexAndScale(long index, int scale, long count) { + if (scale < MIN_SCALE || scale > MAX_SCALE) { + throw new IllegalArgumentException("scale out of range: " + scale); + } + if (index < MIN_INDEX || index > MAX_INDEX) { + throw new IllegalArgumentException("index out of range: " + index); + } + return new ZeroBucket( + index, + scale, + count, + true, + false, + Double.NaN); } /** - * @return A singleton instance of an empty zero bucket with the smallest possible threshold. + * @return singleton empty minimal bucket. */ public static ZeroBucket minimalEmpty() { return MINIMAL_EMPTY; @@ -101,112 +135,129 @@ public static ZeroBucket minimalEmpty() { /** * Creates a zero bucket with the smallest possible threshold and a given count. - * - * @param count The number of values in the bucket. - * @return A new {@link ZeroBucket}. + * If count == 0 returns the singleton. */ public static ZeroBucket minimalWithCount(long count) { if (count == 0) { return MINIMAL_EMPTY; - } else { - return new ZeroBucket(MINIMAL_EMPTY.zeroThreshold(), MINIMAL_EMPTY.index(), MINIMAL_EMPTY.scale(), count); } + // Resolve lazy threshold & index of singleton + double threshold = MINIMAL_EMPTY.zeroThreshold(); + long idx = MINIMAL_EMPTY.index(); + return resolved(threshold, idx, MINIMAL_EMPTY.scale(), count); + } + + /* ===================== Private Constructors ===================== */ + + private ZeroBucket( + long index, + int scale, + long count, + boolean indexComputed, + boolean thresholdComputed, + double realThreshold) { + this.index = index; + this.scale = scale; + this.count = count; + this.indexComputed = indexComputed; + this.thresholdComputed = thresholdComputed; + this.realThreshold = realThreshold; + } + + private static ZeroBucket resolved(double threshold, long index, int scale, long count) { + return new ZeroBucket(index, scale, count, true, true, threshold); + } + + /* ===================== Accessors ===================== */ + + public long count() { + return count; + } + + public int scale() { + return scale; + } + + /** + * Returns index; if threshold authoritative, compute with +1 rule (matches + * original code). + */ + public long index() { + computeIndexIfNeeded(); + return index; } /** - * @return The value of the zero threshold. + * Returns threshold; if index authoritative compute it lazily. */ public double zeroThreshold() { - if (Double.isNaN(realThreshold)) { - realThreshold = exponentiallyScaledToDoubleValue(index(), scale()); - } + computeThresholdIfNeeded(); return realThreshold; } - public long index() { - if (index == Long.MAX_VALUE) { - index = computeIndex(zeroThreshold(), scale()) + 1; + /* ===================== Lazy Computation Helpers ===================== */ + + private void computeIndexIfNeeded() { + if (indexComputed == false) { + index = computeIndex(realThreshold, scale) + 1; + indexComputed = true; } - return index; } - public int scale() { - return scale; + private void computeThresholdIfNeeded() { + if (thresholdComputed == false) { + realThreshold = exponentiallyScaledToDoubleValue(index(), scale); + thresholdComputed = true; + } } - public long count() { - return count; + /* Package-private for tests */ + boolean isIndexComputed() { + return indexComputed; + } + + boolean isThresholdComputed() { + return thresholdComputed; + } + + /* ===================== Original Functional API ===================== */ + + public int compareZeroThreshold(ZeroBucket other) { + return compareExponentiallyScaledValues(index(), scale(), other.index(), other.scale()); } - /** - * Merges this zero bucket with another one. - *