Skip to content

feat(cloudfetch): apply download settings to kernel sessions - #616

Open
aakash-saravanan-db wants to merge 4 commits into
mainfrom
aakash-saravanan_data/feat/cloudfetch-download-settings-20261114
Open

aakash-saravanan-db wants to merge 4 commits into
mainfrom
aakash-saravanan_data/feat/cloudfetch-download-settings-20261114

Conversation

@aakash-saravanan-db

@aakash-saravanan-db aakash-saravanan-db commented Nov 14, 2026 •

Copy link
Copy Markdown
Collaborator

Feature or gap

Node.js applications could set cloudFetchConcurrentDownloads on Thrift connections, but kernel sessions ignored it.

What this PR addresses

Snapshot the existing setting at session open, default 10 (previously 16), and preserve explicit raw chunk overrides. Values other than positive safe integers raise HiveDriverError at session open. Document when mutations take effect and what the chunk window bounds.

Dependencies

Existing native kernel APIs; no API change. Thrift parity with runAsync: true requires the kernel CloudFetch link-prefetch fix (#466), so a Node.js release must use a native build that includes it. Without that fix, kernel sessions using runAsync: true are slower than Thrift on large multi-page CloudFetch results.

Tests and validation

Build, type-check, repository lint and 440 kernel tests pass. Packaged-driver checks verify controls, errors, precedence, complete results and constrained consumption. With a native built from kernel eabe1a54, five alternating measurements per side after equal warmups give DBSQL Preview submission-through-drain medians for 3M ordered rows: default path Thrift 6.030 s → kernel 5.363 s (-11.1%); runAsync: true Thrift 6.016 s → kernel 5.323 s (-11.5%).

Signed-off-by: Aakash Saravanan <aakash.saravanan@databricks.com>
Signed-off-by: Aakash Saravanan <aakash.saravanan@databricks.com>
Signed-off-by: Aakash Saravanan <aakash.saravanan@databricks.com>
Signed-off-by: Aakash Saravanan <aakash.saravanan@databricks.com>

@peco-review-bot peco-review-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: 1 Low

The change looks correct, with one low finding: the session-open validation fires even when a raw chunk override makes the client setting irrelevant. The mapping, case-insensitive override detection, per-session snapshot and caller-config immutability are tested well. I couldn't check whether the native kernel actually reads cloudfetch_max_chunks_in_memory from sessionConf, because the kernel source isn't in this checkout.

Comment thread lib/kernel/KernelBackend.ts Resolved

@peco-review-bot peco-review-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: 1 Low

Looks good, with one low concern. The kernel now rejects an invalid cloudFetchConcurrentDownloads even when an explicit raw override or disabled CloudFetch means the value is never used. I couldn't check the claims about native-kernel behaviour (clamping above 256, invalid raw overrides warning and falling back to 16) because the kernel source isn't in this checkout.

Comment thread lib/kernel/KernelBackend.ts Resolved

This branch was successfully deployed

1 active deployment
azure-prod — 009a51e3 Deployed Nov 14, 2026 by aakash-saravanan-db via e2e-test (24.22.1, 24_22_1) #2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant