Repository navigation
fix(sparkconnect): use server.cores as server pod CPU request - #3252
Merged
google-oss-prow[bot] merged 6 commits intoOct 28, 2026
Merged
google-oss-prow[bot] merged 6 commits into
google-oss-prow[bot] merged 6 commits into
Conversation
Contributor
Author
|
@nabuskey @tariq-hasan can you plz review this PR for me. I will attend to ci-failures (if any) tomorrow morning, off to bed now |
Signed-off-by: Vikas Saxena <Vikas.Saxena.2006@gmail.com>
Signed-off-by: Vikas Saxena <Vikas.Saxena.2006@gmail.com>
Signed-off-by: Vikas Saxena <Vikas.Saxena.2006@gmail.com>
Signed-off-by: Vikas Saxena <Vikas.Saxena.2006@gmail.com>
…he documentation change was generated with help from Claude. Signed-off-by: Vikas Saxena <Vikas.Saxena.2006@gmail.com>
vikas-saxena02
force-pushed
the
fix-sparkconnect-server-cores
branch
from
October 27, 2026 18:45
edacd09 to
1573caf
Compare
Contributor
Author
|
@nabuskey @vjanelle @tariq-hasan can i please get a review on this one as well |
nabuskey
reviewed
Oct 27, 2026
Co-authored-by: Manabu McCloskey <manabu.mccloskey@gmail.com> Signed-off-by: Vikas Saxena <90456542+vikas-saxena02@users.noreply.github.com>
Contributor
Author
|
/retest |
1 similar comment
Contributor
Author
|
/retest |
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: nabuskey The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
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.
Purpose of this PR
Fixes #3112.
spec.server.coreswas passed to the Connect server asspark.driver.cores, which has no effect in client mode, so the field did nothing. As agreed on the issue,coresis kept and now acts as a shorthand forspec.server.coreRequest(added in #3096).Proposed changes:
spark.driver.coresto the Connect server.server.coreswhenserver.coreRequestis unset.coreRequeststill takes precedence.Coresfield description and regenerate the API artifacts, the user guide and the example.Change Category
Rationale
See the reproduction and discussion on #3112. Executor behaviour is unchanged:
executor.coresstill maps tospark.executor.cores.Checklist
Additional Notes
For reviewers:
server.coresnow overrides a CPU request set inserver.template, the same waycoreRequestdoes. A spec withcores: 1and a template CPU limit below 1 was accepted before, becausecoreshad no effect, and is now rejected by the webhook. Is that acceptable, or shouldcoresfall back below the template instead?coressets only the CPU request, not the limit, per the issue discussion. The driver'savailableProcessors()follows the CPU limit, so JVM parallelism is unchanged unlesscoreLimitis also set.server.memory: left unchanged in this PR. It still sizes the heap viaspark.driver.memorybut not the pod. Happy to follow up separately if wanted.AI assistance disclosure: The documentation changes and this PR description were drafted with help from Claude (Anthropic) and reviewed by me. The unit tests had local failures those were remediated with help from Claude as well.