HwangDongJun commented on PR #7752:
URL: https://github.com/apache/kyuubi/pull/7752#issuecomment-5753484623
@pan3793 @ruanwenjun Thanks both for the careful review — you're right to
push on this, and after digging further I don't think this PR should be
merged as-is. Closing it.
To answer @pan3793's question directly: I re-traced the propagation chain
in vanilla Apache Spark's K8s resource manager
(`KubernetesConf`/`BasicDriverFeatureStep`/`BasicExecutorFeatureStep`) and
I could not find any code path where the driver pod *name* is copied into
a Kubernetes *label* value:
- `spark.kubernetes.driver.pod.name` is only ever used to set the driver
pod's `.metadata.name` (`BasicDriverFeatureStep`).
- The driver/executor `resourceNamePrefix` used for labels and the driver
service name is computed independently from `spark.app.name` via
`KubernetesConf.getResourceNamePrefix`, and never reads the configured
driver pod name.
- `spark.kubernetes.driver.label.*` and `spark.kubernetes.executor.label.*`
are parsed completely independently
(`KubernetesUtils.parsePrefixedKeyValuePairs`)
— there's no auto-propagation from driver labels to executor labels.
- The only custom driver label Kyuubi itself sets is
`spark.kubernetes.driver.label.kyuubi-unique-tag=<engineRefId>`
(`KyuubiApplicationManager`), and `engineRefId` is always a 36-char UUID,
so it can never exceed the 63-char limit — which is exactly the point
@ruanwenjun raised.
- The driver Service name (`DriverServiceFeatureStep`) already has its own
independent 63-char check with its own fallback, unrelated to the driver
pod name.
I went back to the original report (#4942) and noticed the failing label
key in that log is `spark-driver-pod-name`, together with an
EMR-internal label (`eks-subscription.amazonaws.com/emr.internal.id`) on
the same pod, and the reporter's Spark build is `3.3.1-amzn-0`. I can't
find `spark-driver-pod-name` anywhere in Apache Spark's history, so this
looks like an EMR-specific patch that copies the driver pod name into a
label — not something vanilla Apache Spark or Kyuubi does. That means the
underlying bug is real for that reporter's environment, but the fix
doesn't belong in Kyuubi's `KubernetesUtils.generateDriverPodName`, since
Kyuubi (on vanilla Spark) never actually produces a driver-pod-name-valued
label that could trip the 63-char limit.
Apologies for not verifying the actual label propagation path before
opening this — I should have confirmed where the value would actually end
up as a label before assuming the chain described in the issue applied
generally. Closing this PR; happy to revisit if someone can point to a
concrete vanilla-Spark/Kyuubi code path I missed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
