-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Refactor stream handling for DF execution #20076
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor stream handling for DF execution #20076
Conversation
|
❌ Gradle check result for e8ce038: 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? |
|
❌ Gradle check result for 645adec: 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? |
|
❌ Gradle check result for 0a6d802: 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? |
|
❌ Gradle check result for e61f803: 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? |
|
❌ Gradle check result for 19fecf4: 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? |
|
❌ Gradle check result for 7f10db4: 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? |
...ns/engine-datafusion/src/main/java/org/opensearch/datafusion/search/RecordBatchIterator.java
Show resolved
Hide resolved
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
|
❌ Gradle check result for a2f6068: 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? |
|
❌ Gradle check result for cadf064: 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? |
|
❌ Gradle check result for ed78205: 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? |
3e0daa3
into
opensearch-project:feature/datafusion
Description
DatafusionEngine: implements AutoCloseable, adds a shared RootAllocator (created in the constructor, closed in close()), removes per-call RootAllocator creation, switches collectors from RecordBatchStream to RecordBatchIterator, updates query/fetch phases to consume an iterator rather than looping on loadNextBatch.
RecordBatchStream: rewritten to use a StreamHandle (instead of raw native pointers), create a child allocator from a parent allocator, expose schema initialization via a CompletableFuture and VectorSchemaRoot creation, delegate loadNextBatch to the StreamHandle, implement AutoCloseable.
New StreamHandle class: extends NativeHandle, stores the runtime pointer, centralizes JNI interactions (streamGetSchema, streamNext, streamClose), provides getSchema(...) and loadNextBatch(...) methods that return CompletableFutures, and converts native Arrow schema/arrays into Arrow Schema/VectorSchemaRoot contents.
New RecordBatchIterator: adds an Iterator that synchronously drives RecordBatchStream.loadNextBatch().join(), simplifying consumers by replacing manual async loop logic.
Resource-management changes: moves allocator lifecycle to engine-level (shared root allocator + per-stream child allocators), avoids closing allocators inside query/fetch code, and consolidates native stream cleanup into StreamHandle/AutoCloseable flows.
Related Issues
Resolves #[Issue number to be closed when this PR is merged]
Check List
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.