[SPARK-59332][CORE] Scope standalone shuffle service local dirs - #58619
[SPARK-59332][CORE] Scope standalone shuffle service local dirs#58619holdenk wants to merge 2 commits into
Conversation
…stering app Nest executor local directories under <root>/<appId>/executor-* on the Worker, and add an opt-in config spark.shuffle.service.requireAppScopedLocalDirs (default false) that makes the external shuffle service reject executor registrations whose local directories do not lie inside the registering application's own per-application directory. The check is opt-in for rolling-upgrade compatibility: enable it only after every Worker is upgraded. The underlying LocalDirValidator already supported this mode (used by the YARN shuffle service); this wires standalone to use it. Co-authored-by: Cursor <cursoragent@cursor.com>
pan3793
changed the title
HyukjinKwon
left a comment
HyukjinKwon
left a comment
There was a problem hiding this comment.
0 blocking, 0 non-blocking, 1 nit.
A clean, well-tested change; one comment-wording nit that would otherwise contradict the config doc.
Nits: 1 minor item (see inline comments).
Verification
Traced the Worker layout against LocalDirValidator: the Worker writes <root>/<appId>/executor-*, and the validator (when scoped) requires the appId to appear as an exact path segment of the canonicalized path under a configured root — the produced layout satisfies it, and pre-upgrade <root>/executor-* paths correctly fail the segment check. The cleanup parent-removal is guarded by appIdDir.getName == id plus an empty-directory check, so the shared root of the pre-upgrade layout is never deleted; WorkerSuite exercises all three cases.
| // application's own per-app directory (the Worker creates executor local dirs under a | ||
| // path containing the app id). The check is opt-in: executors launched by Workers that | ||
| // predate the per-app layout register unscoped paths and would be rejected, so enable it | ||
| // only once every Spark application is upgraded. |
There was a problem hiding this comment.
The rollout unit is the Worker, not the application: the config doc and docs/spark-standalone.md both say to upgrade every Worker first, and the sentence just above already attributes the per-app layout to Workers.
| // only once every Spark application is upgraded. | |
| // only once every Worker is upgraded. |
3 participants
What changes were proposed in this pull request?
Nest executor local directories under //executor-* on the Worker, and add an opt-in config spark.shuffle.service.requireAppScopedLocalDirs (default false) that makes the external shuffle service reject executor registrations whose local directories do not lie inside the registering application's own per-application directory. The check is opt-in for rolling-upgrade compatibility: enable it only after every Worker is upgraded. The underlying LocalDirValidator already supported this mode (used by the YARN shuffle service); this wires standalone to use it.
Why are the changes needed?
Possible race condition.
Does this PR introduce any user-facing change?
There is a new, administrator configurable, config flag.
How was this patch tested?
New test
Was this patch authored or co-authored using generative AI tooling?
Yes, claude and cursor