Repository navigation
Conversation
…the session The deprecation warning for spark.comet.exec.memoryPool.fraction only checked the SparkConf in the driver plugin, but the value is read from the session conf for each query. A value set with SET or spark.conf.set, or in an application that registers the session extension without the plugin, got no warning. CometExecRule now checks the session conf too, and the warning is logged once per JVM. Also state in the memory tuning guide that fair_unified limits each task to the fraction times spark.memory.offHeap.size. Part of apache#6187 Co-authored-by: Copilot <[email protected]> Copilot-Session: 1da6c065-9ae0-4ad5-a84d-12fe9e45d920
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.
Which issue does this PR close?
Part of #6187. This addresses item 2 and the remaining wording in item 1. Item 3 (tests that use the fraction to force spills) is left open; see the note below.
Rationale for this change
The deprecation warning for
spark.comet.exec.memoryPool.fractiononly checks the SparkConf in the driver plugin, butCometExecIterator.getMemoryConfigreads the value from the session conf for each query. A value set withSETorspark.conf.set, or in an application that registers the session extension without the plugin, takes effect without a warning.Item 1 is mostly resolved by #7096 and the guide rewrites that followed it. The
fair_unifiedpool now limits each task tomin(spark.memory.offHeap.size / running_tasks, pool_size), and both guides and the config doc already describe the fraction as a per-task cap. The user guide still did not say what the cap is.What changes are included in this PR?
CometDriverPlugin.warnIfMemoryPoolFractionSettakes the value to check and logs the warning at most once per JVM.CometExecRulepasses the session conf value of each query, soSET,spark.conf.setand extension-only sessions are covered too. Because of the once-per-JVM flag, a session that sets the fraction after it has already been warned about gets no second warning.fair_unifiedlimits each task to the fraction timesspark.memory.offHeap.size.The migration guide is unchanged: the 1.1.0 note that the driver logs a warning still holds, and this change adds no behavior change.
Item 3 is not addressed here. The fraction is set to force spills in more places than
CometTaskMetricsSuite:CometAggregateSuite,CometExecSuite,CometNativeSuite,CometParquetWriterSuite,CometIcebergWriteActionSuiteandCometExecIteratorLifecycleSuiteuse it too. Whether to move the config to the testing category or replace it in tests is still open in the issue.How are these changes tested?
CometPluginsMemoryPoolFractionWarningSuitegains a test that sets the fraction withwithSQLConf, runs two queries and asserts the warning is logged exactly once. It fails without this change because only the SparkConf was checked. The existing tests now reset the once-per-JVM flag first.make format(Mavencompile test-compile, scalafix and spotless): passed locally.npx prettier --check docs/source/user-guide/latest/tuning/memory.md: passed.