You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "LuciferYang (via GitHub)" <gi...@apache.org> on 2023/02/06 05:40:30 UTC

[GitHub] [spark] LuciferYang opened a new pull request, #39894: [SPARK-42351][CORE] Protobuf serializer for FsHistoryProviderMetadata

LuciferYang opened a new pull request, #39894:
URL: https://github.com/apache/spark/pull/39894

   ### What changes were proposed in this pull request?
   Add Protobuf serializer for `FsHistoryProviderMetadata`
   
   ### Why are the changes needed?
   Support fast and compact serialization/deserialization for `FsHistoryProviderMetadata` over RocksDB.
   
   ### Does this PR introduce any user-facing change?
   No
   
   ### How was this patch tested?
   New UT


-- 
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


[GitHub] [spark] gengliangwang commented on pull request #39894: [SPARK-42351][CORE] Protobuf serializer for FsHistoryProviderMetadata

Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on PR #39894:
URL: https://github.com/apache/spark/pull/39894#issuecomment-1424716990

   @LuciferYang So as per the comment in https://issues.apache.org/jira/browse/SPARK-41053?focusedCommentId=17651170&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17651170, let's support protobuf serliazer for all the live UI data for now. For SHS related, we can simply use json since the objects are not frequently used. The `FsHistoryProviderMetadata` is written once only.


-- 
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


[GitHub] [spark] HyukjinKwon commented on a diff in pull request #39894: [SPARK-42351][CORE] Protobuf serializer for FsHistoryProviderMetadata

Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #39894:
URL: https://github.com/apache/spark/pull/39894#discussion_r1097198468


##########
core/src/test/scala/org/apache/spark/status/protobuf/KVStoreProtobufSerializerSuite.scala:
##########
@@ -1448,6 +1449,21 @@ class KVStoreProtobufSerializerSuite extends SparkFunSuite {
     }
   }
 
+  test("FsHistoryProviderMetadata") {
+    Seq( "file:/tmp/spark-events", null).foreach { logDir =>

Review Comment:
   ```suggestion
       Seq("file:/tmp/spark-events", null).foreach { logDir =>
   ```



-- 
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


[GitHub] [spark] LuciferYang commented on pull request #39894: [SPARK-42351][CORE] Protobuf serializer for FsHistoryProviderMetadata

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #39894:
URL: https://github.com/apache/spark/pull/39894#issuecomment-1425090421

   > @LuciferYang So as per the comment in https://issues.apache.org/jira/browse/SPARK-41053?focusedCommentId=17651170&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17651170, let's support protobuf serliazer for all the live UI data for now. For SHS related, we can simply use json since the objects are not frequently used. The `FsHistoryProviderMetadata` is written once only.
   
   OK, close this one and https://github.com/apache/spark/pull/39944, `RocksDB.TypeAliases` also write only in Live UI scene


-- 
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


[GitHub] [spark] LuciferYang commented on a diff in pull request #39894: [SPARK-42351][CORE] Protobuf serializer for FsHistoryProviderMetadata

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #39894:
URL: https://github.com/apache/spark/pull/39894#discussion_r1098116581


##########
core/src/main/protobuf/org/apache/spark/status/protobuf/store_types.proto:
##########
@@ -816,3 +816,9 @@ message StreamingQueryProgress {
 message StreamingQueryProgressWrapper {
   StreamingQueryProgress progress = 1;
 }
+
+message FsHistoryProviderMetadata {
+  int64 version = 1;
+  int64 ui_version = 2;
+  optional string log_dir = 3;
+}

Review Comment:
   done



-- 
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


[GitHub] [spark] LuciferYang commented on pull request #39894: [SPARK-42351][CORE] Protobuf serializer for FsHistoryProviderMetadata

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #39894:
URL: https://github.com/apache/spark/pull/39894#issuecomment-1420709085

   GA passed


-- 
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


[GitHub] [spark] LuciferYang closed pull request #39894: [SPARK-42351][CORE] Protobuf serializer for FsHistoryProviderMetadata

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang closed pull request #39894: [SPARK-42351][CORE] Protobuf serializer for FsHistoryProviderMetadata
URL: https://github.com/apache/spark/pull/39894


-- 
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


[GitHub] [spark] techaddict commented on a diff in pull request #39894: [SPARK-42351][CORE] Protobuf serializer for FsHistoryProviderMetadata

Posted by "techaddict (via GitHub)" <gi...@apache.org>.
techaddict commented on code in PR #39894:
URL: https://github.com/apache/spark/pull/39894#discussion_r1097331319


##########
core/src/main/protobuf/org/apache/spark/status/protobuf/store_types.proto:
##########
@@ -816,3 +816,9 @@ message StreamingQueryProgress {
 message StreamingQueryProgressWrapper {
   StreamingQueryProgress progress = 1;
 }
+
+message FsHistoryProviderMetadata {
+  int64 version = 1;
+  int64 ui_version = 2;
+  optional string log_dir = 3;
+}

Review Comment:
   :nit add a new line at the end



-- 
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