You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/12/16 20:24:16 UTC
[GitHub] [spark] techaddict opened a new pull request, #39096: [SPARK-41421][UI] Protobuf serializer for ApplicationEnvironmentInfoWrapper
techaddict opened a new pull request, #39096:
URL: https://github.com/apache/spark/pull/39096
### What changes were proposed in this pull request?
Add Protobuf serializer for ApplicationEnvironmentInfoWrapper
### Why are the changes needed?
Support fast and compact serialization/deserialization for ApplicationEnvironmentInfoWrapper 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 #39096: [SPARK-41421][UI] Protobuf serializer for ApplicationEnvironmentInfoWrapper
Posted by GitBox <gi...@apache.org>.
gengliangwang commented on PR #39096:
URL: https://github.com/apache/spark/pull/39096#issuecomment-1355701397
@techaddict LGTM except for two minor comments. Thanks for working on it.
--
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 #39096: [SPARK-41421][UI] Protobuf serializer for ApplicationEnvironmentInfoWrapper
Posted by GitBox <gi...@apache.org>.
gengliangwang commented on PR #39096:
URL: https://github.com/apache/spark/pull/39096#issuecomment-1355950671
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
[GitHub] [spark] gengliangwang commented on a diff in pull request #39096: [SPARK-41421][UI] Protobuf serializer for ApplicationEnvironmentInfoWrapper
Posted by GitBox <gi...@apache.org>.
gengliangwang commented on code in PR #39096:
URL: https://github.com/apache/spark/pull/39096#discussion_r1051188448
##########
core/src/main/protobuf/org/apache/spark/status/protobuf/store_types.proto:
##########
@@ -106,3 +106,45 @@ message TaskDataWrapper {
int64 stage_id = 40;
int32 stage_attempt_id = 41;
}
+
+message ExecutorResourceRequest {
+ string resource_name = 1;
+ int64 amount = 2;
+ string discoveryScript = 3;
+ string vendor = 4;
+}
+
+message TaskResourceRequest {
+ string resource_name = 1;
+ double amount = 2;
+}
+
+message ResourceProfileInfo {
+ int32 id = 1;
+ map<string, ExecutorResourceRequest> executor_resources = 2;
+ map<string, TaskResourceRequest> task_resources = 3;
+}
+
+message RuntimeInfo {
+ string java_version = 1;
+ string java_home = 2;
+ string scala_version = 3;
+}
+
+message ApplicationEnvironmentInfo {
+ message PairSS {
Review Comment:
Shall we name it `PairStrings` and move it out of `ApplicationEnvironmentInfo`? In the future this might be reused.
--
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 a diff in pull request #39096: [SPARK-41421][UI] Protobuf serializer for ApplicationEnvironmentInfoWrapper
Posted by GitBox <gi...@apache.org>.
gengliangwang commented on code in PR #39096:
URL: https://github.com/apache/spark/pull/39096#discussion_r1051192373
##########
core/src/test/scala/org/apache/spark/status/protobuf/KVStoreProtobufSerializerSuite.scala:
##########
@@ -176,4 +177,83 @@ class KVStoreProtobufSerializerSuite extends SparkFunSuite {
assert(result.stageId == input.stageId)
assert(result.stageAttemptId == input.stageAttemptId)
}
+
+ test("Application Environment Info") {
+ val input = new ApplicationEnvironmentInfoWrapper(
+ new ApplicationEnvironmentInfo(
+ runtime = new RuntimeInfo(
+ javaVersion = "1.8",
+ javaHome = "/tmp/java",
+ scalaVersion = "2.13"),
+ sparkProperties = Seq(("1", "2")),
+ hadoopProperties = Seq(("1", "2")),
Review Comment:
I would suggest setting different values for these fields in case of mistakes on the fields of serializer and deserializer
--
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 pull request #39096: [SPARK-41421][UI] Protobuf serializer for ApplicationEnvironmentInfoWrapper
Posted by GitBox <gi...@apache.org>.
techaddict commented on PR #39096:
URL: https://github.com/apache/spark/pull/39096#issuecomment-1355722437
@gengliangwang Thanks for the reivew, addressed comments.
--
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 closed pull request #39096: [SPARK-41421][UI] Protobuf serializer for ApplicationEnvironmentInfoWrapper
Posted by GitBox <gi...@apache.org>.
gengliangwang closed pull request #39096: [SPARK-41421][UI] Protobuf serializer for ApplicationEnvironmentInfoWrapper
URL: https://github.com/apache/spark/pull/39096
--
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