[GitHub] [spark] LuciferYang commented on a diff in pull request #39202: [WIP][SPARK-41685][UI] Support Protobuf serializer for the KVStore in History server

[GitBox <gi...@apache.org>]

LuciferYang commented on code in PR #39202:
URL: https://github.com/apache/spark/pull/39202#discussion_r1057124135


##########
core/src/main/scala/org/apache/spark/internal/config/History.scala:
##########
@@ -79,6 +79,21 @@ private[spark] object History {
     .stringConf
     .createOptional
 
+  object LocalStoreSerializer extends Enumeration {
+    val JSON, PROTOBUF = Value
+  }
+
+  val LOCAL_STORE_SERIALIZER = ConfigBuilder("spark.history.store.serializer")
+    .doc("Serializer for writing/reading in-memory UI objects to/from disk-based KV Store; " +

Review Comment:
   `disk-based KV Store;` , `;` or `:`?
   
   



##########
core/src/main/scala/org/apache/spark/status/KVUtils.scala:
##########
@@ -92,6 +92,17 @@ private[spark] object KVUtils extends Logging {
     db
   }
 
+  def serializerForHistoryServer(conf: SparkConf): KVStoreScalaSerializer = {
+    History.LocalStoreSerializer.withName(conf.get(History.LOCAL_STORE_SERIALIZER)) match {
+      case History.LocalStoreSerializer.JSON =>
+        new KVStoreScalaSerializer
+      case History.LocalStoreSerializer.PROTOBUF =>
+        new KVStoreProtobufSerializer()

Review Comment:
   nit: both have `()` or not, it seems uniform
   
   



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