Skip to content

fix(core): stop recommending maxBytesInFlight - #2107

Open
WilliamK112 wants to merge 4 commits into
NVIDIA:devfrom
WilliamK112:codex/remove-max-bytes-in-flight-recommendation
Open

fix(core): stop recommending maxBytesInFlight#2107
WilliamK112 wants to merge 4 commits into
NVIDIA:devfrom
WilliamK112:codex/remove-max-bytes-in-flight-recommendation

Conversation

@WilliamK112

@WilliamK112 WilliamK112 commented Jun 23, 2026

Copy link
Copy Markdown

What changed

Removes the AutoTuner recommendation for spark.rapids.shuffle.multiThreaded.maxBytesInFlight.

Why

Fixes #1710. The issue notes that this recommendation should be removed because it does not use metrics to make the decision, and the issue discussion has maintainer confirmation to remove it.

Details

  • Removed the MAX_BYTES_IN_FLIGHT tuning config and recommendation path from AutoTuner.
  • Simplified memory calculation so it no longer carries a maxBytesInFlight recommendation flag.
  • Updated profiling and qualification expected outputs to remove the recommended Spark property and reflect the reclaimed memory in pinned pool sizing.
  • Kept the tuning table Spark property entry so explicit user configs for that Spark key can still be recognized by the tuning metadata.
  • Synced with the latest dev branch while preserving the new PySpark memory-rebalancing logic.

Validation

  • git diff --check
  • cd core && JAVA_HOME=$(/usr/libexec/java_home -v 17) mvn -DwildcardSuites=com.nvidia.spark.rapids.tool.tuning.ProfilingAutoTunerSuiteV2 test — 64 passed
  • cd core && JAVA_HOME=$(/usr/libexec/java_home -v 17) mvn -DwildcardSuites=com.nvidia.spark.rapids.tool.tuning.QualificationAutoTunerSuite test — 48 passed
  • cd core && JAVA_HOME=$(/usr/libexec/java_home -v 17) mvn -DwildcardSuites=com.nvidia.spark.rapids.tool.tuning.ProfilingAutoTunerSuite test — 152 passed, 7 ignored
  • cd core && JAVA_HOME=$(/usr/libexec/java_home -v 17) mvn -DwildcardSuites=com.nvidia.spark.rapids.tool.tuning.config.PySparkMemoryTuningPolicySuite test — 7 passed
  • cd core && JAVA_HOME=$(/usr/libexec/java_home -v 17) mvn -DskipTests verify — build and Scalastyle passed (314 files, 0 errors, 0 warnings)

Signed-off-by: WilliamK112 <164879897+WilliamK112@users.noreply.github.com>
@github-actions github-actions Bot added the core_tools Scope the core module (scala) label Jun 23, 2026
…es-in-flight-recommendation

Signed-off-by: WilliamK112 <164879897+WilliamK112@users.noreply.github.com>

# Conflicts:
#	core/src/main/scala/com/nvidia/spark/rapids/tool/tuning/AutoTuner.scala
@WilliamK112
WilliamK112 marked this pull request as ready for review August 22, 2026 18:32
@parthosa
parthosa self-requested a review August 25, 2026 15:24
parthosa
parthosa previously approved these changes Aug 25, 2026

@parthosa parthosa left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTME. Could you please resolve the merge conflicts?

@parthosa
parthosa dismissed their stale review August 25, 2026 17:09

Revoking approval pending merge conflict resolution.

…es-in-flight-recommendation

Signed-off-by: WilliamK112 <164879897+WilliamK112@users.noreply.github.com>

# Conflicts:
#	core/src/main/scala/com/nvidia/spark/rapids/tool/tuning/AutoTuner.scala
@WilliamK112

Copy link
Copy Markdown
Author

Resolved the merge conflict in 94995bd by merging the latest dev and preserving this PR's removal of the maxBytesInFlight recommendation. The resulting diff against current dev remains limited to the original five tuning files.

Validation (JDK 17):
mvn clean test -Dsuites=com.nvidia.spark.rapids.tool.tuning.ProfilingAutoTunerSuite,com.nvidia.spark.rapids.tool.tuning.ProfilingAutoTunerSuiteV2,com.nvidia.spark.rapids.tool.tuning.QualificationAutoTunerSuite

Result: 200 tests passed, 0 failed (7 ignored).

Signed-off-by: WilliamK112 <164879897+WilliamK112@users.noreply.github.com>
@WilliamK112

Copy link
Copy Markdown
Author

The conflict reappeared after dev advanced with #2127. I have now merged that upstream commit in e7051e4, retaining its offline plugin-staleness tests and this PR’s intended five-file diff. A clean JDK 17 run completed with 200 passed, 0 failed, and 7 ignored; both hosted checks are also green.

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

Labels

core_tools Scope the core module (scala)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] AutoTuner: Remove recommendation for maxBytesInFlight

3 participants