ZING-43245: parallelize file signing - #17
Open
KostashchukIryna wants to merge 5 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
Parallelizes URL signing for table and CDF queries to reduce latency.
Changes:
- Adds parallel signing using a shared thread pool.
- Preserves action ordering and pagination.
- Adds configurable signing concurrency.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
DeltaSharedTable.scala |
Implements parallel file signing. |
DeltaSharingService.scala |
Configures the shared signing pool. |
ServerConfig.scala |
Adds the pool-size setting. |
manifests/base/configmap.yaml |
Sets deployment pool size to 32. |
Suppressed comments (1)
server/src/main/scala/io/delta/standalone/internal/DeltaSharedTable.scala:1223
newFixedThreadPoolbounds only the worker count; itsLinkedBlockingQueueis unbounded. BecauseparallelSigneagerly submits one future per file (up to 10,000 per request), concurrent requests can accumulate an unbounded backlog, retain all query state, and starve later requests. Timed-out futures also remain queued and continue running. Use bounded submission with explicit backpressure/rejection handling, or process each request in bounded batches.
// Shared, bounded thread pool for parallel file signing across all tables/requests.
private lazy val signingExecutionContext: ExecutionContext = ExecutionContext.fromExecutorService(
java.util.concurrent.Executors.newFixedThreadPool(signingThreadPoolSize.get()))
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+77
to
+78
| // The number of threads used to sign file URLs in parallel (queryTable/queryTableChanges). | ||
| @BeanProperty var signingThreadPoolSize: Int |
bharatZen
approved these changes
Aug 20, 2026
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
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.
Tested in Dev comparing with current master version running. Used identical CDF version ranges to get the same number of rows. Verified that the number of rows is the same to check if there is a regression. So I saw an improvement in time spent on signing (- ~70%) based on perf logs that were added before. It's more noticable change in smaller queries though, as in big ones cdfReplay and cdcSpecBuild are still quite high. Maybe we should look if that can be optimized