[SPARK-56413][SQL][UDF] gRPC implementation of the UDF worker protocol dispatcher - #55683
[SPARK-56413][SQL][UDF] gRPC implementation of the UDF worker protocol dispatcher#55683haiyangsun-db wants to merge 3 commits into
Conversation
93ec09e to
eef803e
Compare
eef803e to
8d45dfe
Compare
e45886e to
9ada1b2
Compare
| * - [[cancel]] is thread-safe and idempotent. | ||
| * - [[doClose]] half-closes the request stream after the terminator arrives. | ||
| * | ||
| * NOTE: this class does not yet implement payload chunking; the entire |
There was a problem hiding this comment.
nit: Add a TODO with the JIRA tracking ticket for this
|
Hi @dtenedor @hvanhovell @Yicong-Huang , please feel free to review this changes. It's the Dispatcher/Session implementation of the gRPC protocol following last PR. Spark can then use the Dispatcher and session interface to talk to udf worker, which encapsulates the protocol details or worker management. |
| def detect(): UnixDomainSocketTransport = { | ||
| val os = System.getProperty("os.name", "").toLowerCase(Locale.ROOT) | ||
| if (isLinux(os) && epollAvailable) { | ||
| EpollTransport |
There was a problem hiding this comment.
I think Netty is also adding support iouring.
| throw new UnsupportedOperationException( | ||
| s"No Netty native UDS transport available on os=$os " + | ||
| s"(epoll: $epollErr, kqueue: $kqueueErr). " + | ||
| "UDS-backed gRPC requires netty-transport-native-epoll on Linux or " + |
There was a problem hiding this comment.
Is there a socket based fallback?
There was a problem hiding this comment.
could be, but out-of-scoped this PR
| message: String, | ||
| cause: Throwable = null, | ||
| val executionError: ExecutionError = null) | ||
| extends RuntimeException(message, cause) { |
There was a problem hiding this comment.
Should this be an SparkRuntimeException? I understand that this requires an additional dependency.
There was a problem hiding this comment.
Spark side should wrap exception thrown in this module into SparkRuntimeException, let me update the doc about that.
The intention is to make this module as isolated from spark as possible.
|
|
||
| /** | ||
| * :: Experimental :: | ||
| * Exception thrown by [[GrpcWorkerSession]] when the UDF execution fails |
There was a problem hiding this comment.
Is this a mix of system and user errors? If we can, it would be preferable to separate these.
There was a problem hiding this comment.
We shall have three layers of errors:
- error from udf
- error from worker / protocol (still logic at worker level)
- transport error (grpc, etc)
I was trying to merge 1/2 into one big category as these are programmable errors usually and use subclasses to tell udf-only error from worker/protocol error.
| val DEFAULT_TERMINAL_TIMEOUT_MS: Long = 30000L | ||
|
|
||
| // Items the gRPC response observer pushes onto outputQueue. Using a sealed | ||
| // ADT (rather than an identity-typed `Array[Byte]` sentinel) means an |
There was a problem hiding this comment.
A bit of a pedantic comment. Yes, ADTs are easier to reason about that naked byte arrays.
| val u = err.getUser | ||
| val cls = if (u.hasErrorClass) s"[${u.getErrorClass}] " else "" | ||
| s"$cls${u.getMessage}" | ||
| case ExecutionError.KindCase.WORKER => |
There was a problem hiding this comment.
Can a worker error be caused by the user?
There was a problem hiding this comment.
It's possible, that's why i tend to combine worker error with user error, and distinguish using subclasses => they are definitely different from transport-level errors.
| terminalTimeoutMs: Long = DEFAULT_TERMINAL_TIMEOUT_MS) | ||
| extends WorkerSession(workerHandle, logger) { | ||
|
|
||
| require(channel != null, "channel is required") |
There was a problem hiding this comment.
Not checking the handle?
| // We deliberately do NOT bound this queue: blocking the gRPC receiving | ||
| // thread is worse than the current unbounded-but-fast behaviour. A proto | ||
| // change is out of scope here. | ||
| private val outputQueue = new LinkedBlockingQueue[QueueItem]() |
There was a problem hiding this comment.
We should have metrics for this.
| // Latch fired when `InitResponse` (success or error) or a transport error | ||
| // arrives. init() blocks on this; until it fires we have no proof the | ||
| // worker actually accepted the session. | ||
| private val initLatch = new CountDownLatch(1) |
There was a problem hiding this comment.
A Phaser might be more appropriate. You can combine this with other latch.
|
|
||
| // (2) Send next input batch when input has data and session is live. | ||
| if (!cancelRequested.get() && !finishedSending.get() && input.hasNext) { | ||
| val batch = try input.next() catch { |
There was a problem hiding this comment.
This sends a singular batch per iteration. Shouldn't we try to fill the send queue to a certain level?
There was a problem hiding this comment.
It's a while loop, it will keep sending input until seeing a response.
There was a problem hiding this comment.
we prefer consuming results over sending to avoid results filling up buffer
| // (4) Block for late output or the terminator. See class doc | ||
| // above for the per-poll-vs-total-session timeout semantics. | ||
| val item = try { | ||
| outputQueue.poll(terminalTimeoutMs, TimeUnit.MILLISECONDS) |
There was a problem hiding this comment.
How does this work well with things like mapPartitions? In theory you can have a mapPartitions function that needs to consume all data first (e.g. home grown aggregation). I think that will time out if the data is partitioned into multi batches.
There was a problem hiding this comment.
This is a loop, and this poll with timeout only happens when all input batches have been sent in this class.
If input are not all sent, we use poll without timeout (non-blocking) to check if there is any response.
We know at this moment, receiving responses is the only thing to do.
There was a problem hiding this comment.
when we hit this branch, it means we sent all input and finish messages, and the only thing left is to wait for result response.
d749c06 to
c50e125
Compare
| // runs on the WorkerSession.close() path and an exception here would | ||
| // mask any in-flight failure the caller is already handling. | ||
| val msg = s"releaseSession called without a matching acquireSession (count=$c)" | ||
| assert(c >= 0, msg) |
There was a problem hiding this comment.
IMO this should be a harder error than an assertion error.
|
Split the WorkerSession state-machine change into a separate PR for easy review: https://github.com/apache/spark/pull/56397/changes |
ca42ad4 to
1ffaa80
Compare
There was a problem hiding this comment.
@haiyangsun-db FYI: please coordinate with #56741, because of possible merge order overlap.
|
@uros-b thank you for the reminder, i will update |
What changes were proposed in this pull request?
Adds the first transport implementation of the UDF worker protocol: gRPC bidirectional streaming over a Unix domain socket, as a new
udf-worker-grpcmodule.This PR builds on the engine-side
coreabstractions and the proto split introduced in the base PR#56397 (which is the base of this branch); those are not part of this diff. For context, the base PR provides:udf_message.proto(message types) andudf_service.proto(theUdfWorkergRPC service), keeping gRPC off theclasspath of the message consumers (
core,catalyst,sql-core);WorkerSessionSessionStatestate machine, theTerminationtype,and the
WorkerHandletrait.This PR is scoped to the gRPC implementation on top of those abstractions:
New
udf-worker-grpcmodule:DirectGrpcDispatcher-- aDirectWorkerDispatcherthat wires UDS gRPCinto the existing worker spawn / lifecycle machinery (private 0700 socket
dir, atomic POSIX permissions with an owner-only non-POSIX fallback,
recursive cleanup, UDS path-length-safe socket naming).
validateTransportSupportrequires the spec to declare UDS transport andBIDIRECTIONAL_STREAMING.GrpcWorkerSession-- the engine-sideWorkerSessiondriving onebidirectional
Executestream per UDF execution. Honors the proto orderinginvariants (Init -> DataRequest* -> Finish, Cancel-not-before-Finish on the
engine side; InitResponse -> DataResponse* -> terminator on the worker
side), serializes wire writes under a single
requestLock, surfacestransport / execution / protocol failures as
GrpcWorkerSessionException,and bounds every wait by an explicit timeout. It keeps no parallel state
machine -- it drives the single
SessionStateowned by the baseWorkerSession, advancing only the protocol-event edges.GrpcWorkerChannel-- per-workerManagedChannelplus a dedicated NettyEventLoopGroupover the UDS, with deterministic close ordering.UnixDomainSocketTransport-- runtime detection of Linuxepoll/macOS
kqueuenative UDS transports, with a loud failure when neither isavailable.
Integration:
sql/coregains a test-scope dependency on the grpc test-jar soPythonUDFWorkerSpecificationSuitecan reuse the grpc test dispatcher; gRPCstays off
sql/core's main classpath. The module is@Experimentaland hasno production callers yet.
Why are the changes needed?
The UDF worker protocol defines a language-agnostic execution surface but did
not yet have an implementation. This PR delivers gRPC-over-UDS as the first
transport so the dispatcher can run UDFs end to end. Native UDS via Netty
avoids TCP overhead on the local socket; gRPC bidirectional streaming gives us
ordered delivery, per-message size limits, and well-tested flow control.
Does this PR introduce any user-facing change?
No.
udf/workeris@Experimentaland not yet wired into any Spark executionpath.
How was this patch tested?
GrpcWorkerSessionConcurrencySuite(in-process tests) pins thewire-ordering and fast-fail invariants: a worker-emitted ERROR / FINISH /
CANCEL /
onCompleted()beforeInitResponseall failinit()fast;repeated
hasNextafter exhaustion returns immediately;close()concurrentwith
process()terminates cleanly; and the same fast-fail / data-phasepaths are re-run under cross-thread (non-
directExecutor) delivery socorrectness does not silently depend on reentrant delivery.
DirectGrpcDispatcherIntegrationSuite(subprocess tests viaEchoGrpcWorkerMain): end-to-end echo, sequential and concurrent sessions,worker-crash mid-process, init failure, capability-mismatch rejection
(including a spec with no declared capabilities), socket-directory cleanup,
reuse-readiness.
EchoProtocolSuiterefactored onto a sharedEchoWorkerServiceandcontinues to cover the wire protocol end to end.
DirectWorkerDispatcherSuiteported to the gRPC test dispatcher: spawn /SIGTERM / SIGKILL escalation / environment lifecycle / concurrent close /
timeout clamping.
sql/Test/compileclean.Was this patch authored or co-authored using generative AI tooling?
Yes