Add bounded SEA API support for CloudFetch (UseBoundedSeaApi)#1468
Open
gopalldb wants to merge 4 commits into
Open
Add bounded SEA API support for CloudFetch (UseBoundedSeaApi)#1468gopalldb wants to merge 4 commits into
gopalldb wants to merge 4 commits into
Conversation
Part 1 of bounded SEA API compliance for CloudFetch:
1. New connection property UseBoundedSeaApi (default 0/off). When enabled:
- Sends row_offset query parameter on GetResultData requests
- Forces StreamingChunkProvider (which uses next_chunk_index, not
total_chunk_count) even when streaming is explicitly disabled
2. StreamingChunkProvider already uses next_chunk_index for continuation
and end-of-stream detection — no changes needed to its core logic.
3. Legacy RemoteChunkProvider (uses total_chunk_count) is bypassed when
bounded SEA is enabled.
row_offset is derived from the previous link's row_offset + row_count
and sent as a query parameter on /sql/statements/{id}/result/chunks/{idx}.
This is required for future >100GB results and cluster-side fetch.
Co-authored-by: Isaac
Signed-off-by: Gopal Lal <gopal.lal@databricks.com>
During coalesced link refresh, the server may return links for chunks not yet in the provider's map (newly-discovered chunks beyond highestKnownChunkIndex). Previously these were silently skipped. Now: create new chunks from refresh response links, update highestKnownChunkIndex, and set endOfStreamReached from the response's hasMore flag. Follows the per-chunk state-machine reconciliation from the bounded SEA API spec. Co-authored-by: Isaac Signed-off-by: Gopal Lal <gopal.lal@databricks.com>
Fixes 3 gaps found by comparing with the legacy ChunkDownloadTask: 1. Outer catch(Throwable) + exception chaining in finally: uncaught exceptions were lost — the finally block created a generic exception without the original cause. Now captures uncaughtException and chains it, matching ChunkDownloadTask's pattern. 2. Thread context propagation: download threads had no connection context or statementId for telemetry/logging. Now captures caller's context via DatabricksThreadContextHolder and clears in finally. 3. Download timing: added task-level timing log (totalMs, retries) matching ChunkDownloadTask's diagnostics. Also includes the RuntimeException catch (parity with PR databricks#1302). Co-authored-by: Isaac Signed-off-by: Gopal Lal <gopal.lal@databricks.com>
gopalldb
force-pushed
the
feature/bounded-sea-api-cloudfetch
branch
from
e8e6bde to
e9dcd7c
Compare
P0-1: Remove redundant chunk.setStatus(DOWNLOAD_FAILED) in inner catch — defer entirely to finally block. Fixes StreamingChunkDownloadTaskTest. P0-2: Add NEXT_CHANGELOG.md entry under ### Added for UseBoundedSeaApi. P1-1: Call triggerDownloads() after reconciliation creates new chunks from refresh response — prevents newly-discovered chunks sitting PENDING. P1-2/P1-3: Un-gated changes (new chunk creation, EOS from refresh, triggerDownloads) are intentional parity fixes for all EnableStreamingChunkProvider=1 users. EnableStreamingChunkProvider defaults to off, so default users are unaffected. P1-4: Revert RuntimeException from inner catch — DatabricksError is caught by outer catch(Throwable) and fails immediately (no retry), matching ChunkDownloadTask behavior exactly. NPE/ISE won't be retried. P2-1: Always send row_offset (even 0 for chunk 0) when bounded SEA enabled — explicit is safer than relying on server default. P2-3: Update nextLinkFetchIndex after reconciliation to avoid prefetch thread re-fetching chunks already discovered via refresh. P2-5: Add "Requires server support" to connection property help text. Co-authored-by: Isaac Signed-off-by: Gopal Lal <gopal.lal@databricks.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Bounded SEA API compliance for CloudFetch. Brings the driver into alignment with the bounded SEA API contract for OSS drivers. Gated behind
UseBoundedSeaApi=0(default off).When
UseBoundedSeaApi=1is set:Sends
row_offseton GetResultData requests — appended as a query parameter on/sql/statements/{id}/result/chunks/{idx}?row_offset=N. Derived from the previous link'srow_offset + row_count. Required for future >100GB results and cluster-side fetch.Forces
StreamingChunkProvider— which discovers chunk links incrementally vianext_chunk_indexchaining (nottotal_chunk_count). Downloads are still concurrent from presigned URLs — the "streaming" refers to link discovery, not data transfer. This bypasses the legacyRemoteChunkProviderwhich pre-allocates a fixed-size chunk map from the deprecatedtotal_chunk_countmanifest field.Batched link refresh reconciliation — during coalesced link refresh on URL expiry, newly-discovered chunks from the server response are added to the provider's map (previously silently skipped). End-of-stream flag updated from refresh response. Follows per-chunk state-machine reconciliation from the bounded SEA API spec.
Default: off (
UseBoundedSeaApi=0). No behavior change unless explicitly enabled.What already works (no changes needed)
StreamingChunkProvider(the default SEA CloudFetch provider) already:lastLink.getNextChunkIndex()to chain to the next batch of linksnextChunkIndex == nullon the last linktotal_chunk_countfor control flowmaxChunksInMemoryparallel downloads)refetchLockdedup on URL expiryChanges
DatabricksJdbcUrlParams.javaUseBoundedSeaApiproperty (default0)IDatabricksConnectionContext.javaisBoundedSeaApiEnabled()methodDatabricksConnectionContext.javaDatabricksSdkClient.java?row_offset=Non GetResultData when gate is onArrowStreamResult.javaStreamingChunkProviderwhen gate is onSeaChunkLinkFetcher.javaStreamingChunkProvider.javaNO_CHANGELOG=true
Test plan
UseBoundedSeaApi=0(default): zero behavior changeUseBoundedSeaApi=1against live warehouse (follow-up)This pull request was AI-assisted by Isaac.