[SPARK-47035][SS][CONNECT] Protocol for Client-Side Listener #45091

WweiL · 2024-02-13T20:09:13Z

What changes were proposed in this pull request?

Currently, the StreamingQueryListener for Connect runs on the server side.

From the user point of view, the purpose of a listener is mainly to have a pushing mechanism that notifies them when queries start / end / make progress. Before, the server-side listener essentially loses this functionality.

The new listener will be running on the client side, with the server continuously pushing the new listener events to the client, and the client will call corresponding callback functions for different listener event types.

This is the first PR that defines the protocol of this new listener.

Why are the changes needed?

Add client side listener which makes more sense.

Does this PR introduce any user-facing change?

Not for this one.

How was this patch tested?

No need for this one, new tests will be added later.

Was this patch authored or co-authored using generative AI tooling?

No

WweiL · 2024-02-13T20:11:16Z

@grundprinzip

connector/connect/common/src/main/protobuf/spark/connect/commands.proto

grundprinzip · 2024-02-22T09:39:23Z

connector/connect/common/src/main/protobuf/spark/connect/commands.proto

+// The event_type for QueryProgressEvent is 1;
+// for QueryTerminatedEvent is 2; for QueryIdleEvent is 3;
+// There is no QueryStartedEvent defined here,


Should we formalize this somehow?

Yes we can use similar mechanism as PythonEvalType
https://github.com/databricks/runtime/blob/95f1afae584a89d20ecf2e411f8b3a267d354e46/python/pyspark/rdd.py#L141

that would be a bit out of the scope of this protocol pr?

oh I see what you meant, created a enum in proto file

grundprinzip · 2024-02-22T09:39:37Z

connector/connect/common/src/main/protobuf/spark/connect/commands.proto

+// There is no QueryStartedEvent defined here,
+// it is added as a field in WriteStreamOperationStartResult
+message StreamingQueryListenerEvents {
+  string event_json = 1;


Is this guaranteed to be JSON?

Yes all streaming query events have a json method

spark/sql/core/src/main/scala/org/apache/spark/sql/streaming/StreamingQueryListener.scala

Line 127 in fd518da

class QueryStartedEvent private[sql](

on client there are fromJson methods to reconstruct the event:

spark/python/pyspark/sql/streaming/listener.py

Line 194 in bbf06c8

def fromJson(cls, j: Dict[str, Any]) -> "QueryStartedEvent":

grundprinzip

some nits

ueshin · 2024-02-23T18:34:02Z

connector/connect/common/src/main/protobuf/spark/connect/commands.proto

+enum StreamingQueryEventType {
+  QUERY_PROGRESS_EVENT = 0;
+  QUERY_TERMINATED_EVENT = 1;
+  QUERY_IDLE_EVENT = 2;
+}


We need something like QUERY_EVENT_UNSPECIFIED as 0?
https://protobuf.dev/programming-guides/proto3/#enum

Also, each variable should have the same prefix as the enum name? I'm not sure if this is a strict rule or not, though. @grundprinzip @amaliujia

+1 for QUERY_EVENT_UNSPECIFIED = 0.

No IIRC we didn't have a strict rule to ask each variable have the same prefix as the enum name.

Thanks for the suggestion! let me update

xinrong-meng · 2024-02-23T19:26:51Z

LGTM once CI pass, thank you!

ueshin · 2024-02-23T21:03:25Z

Thanks! merging to master.

### What changes were proposed in this pull request? Currently, the StreamingQueryListener for Connect runs on the server side. From the user point of view, the purpose of a listener is mainly to have a pushing mechanism that notifies them when queries start / end / make progress. Before, the server-side listener essentially loses this functionality, and we find out that internally there are needs for the client side listener. The new listener will be running on the client side, with the server continuously pushing the new listener events to the client, and the client will call corresponding callback functions for different listener event types. This is the first PR that defines the protocol of this new listener. ### Why are the changes needed? Add client side listener which makes more sense. ### Does this PR introduce _any_ user-facing change? Not for this one. ### How was this patch tested? No need for this one, new tests will be added later. ### Was this patch authored or co-authored using generative AI tooling? No Closes apache#45091 from WweiL/SPARK-47035-protocol. Authored-by: Wei Liu <wei.liu@databricks.com> Signed-off-by: Takuya UESHIN <ueshin@databricks.com>

…ient side listener ### What changes were proposed in this pull request? Followup of previous protocol change #45091. Add the request proto `Command` and response proto message to `ExecutePlanResponse` ### Why are the changes needed? Continuation of client side listener for spark connect ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Will be tested in subsequent PR, the proto change itself doesn't do any harm ### Was this patch authored or co-authored using generative AI tooling? No Closes #45444 from WweiL/SPARK-47035-protocol-followup. Authored-by: Wei Liu <wei.liu@databricks.com> Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>

…ient side listener ### What changes were proposed in this pull request? Followup of previous protocol change apache#45091. Add the request proto `Command` and response proto message to `ExecutePlanResponse` ### Why are the changes needed? Continuation of client side listener for spark connect ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Will be tested in subsequent PR, the proto change itself doesn't do any harm ### Was this patch authored or co-authored using generative AI tooling? No Closes apache#45444 from WweiL/SPARK-47035-protocol-followup. Authored-by: Wei Liu <wei.liu@databricks.com> Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>

Currently, the StreamingQueryListener for Connect runs on the server side. From the user point of view, the purpose of a listener is mainly to have a pushing mechanism that notifies them when queries start / end / make progress. Before, the server-side listener essentially loses this functionality, and we find out that internally there are needs for the client side listener. The new listener will be running on the client side, with the server continuously pushing the new listener events to the client, and the client will call corresponding callback functions for different listener event types. This is the first PR that defines the protocol of this new listener. Add client side listener which makes more sense. Not for this one. No need for this one, new tests will be added later. No Closes apache#45091 from WweiL/SPARK-47035-protocol. Authored-by: Wei Liu <wei.liu@databricks.com> Signed-off-by: Takuya UESHIN <ueshin@databricks.com>

…ient side listener Followup of previous protocol change apache#45091. Add the request proto `Command` and response proto message to `ExecutePlanResponse` Continuation of client side listener for spark connect No Will be tested in subsequent PR, the proto change itself doesn't do any harm No Closes apache#45444 from WweiL/SPARK-47035-protocol-followup. Authored-by: Wei Liu <wei.liu@databricks.com> Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>

done

4db7af2

github-actions bot added SQL PYTHON CONNECT labels Feb 13, 2024

WweiL added 6 commits February 13, 2024 12:12

typo

32a3514

typo

70f36d5

revision

7d3efb0

minor

7124ada

minor

5b3e2f7

add oneof to StreamingQueryListenerBusCommand

4c73601

github-actions bot added the STRUCTURED STREAMING label Feb 21, 2024

only protocol change

bbf06c8

github-actions bot removed the STRUCTURED STREAMING label Feb 21, 2024

grundprinzip reviewed Feb 22, 2024

View reviewed changes

connector/connect/common/src/main/protobuf/spark/connect/commands.proto Outdated Show resolved Hide resolved

grundprinzip reviewed Feb 22, 2024

View reviewed changes

grundprinzip suggested changes Feb 22, 2024

View reviewed changes

WweiL added 5 commits February 22, 2024 10:05

typo

817c37d

regen

f8a034e

use evnum for query event type

96883e2

comment addition

3a9c4b4

retrigger

cd115c3

WweiL requested a review from grundprinzip February 23, 2024 09:03

grundprinzip approved these changes Feb 23, 2024

View reviewed changes

ueshin reviewed Feb 23, 2024

View reviewed changes

add unspecified event type

77a95dd

xinrong-meng approved these changes Feb 23, 2024

View reviewed changes

ueshin approved these changes Feb 23, 2024

View reviewed changes

ueshin closed this in 772a445 Feb 23, 2024

WweiL mentioned this pull request Mar 8, 2024

[SPARK-47035][SS][CONNECT][FOLLOWUP] Additional protobuf change to client side listener #45444

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-47035][SS][CONNECT] Protocol for Client-Side Listener #45091

[SPARK-47035][SS][CONNECT] Protocol for Client-Side Listener #45091

WweiL commented Feb 13, 2024 •

edited

WweiL commented Feb 13, 2024

grundprinzip Feb 22, 2024

WweiL Feb 22, 2024

WweiL Feb 22, 2024

WweiL Feb 22, 2024 •

edited

grundprinzip Feb 22, 2024

WweiL Feb 22, 2024

grundprinzip left a comment

ueshin Feb 23, 2024

ueshin Feb 23, 2024 •

edited

amaliujia Feb 23, 2024

WweiL Feb 23, 2024

xinrong-meng commented Feb 23, 2024

ueshin commented Feb 23, 2024

[SPARK-47035][SS][CONNECT] Protocol for Client-Side Listener #45091

[SPARK-47035][SS][CONNECT] Protocol for Client-Side Listener #45091

Conversation

WweiL commented Feb 13, 2024 • edited

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Was this patch authored or co-authored using generative AI tooling?

WweiL commented Feb 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WweiL Feb 22, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grundprinzip left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ueshin Feb 23, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xinrong-meng commented Feb 23, 2024

ueshin commented Feb 23, 2024

WweiL commented Feb 13, 2024 •

edited

WweiL Feb 22, 2024 •

edited

ueshin Feb 23, 2024 •

edited