Adding MergeStats support for Composite Engine #20318

darjisagar7 · 2025-12-24T13:42:11Z

Description

Adding MergeStats support for Composite Engine

curl --location --request GET 'localhost:9200/index-7/_stats/merge' | jq '.'
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  1302  100  1302    0     0     88      0  0:00:14  0:00:14 --:--:--   289
{
  "_shards": {
    "total": 1,
    "successful": 1,
    "failed": 0
  },
  "_all": {
    "primaries": {
      "merges": {
        "current": 0,
        "current_docs": 0,
        "current_size_in_bytes": 0,
        "total": 1,
        "total_time_in_millis": 21,
        "total_docs": 66,
        "total_size_in_bytes": 11,
        "total_stopped_time_in_millis": 0,
        "total_throttled_time_in_millis": 0,
        "total_auto_throttle_in_bytes": 0,
        "unreferenced_file_cleanups_performed": 0
      }
    },
    "total": {
      "merges": {
        "current": 0,
        "current_docs": 0,
        "current_size_in_bytes": 0,
        "total": 1,
        "total_time_in_millis": 21,
        "total_docs": 66,
        "total_size_in_bytes": 11,
        "total_stopped_time_in_millis": 0,
        "total_throttled_time_in_millis": 0,
        "total_auto_throttle_in_bytes": 0,
        "unreferenced_file_cleanups_performed": 0
      }
    }
  },
  "indices": {
    "index-7": {
      "uuid": "AcXSnbxCSdWWNLK3AwFQ_A",
      "primaries": {
        "merges": {
          "current": 0,
          "current_docs": 0,
          "current_size_in_bytes": 0,
          "total": 1,
          "total_time_in_millis": 21,
          "total_docs": 66,
          "total_size_in_bytes": 11,
          "total_stopped_time_in_millis": 0,
          "total_throttled_time_in_millis": 0,
          "total_auto_throttle_in_bytes": 0,
          "unreferenced_file_cleanups_performed": 0
        }
      },
      "total": {
        "merges": {
          "current": 0,
          "current_docs": 0,
          "current_size_in_bytes": 0,
          "total": 1,
          "total_time_in_millis": 21,
          "total_docs": 66,
          "total_size_in_bytes": 11,
          "total_stopped_time_in_millis": 0,
          "total_throttled_time_in_millis": 0,
          "total_auto_throttle_in_bytes": 0,
          "unreferenced_file_cleanups_performed": 0
        }
      }
    }
  }
}

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

Check List

Functionality includes testing.
API changes companion pull request created, if applicable.
Public documentation issue/PR created, if applicable.

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.

coderabbitai · 2025-12-24T13:42:16Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

github-actions · 2025-12-24T13:59:34Z

❌ Gradle check result for a73ce78: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

dhwanilpatel · 2025-12-26T06:55:17Z

server/src/main/java/org/opensearch/index/engine/exec/merge/MergeScheduler.java

+
+    public synchronized double getIORateLimitMBPerSec() {
+        // TODO: return the value from the Rust if the doAutoIOThrottle is enabled else INFINITY
+        return Double.POSITIVE_INFINITY;


Can we return -1 if it is not supported?

With Lucene this is the number populated, so using the same.

dhwanilpatel · 2025-12-26T06:59:30Z

server/src/main/java/org/opensearch/index/engine/exec/merge/OneMerge.java

+    public long getTotalSizeInBytes() {
+        long totalSize = 0;
+        for (CatalogSnapshot.Segment segment : segmentsToMerge) {
+            for (WriterFileSet writerFileSet : segment.getDFGroupedSearchableFiles().values()) {
+                totalSize += writerFileSet.getTotalSize();
+            }
+        }
+        return totalSize;
+    }
+
+    public long getTotalNumDocs() {
+        long totalDocs = 0;
+        for (CatalogSnapshot.Segment segment : segmentsToMerge) {
+            for (WriterFileSet writerFileSet : segment.getDFGroupedSearchableFiles().values()) {
+                totalDocs += writerFileSet.getNumRows();
+            }
+        }
+        return totalDocs;
+    }
+


Instead of calculating it everytime, we can calculate this during oneMerge creation and store.

dhwanilpatel · 2025-12-26T07:02:43Z

server/src/main/java/org/opensearch/index/engine/exec/WriterFileSet.java

+    public long getNumRows() {
+        return numRows;
+    }


We will need to use this num of rows in CompositeMergePolicy as well? Can you update that logic as well?

dhwanilpatel · 2025-12-26T07:03:44Z

server/src/main/java/org/opensearch/index/engine/exec/coord/CompositeEngine.java

+    public DocsStats docStats() {
+        return null;
+    }
+
+    @Override
+    public SegmentsStats segmentsStats(boolean includeSegmentFileSizes, boolean includeUnloadedSegments) {
+        return null;
+    }
+


Please check index stats PR as well to see both stats are following same approach?

github-actions · 2025-12-29T12:16:12Z

❌ Gradle check result for 1c26135: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

raghuvanshraj · 2025-12-31T08:28:40Z

...quet-data-format/src/main/java/com/parquet/parquetdataformat/bridge/NativeParquetWriter.java

    public void close() {
        try {
-            RustBridge.closeWriter(filePath);
+            metadata = RustBridge.closeWriter(filePath);


where is this metadata being used? getMetadata should not be called after the writer is closed.

It is used in the flush function in VSRManager class

raghuvanshraj · 2025-12-31T08:30:27Z

modules/parquet-data-format/src/main/java/com/parquet/parquetdataformat/vsr/VSRManager.java

            try (ArrowExport export = currentVSR.exportToArrow()) {
                writer.write(export.getArrayAddress(), export.getSchemaAddress());
                writer.close();
+                writerClosed = true;


The writer is not closed at this point right?

it is closed.

raghuvanshraj · 2025-12-31T08:34:28Z

...es/parquet-data-format/src/main/java/com/parquet/parquetdataformat/writer/ParquetWriter.java

            .writerGeneration(writerGeneration)
-            .addFile(file.getFileName().toString())
+            .addFile(filePath.getFileName().toString())
+            .addNumRows(parquetFileMetadata.numRows())


Why not add the entire metadata? We can reuse it later to store checksums as well?

ParquetFileMetadata is created in plugin and this code is in core.

raghuvanshraj · 2025-12-31T08:37:05Z

modules/parquet-data-format/src/main/rust/src/lib.rs

+                    // No writer was found, but this is not necessarily an error
+                    // Return null to indicate no metadata available
+                    JObject::null().into_raw()


Ideally if close should never be called for a closed writer. If some flow is doing that today, its a problem. Also, not that metadata is being returned, we should throw an error in this flow. Can take it as a follow up too. I don't want any incorrect conversion happening due to empty/null metadata.

shank9918 · 2026-01-05T05:37:40Z

server/src/main/java/org/opensearch/index/shard/IndexShard.java


    public MergeStats mergeStats() {
-        final StatsHolder engine = getStatsHolderOrNull();
+        final StatsHolder engine = (StatsHolder) getCompositeEngineOrNull();


this needs to be modified. take the latest change and use getStatsHolderOrNull as it was before.

shank9918 · 2026-01-05T05:37:58Z

server/src/main/java/org/opensearch/index/shard/IndexShard.java

+     * closed.
+     */
+    protected CompositeEngine getCompositeEngineOrNull() {
+        return this.currentCompositeEngineReference.get();


this is not required.

shank9918 · 2026-01-05T05:41:29Z

server/src/main/java/org/opensearch/index/engine/exec/coord/CompositeEngine.java

+
+    @Override
+    public MergeStats getMergeStats() {
+        return mergeScheduler.stats();


We will need details for on going merges in the segment stats api.

github-actions · 2026-01-06T09:05:50Z

❌ Gradle check result for f18a032: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

github-actions · 2026-01-06T09:23:38Z

❌ Gradle check result for e807572: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

github-actions · 2026-01-07T10:38:58Z

❌ Gradle check result for f713e09: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

raghuvanshraj · 2026-01-07T11:37:05Z

...quet-data-format/src/main/java/com/parquet/parquetdataformat/bridge/NativeParquetWriter.java

+        if (!writerClosed.get()) {
+            try {
+                metadata = RustBridge.closeWriter(filePath);
+                writerClosed.set(true);


We should use compareAndSet here.

Also, this should be done before the native call, and only if compareAndSet returns true.

Signed-off-by: Sagar Darji <[email protected]>

github-actions · 2026-01-07T13:18:02Z

❌ Gradle check result for 51343dc: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

darjisagar7 requested review from a team, Bukhtawar, CEHENKLE, Rishikesh1159, anasalkouz, andrross, ashking94, cwperks, dbwiddis, gbbafna, jed326, kotwanikunal, mch2, msfroh, owaiskazi19, reta, sachinpkale, saratvemulapalli, shwetathareja and sohami as code owners December 24, 2025 13:42

darjisagar7 force-pushed the MergeStats branch from 0f6cb7a to a73ce78 Compare December 24, 2025 13:46

dhwanilpatel reviewed Dec 26, 2025

View reviewed changes

darjisagar7 force-pushed the MergeStats branch from a73ce78 to 1c26135 Compare December 29, 2025 12:03

dhwanilpatel approved these changes Dec 29, 2025

View reviewed changes

raghuvanshraj suggested changes Dec 31, 2025

View reviewed changes

raghuvanshraj added 2 commits January 6, 2026 13:36

Initial changes for getting record count on ArrowWriter close

a31b846

Adding getFileMetadata JNI

ae43b28

raghuvanshraj added 2 commits January 6, 2026 13:36

ongoing

389633e

Changing ParquetFileMetadata to record

8557d79

darjisagar7 force-pushed the MergeStats branch from 1c26135 to f18a032 Compare January 6, 2026 08:56

shank9918 reviewed Jan 6, 2026

View reviewed changes

darjisagar7 force-pushed the MergeStats branch from f18a032 to e807572 Compare January 6, 2026 09:11

darjisagar7 force-pushed the MergeStats branch from e807572 to f713e09 Compare January 7, 2026 10:32

raghuvanshraj suggested changes Jan 7, 2026

View reviewed changes

[Merge] Adding MergeStats support for Composite Engine

51343dc

Signed-off-by: Sagar Darji <[email protected]>

darjisagar7 force-pushed the MergeStats branch from f713e09 to 51343dc Compare January 7, 2026 13:11

bharath-techie merged commit befa361 into opensearch-project:feature/datafusion Jan 7, 2026
7 of 31 checks passed

Adding MergeStats support for Composite Engine #20318

Adding MergeStats support for Composite Engine #20318

Conversation

darjisagar7 commented Dec 24, 2025

Description

Related Issues

Check List

Uh oh!

coderabbitai bot commented Dec 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

github-actions bot commented Dec 24, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Dec 29, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jan 6, 2026

Uh oh!

github-actions bot commented Jan 6, 2026

Uh oh!

github-actions bot commented Jan 7, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jan 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

coderabbitai bot commented Dec 24, 2025 •

edited

Loading