[SPARK-50117][BUILD][SS] Change to using `maven/sbt` plugin to generate `StateMessage.java` #48654

LuciferYang · 2024-10-25T08:56:22Z

What changes were proposed in this pull request?

This pr changes the process to dynamically generate StateMessage.java during build using the corresponding maven/sbt plugin.

Why are the changes needed?

Maintain the same generation way as the Java files corresponding to other pb files in Spark.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Pass GitHub Actions
All maven test passed: https://github.com/LuciferYang/spark/runs/32055792763

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

No

LuciferYang · 2024-10-25T09:14:10Z

.github/workflows/build_and_test.yml

          npm install --save-dev
          node --experimental-vm-modules node_modules/.bin/jest
+
+  maven-test:


will revert after test

All passed: https://github.com/LuciferYang/spark/actions/runs/11515334076/job/32059403290

This reverts commit 20d0be9.

LuciferYang · 2024-10-25T13:11:54Z

cc @dongjoon-hyun FYI

dongjoon-hyun · 2024-10-25T14:12:20Z

Could you resolve the conflicts, @LuciferYang ?

dongjoon-hyun

cc @bogao007 , @jingz-db , @HeartSaVioR from SPARK-49821

#48290

LuciferYang · 2024-10-25T14:54:10Z

Could you resolve the conflicts, @LuciferYang ?

done

dongjoon-hyun

+1, LGTM from my side. Thank you, @LuciferYang .

HeartSaVioR · 2024-10-26T02:08:40Z

Thanks for working on this, @LuciferYang !
I don't have a full context of how PySpark folks do the generated code thing in Spark Connect, but we also have py file being generated from proto. Will we need to do the same for generated py file? Or would there be technical reason not to do this? cc. @HyukjinKwon

LuciferYang · 2024-10-26T05:53:08Z

Thank you for the question from @HeartSaVioR . This was also an issue I intended to discuss in SPARK-49821, and I apologize for forgetting to leave comments yesterday.

Regarding the spark-connect module, there is a script called dev/connect-gen-protos.sh. This script is used by developers to manually regenerate the corresponding _pb2.py and _pb2.pyi files and submit them when changes are made to the .proto files in the connect module. Personally, I think we should provide a similar way to automate this process in the future, but as my understanding of Python is very superficial, I think we need @HyukjinKwon to confirm if there is a more reasonable solution.

LuciferYang · 2024-10-27T14:56:39Z

@HeartSaVioR @dongjoon-hyun Can we merge this one first? I have created a new Jira ticket: SPARK-50139, to remind us that we should provide a tool to assist in regenerating StateMessage_pb2.py and StateMessage_pb2.pyi. We can complete this TODO after determining the most reasonable approach for PySpark.

dongjoon-hyun · 2024-10-28T05:20:25Z

To @LuciferYang , I agree with you.

I believe we can handle that seperately in SPARK-50139 if needed. Or, we can do a follow-up PR if something is missing in this PR's context.

dongjoon-hyun · 2024-10-28T05:22:15Z

If there is no further comments until Tomorrow (72 hours after #48654 (comment)), I believe you can merge this.

LuciferYang · 2024-10-28T05:26:38Z

If there is no further comments until Tomorrow (72 hours after #48654 (comment)), I believe you can merge this.

Got it ~

HeartSaVioR · 2024-10-28T22:37:20Z

Would we want to also add StateMessage.java to gitignore so that we don't try to add the file again?

LuciferYang · 2024-10-29T00:47:35Z

Would we want to also add StateMessage.java to gitignore so that we don't try to add the file again?

double checked:

Maven

build/mvn clean install -DskipTests -pl sql/core -am


[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary for Spark Project Parent POM 4.0.0-SNAPSHOT:
[INFO] 
[INFO] Spark Project Parent POM ........................... SUCCESS [  3.451 s]
[INFO] Spark Project Tags ................................. SUCCESS [  5.497 s]
[INFO] Spark Project Sketch ............................... SUCCESS [  5.819 s]
[INFO] Spark Project Common Utils ......................... SUCCESS [ 26.619 s]
[INFO] Spark Project Local DB ............................. SUCCESS [ 13.609 s]
[INFO] Spark Project Networking ........................... SUCCESS [ 16.461 s]
[INFO] Spark Project Shuffle Streaming Service ............ SUCCESS [ 15.016 s]
[INFO] Spark Project Variant .............................. SUCCESS [  4.177 s]
[INFO] Spark Project Unsafe ............................... SUCCESS [ 14.669 s]
[INFO] Spark Project Connect Shims ........................ SUCCESS [  7.762 s]
[INFO] Spark Project Launcher ............................. SUCCESS [  7.345 s]
[INFO] Spark Project Core ................................. SUCCESS [02:26 min]
[INFO] Spark Project SQL API .............................. SUCCESS [ 40.995 s]
[INFO] Spark Project Catalyst ............................. SUCCESS [03:04 min]
[INFO] Spark Project SQL .................................. SUCCESS [03:19 min]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  11:33 min
[INFO] Finished at: 2024-10-29T08:28:02+08:00
[INFO] ------------------------------------------------------------------------


git status

On branch state_message-pb
Your branch is up to date with 'origin/state_message-pb'.

nothing to commit, working tree clean


find sql/core -name "*StateMessage.java"
sql/core/target/generated-sources/org/apache/spark/sql/execution/streaming/state/StateMessage.java

SBT

build/sbt clean "sql/compile"
[success] Total time: 109 s (01:49), completed 2024年10月29日 上午8:34:33

git status
On branch state_message-pb
Your branch is up to date with 'origin/state_message-pb'.

nothing to commit, working tree clean

find sql/core -name "*StateMessage.java"
sql/core/target/scala-2.13/src_managed/main/org/apache/spark/sql/execution/streaming/state/StateMessage.java

The StateMessage.java files generated by the maven/sbt plugin are all located in the sql/core/target directory. They are all visible on the classpath and have already been made invisible to git. I believe that under normal circumstances, they will not be accidentally added by git add, so there is no need to modify the .gitignore file any further.

HeartSaVioR · 2024-10-29T02:11:03Z

Ah OK, so the generated file is placed in target, not source directory. Sounds good.

HeartSaVioR · 2024-10-29T02:18:37Z

Please allow me to wait for a day to sync with folks working on this (TWS in PySpark). I've asked them to comment and they will come in US tz. If there is no comment in tomorrow US tz, I'm OK with the fix.

LuciferYang · 2024-10-29T02:42:27Z

Please allow me to wait for a day to sync with folks working on this (TWS in PySpark). I've asked them to comment and they will come in US tz. If there is no comment in tomorrow US tz, I'm OK with the fix.

OK ~

dongjoon-hyun · 2024-10-29T03:53:10Z

Thank you for the sync-up with them, @HeartSaVioR .

bogao007 · 2024-10-29T16:32:00Z

@LuciferYang Thanks for working on this change! QQ: for the dev loop after this is merged, if we add changes to StateMessage.proto, would command build/sbt clean package be enough to automatically generate the java file and get picked up by the compiler?

dongjoon-hyun · 2024-10-29T16:46:20Z

To @bogao007 , yes, exactly.

FYI, Apache Spark has many proto files (including this) and use them (except this) in that way.

$ find . -type f -name "*.proto" | grep -v test
./core/src/main/protobuf/org/apache/spark/status/protobuf/store_types.proto
./sql/core/src/main/java/org/apache/spark/sql/execution/streaming/StateMessage.proto
./sql/connect/common/src/main/protobuf/spark/connect/relations.proto
./sql/connect/common/src/main/protobuf/spark/connect/catalog.proto
./sql/connect/common/src/main/protobuf/spark/connect/base.proto
./sql/connect/common/src/main/protobuf/spark/connect/example_plugins.proto
./sql/connect/common/src/main/protobuf/spark/connect/types.proto
./sql/connect/common/src/main/protobuf/spark/connect/expressions.proto
./sql/connect/common/src/main/protobuf/spark/connect/commands.proto
./sql/connect/common/src/main/protobuf/spark/connect/common.proto

LuciferYang · 2024-10-29T17:03:25Z

Thanks for working on this change! QQ: for the dev loop after this is merged, if we add changes to StateMessage.proto, would command build/sbt clean package be enough to automatically generate the java file and get picked up by the compiler?

@bogao007 Yes, it would be enough.

Thanks for your help! @dongjoon-hyun

HeartSaVioR

+1

LuciferYang · 2024-10-30T02:11:28Z

Merged into master for Spark 4.0. Thanks @dongjoon-hyun @HeartSaVioR and @bogao007

dongjoon-hyun · 2024-10-30T02:38:53Z

Thank you all again!

LuciferYang added 3 commits October 25, 2024 16:24

init

9d87a4c

add proto file

013848a

rename

7021cc8

LuciferYang marked this pull request as draft October 25, 2024 08:56

github-actions bot added SQL STRUCTURED STREAMING BUILD labels Oct 25, 2024

LuciferYang changed the title ~~Change to using a plugin to generate StateMessage.java~~ Change to using maven/sbt plugin to generate StateMessage.java Oct 25, 2024

LuciferYang changed the title ~~Change to using maven/sbt plugin to generate StateMessage.java~~ Change to using maven/sbt plugin to generate StateMessage.java Oct 25, 2024

test maven

20d0be9

github-actions bot added the INFRA label Oct 25, 2024

LuciferYang changed the title ~~Change to using maven/sbt plugin to generate StateMessage.java~~ [BUILD][SS] Change to using maven/sbt plugin to generate StateMessage.java Oct 25, 2024

LuciferYang commented Oct 25, 2024

View reviewed changes

LuciferYang changed the title ~~[BUILD][SS] Change to using maven/sbt plugin to generate StateMessage.java~~ [SPARK-50117][BUILD][SS] Change to using maven/sbt plugin to generate StateMessage.java Oct 25, 2024

Revert "test maven"

2ab232f

This reverts commit 20d0be9.

LuciferYang marked this pull request as ready for review October 25, 2024 13:09

github-actions bot removed the INFRA label Oct 25, 2024

LuciferYang mentioned this pull request Oct 25, 2024

[SPARK-50107][SQL][BUILD] Upgrade protobuf-java to 4.28.3 #48646

Closed

dongjoon-hyun reviewed Oct 25, 2024

View reviewed changes

Merge branch 'upmaster' into state_message-pb

3644e54

dongjoon-hyun approved these changes Oct 25, 2024

View reviewed changes

HeartSaVioR approved these changes Oct 29, 2024

View reviewed changes

LuciferYang closed this in 413242b Oct 30, 2024

LuciferYang deleted the state_message-pb branch May 2, 2025 05:25

[SPARK-50117][BUILD][SS] Change to using maven/sbt plugin to generate StateMessage.java #48654

[SPARK-50117][BUILD][SS] Change to using maven/sbt plugin to generate StateMessage.java #48654

Uh oh!

Conversation

LuciferYang commented Oct 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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?

Uh oh!

LuciferYang Oct 25, 2024

Choose a reason for hiding this comment

Uh oh!

LuciferYang Oct 25, 2024

Choose a reason for hiding this comment

Uh oh!

LuciferYang commented Oct 25, 2024

Uh oh!

dongjoon-hyun commented Oct 25, 2024

Uh oh!

dongjoon-hyun left a comment

Choose a reason for hiding this comment

Uh oh!

LuciferYang commented Oct 25, 2024

Uh oh!

dongjoon-hyun left a comment

Choose a reason for hiding this comment

Uh oh!

HeartSaVioR commented Oct 26, 2024

Uh oh!

LuciferYang commented Oct 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LuciferYang commented Oct 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dongjoon-hyun commented Oct 28, 2024

Uh oh!

dongjoon-hyun commented Oct 28, 2024

Uh oh!

LuciferYang commented Oct 28, 2024

Uh oh!

HeartSaVioR commented Oct 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LuciferYang commented Oct 29, 2024

Uh oh!

HeartSaVioR commented Oct 29, 2024

Uh oh!

HeartSaVioR commented Oct 29, 2024

Uh oh!

LuciferYang commented Oct 29, 2024

Uh oh!

dongjoon-hyun commented Oct 29, 2024

Uh oh!

bogao007 commented Oct 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dongjoon-hyun commented Oct 29, 2024

Uh oh!

LuciferYang commented Oct 29, 2024

Uh oh!

HeartSaVioR left a comment

Choose a reason for hiding this comment

Uh oh!

LuciferYang commented Oct 30, 2024

Uh oh!

dongjoon-hyun commented Oct 30, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[SPARK-50117][BUILD][SS] Change to using `maven/sbt` plugin to generate `StateMessage.java` #48654

[SPARK-50117][BUILD][SS] Change to using `maven/sbt` plugin to generate `StateMessage.java` #48654

LuciferYang commented Oct 25, 2024 •

edited

Loading

LuciferYang commented Oct 26, 2024 •

edited

Loading

LuciferYang commented Oct 27, 2024 •

edited

Loading

HeartSaVioR commented Oct 28, 2024 •

edited

Loading

bogao007 commented Oct 29, 2024 •

edited

Loading