You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "HeartSaVioR (via GitHub)" <gi...@apache.org> on 2025/04/02 09:11:44 UTC
[PR] [SPARK-51690][SS] Change the protocol of ListState.put()/get()/appendList() from Arrow to simple custom protocol [spark]
HeartSaVioR opened a new pull request, #50488:
URL: https://github.com/apache/spark/pull/50488
### What changes were proposed in this pull request?
This PR proposes to get rid of usage for Arrow on sending multiple elements of ListState and replace it with simple custom protocol.
The custom protocol we are proposing is super simple and widely used.
1. Write the size of the element (in bytes), if there is no more element, write -1
2. Write the element (as bytes)
3. Go back to 1
Note that this PR only makes change to ListState - we are aware that there are more usages of Arrow in other state types or other functionality (timer). We want to improve over time via benchmarking and addressing if it shows the latency implication.
### Why are the changes needed?
For small number of elements, Arrow does not perform very well compared to the custom protocol. In the benchmark, we have three elements to exchange between Python worker and JVM, and replacing Arrow with custom protocol could cut the elapsed time on state interaction by 1/3.
Given the natural performance diff between Scala version of transformWithState and PySpark version of transformWithStateInPandas, I think users must use the Scala version to handle noticeable volume of workloads. We can position PySpark version to aim for more lightweight workloads - we can revisit if we see the opposite demands.
### Does this PR introduce _any_ user-facing change?
No, it's an internal change.
### How was this patch tested?
Existing UT, with modification about mock expectation.
### Was this patch authored or co-authored using generative AI tooling?
No.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51690][SS] Change the protocol of ListState.put()/get()/appendList() from Arrow to simple custom protocol [spark]
Posted by "HeartSaVioR (via GitHub)" <gi...@apache.org>.
HeartSaVioR commented on PR #50488:
URL: https://github.com/apache/spark/pull/50488#issuecomment-2782099922
@HyukjinKwon Friendly reminder.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51690][SS] Change the protocol of ListState.put()/get()/appendList() from Arrow to simple custom protocol [spark]
Posted by "HeartSaVioR (via GitHub)" <gi...@apache.org>.
HeartSaVioR commented on PR #50488:
URL: https://github.com/apache/spark/pull/50488#issuecomment-2782330672
Thanks! Merging to master.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51690][SS] Change the protocol of ListState.put()/get()/appendList() from Arrow to simple custom protocol [spark]
Posted by "HeartSaVioR (via GitHub)" <gi...@apache.org>.
HeartSaVioR commented on PR #50488:
URL: https://github.com/apache/spark/pull/50488#issuecomment-2771933661
cc. @anishshri-db @bogao007 for review, thanks!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51690][SS] Change the protocol of ListState.put()/get()/appendList() from Arrow to simple custom protocol [spark]
Posted by "HeartSaVioR (via GitHub)" <gi...@apache.org>.
HeartSaVioR commented on PR #50488:
URL: https://github.com/apache/spark/pull/50488#issuecomment-2777320014
@HyukjinKwon Could you please take a look? Thanks!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51690][SS] Change the protocol of ListState.put()/get()/appendList() from Arrow to simple custom protocol [spark]
Posted by "HeartSaVioR (via GitHub)" <gi...@apache.org>.
HeartSaVioR commented on code in PR #50488:
URL: https://github.com/apache/spark/pull/50488#discussion_r2027901718
##########
python/pyspark/sql/streaming/list_state_client.py:
##########
@@ -41,6 +41,7 @@ def __init__(
# A dictionary to store the mapping between list state name and a tuple of pandas DataFrame
# and the index of the last row that was read.
self.pandas_df_dict: Dict[str, Tuple["PandasDataFrameLike", int]] = {}
Review Comment:
Ah I missed that this is "list" state client. My bad.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51690][SS] Change the protocol of ListState.put()/get()/appendList() from Arrow to simple custom protocol [spark]
Posted by "HeartSaVioR (via GitHub)" <gi...@apache.org>.
HeartSaVioR commented on code in PR #50488:
URL: https://github.com/apache/spark/pull/50488#discussion_r2027834178
##########
python/pyspark/sql/streaming/list_state_client.py:
##########
@@ -143,7 +131,7 @@ def append_list(self, state_name: str, values: List[Tuple]) -> None:
self._stateful_processor_api_client._send_proto_message(message.SerializeToString())
- self._stateful_processor_api_client._send_arrow_state(self.schema, values)
+ self._stateful_processor_api_client._send_list_state(self.schema, values)
Review Comment:
Maybe I can double confirm with benchmarking and address the other parts. This part I did the benchmark and we have to be backed by the number.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51690][SS] Change the protocol of ListState.put()/get()/appendList() from Arrow to simple custom protocol [spark]
Posted by "anishshri-db (via GitHub)" <gi...@apache.org>.
anishshri-db commented on code in PR #50488:
URL: https://github.com/apache/spark/pull/50488#discussion_r2027832640
##########
python/pyspark/sql/streaming/list_state_client.py:
##########
@@ -143,7 +131,7 @@ def append_list(self, state_name: str, values: List[Tuple]) -> None:
self._stateful_processor_api_client._send_proto_message(message.SerializeToString())
- self._stateful_processor_api_client._send_arrow_state(self.schema, values)
+ self._stateful_processor_api_client._send_list_state(self.schema, values)
Review Comment:
nit: should we add a TODO for other places we might change this in the future ?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51690][SS] Change the protocol of ListState.put()/get()/appendList() from Arrow to simple custom protocol [spark]
Posted by "HeartSaVioR (via GitHub)" <gi...@apache.org>.
HeartSaVioR closed pull request #50488: [SPARK-51690][SS] Change the protocol of ListState.put()/get()/appendList() from Arrow to simple custom protocol
URL: https://github.com/apache/spark/pull/50488
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51690][SS] Change the protocol of ListState.put()/get()/appendList() from Arrow to simple custom protocol [spark]
Posted by "bogao007 (via GitHub)" <gi...@apache.org>.
bogao007 commented on code in PR #50488:
URL: https://github.com/apache/spark/pull/50488#discussion_r2027867499
##########
python/pyspark/sql/streaming/list_state_client.py:
##########
@@ -41,6 +41,7 @@ def __init__(
# A dictionary to store the mapping between list state name and a tuple of pandas DataFrame
# and the index of the last row that was read.
self.pandas_df_dict: Dict[str, Tuple["PandasDataFrameLike", int]] = {}
Review Comment:
Is `pandas_df_dict` still being used anywhere?
##########
python/pyspark/sql/streaming/list_state_client.py:
##########
@@ -143,7 +131,7 @@ def append_list(self, state_name: str, values: List[Tuple]) -> None:
self._stateful_processor_api_client._send_proto_message(message.SerializeToString())
- self._stateful_processor_api_client._send_arrow_state(self.schema, values)
+ self._stateful_processor_api_client._send_list_state(self.schema, values)
Review Comment:
+1, we use the same framework for MapState as well
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org