Skip to content

Follow-ups to #6163: correct the fair_unified description and prepare spark.comet.exec.memoryPool.fraction for removal #6187

Description

@andygrove

#6163 deprecated spark.comet.exec.memoryPool.fraction. This issue collects the follow-up work.

1. The new text describes fair_unified as a per-consumer limit

The config doc (CometConf.scala#L950-L952) and the memory management guide (memory_management.md#L206-L207) say that fair_unified limits each memory consumer in a task to fraction * spark.memory.offHeap.size / num_consumers. In fact the pool caps the task's combined reservations at that value. CometFairMemoryPool::try_grow compares the pool-wide state.used against pool_size / num (fair_pool.rs#L161), which is #5961. The memory management guide says as much a few sections later ("It is not a per-consumer quota", memory_management.md#L268-L282), so the page now contradicts itself.

Both guides also say Spark's per-task limit is the tighter one "whenever more than one task is running" (memory_management.md#L207-L209, tuning.md#L114-L117), which also rests on the per-consumer reading. With the real cap of fraction * offHeap / num_consumers, Spark's offHeap / N is tighter only when N * fraction > num_consumers. The reason for the deprecation still holds: the fraction applies per task, so the tasks together can still take the whole pool.

Suggested wording: "caps the combined reservations of the task's consumers at memory_limit / num_consumers (see #5961)". If #5961 is fixed first and restores per-consumer limits, the current wording becomes correct instead.

The tuning guide's older pool type description has the same problem, predating #6163: it says fair_unified prevents operators from each using more than pool_size / num_reservations (tuning.md#L135-L136).

2. The deprecation warning misses session-level settings

warnIfMemoryPoolFractionSet checks the SparkConf in the driver plugin (Plugins.scala#L186), but the value is read per query from the session conf (CometExecIterator.scala#L618). A value set with spark.conf.set or SET gets no warning, and neither does an application that registers the session extension without the plugin (#6186).

3. Tests rely on the fraction to force spills

CometTaskMetricsSuite sets the fraction to 0.002 to force native spills (L145, L232, L579). Removing the config needs another way to force spills in tests. Alternatively, move it to the testing category instead of removing it.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

area:memoryMemory pools, reservations, OOM handlingdocumentationImprovements or additions to documentationenhancementNew feature or request

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions