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 2021/06/15 06:37:38 UTC
[GitHub] [spark] vkorukanti opened a new pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
vkorukanti opened a new pull request #32914:
URL: https://github.com/apache/spark/pull/32914
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes.
6. If possible, provide a concise example to reproduce the issue for a faster review.
7. If you want to add a new configuration, please read the guideline first for naming configurations in
'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
8. If you want to add or modify an error message, please read the guideline first:
https://spark.apache.org/error-message-guidelines.html
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
Code in [SymmetricHashJoinStateManager](https://github.com/apache/spark/blob/master/sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/state/SymmetricHashJoinStateManager.scala#L321) relies on the subclass implementations of [StateStoreCustomMetric](https://github.com/apache/spark/blob/master/sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/state/StateStore.scala#L187).
If a new subclass of `StateStoreCustomMetric` is added, it requires code changes to `SymmetricHashJoinStateManager` and we may miss the update if there is no existing test coverage. To prevent it, add a method `withNewDesc(desc : String)` to `StateStoreCustomMetric` and use it in `SymmetricHashJoinStateManager`
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
To prevent the above mentioned issues, add a method `withNewDesc(desc : String)` to `StateStoreCustomMetric` and use it in `SymmetricHashJoinStateManager`
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
If no, write 'No'.
-->
No
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
Existing 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.
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] SparkQA commented on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-862041004
**[Test build #139839 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139839/testReport)** for PR 32914 at commit [`7211c4b`](https://github.com/apache/spark/commit/7211c4b407e5bf8353af3d2cbf6f072c79d5a175).
--
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.
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] SparkQA commented on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-862072595
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44367/
--
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.
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] vkorukanti commented on a change in pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
vkorukanti commented on a change in pull request #32914:
URL: https://github.com/apache/spark/pull/32914#discussion_r651734966
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/state/SymmetricHashJoinStateManager.scala
##########
@@ -319,15 +319,7 @@ class SymmetricHashJoinStateManager(
keyWithIndexToValueMetrics.numKeys, // represent each buffered row only once
keyToNumValuesMetrics.memoryUsedBytes + keyWithIndexToValueMetrics.memoryUsedBytes,
keyWithIndexToValueMetrics.customMetrics.map {
- case (s @ StateStoreCustomSumMetric(_, desc), value) =>
Review comment:
The main point is to avoid making changes here (in `SymmetricHashJoinManager`) whenever you add a new case class. You just add all the code required in the case class when a new one is added. It is easy to miss adding the code here and with this patch we can avoid exposing the subclasses of `StateStoreCustomMetric` here.
--
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.
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] AmplabJenkins removed a comment on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-861999568
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/44358/
--
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.
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] AmplabJenkins commented on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-861980486
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/139830/
--
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.
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] tdas commented on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
tdas commented on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-861920495
LGTM once again. thank you for catching the other place where we were doing the same mistake.
--
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.
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] AmplabJenkins removed a comment on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-861980486
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/139830/
--
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.
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] SparkQA removed a comment on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-861969627
**[Test build #139830 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139830/testReport)** for PR 32914 at commit [`5dddc66`](https://github.com/apache/spark/commit/5dddc6644c9e76f78ebd0d44a4c23ee80b0fd55c).
--
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.
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] AmplabJenkins removed a comment on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-862040635
--
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.
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] tdas commented on a change in pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
tdas commented on a change in pull request #32914:
URL: https://github.com/apache/spark/pull/32914#discussion_r652206030
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/state/SymmetricHashJoinStateManager.scala
##########
@@ -319,15 +319,7 @@ class SymmetricHashJoinStateManager(
keyWithIndexToValueMetrics.numKeys, // represent each buffered row only once
keyToNumValuesMetrics.memoryUsedBytes + keyWithIndexToValueMetrics.memoryUsedBytes,
keyWithIndexToValueMetrics.customMetrics.map {
- case (s @ StateStoreCustomSumMetric(_, desc), value) =>
Review comment:
+1
this is easier to maintain. exhaustive enumeration of subclasses is brittle to addition of subclasses.
--
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.
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] SparkQA commented on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-862048837
**[Test build #139829 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139829/testReport)** for PR 32914 at commit [`1031593`](https://github.com/apache/spark/commit/10315931fff2ad06030cbc8e017f01e0be8593bc).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
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.
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] SparkQA commented on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-862033749
**[Test build #139833 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139833/testReport)** for PR 32914 at commit [`5dddc66`](https://github.com/apache/spark/commit/5dddc6644c9e76f78ebd0d44a4c23ee80b0fd55c).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
--
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.
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] SparkQA commented on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-862192731
**[Test build #139839 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139839/testReport)** for PR 32914 at commit [`7211c4b`](https://github.com/apache/spark/commit/7211c4b407e5bf8353af3d2cbf6f072c79d5a175).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
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.
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] AmplabJenkins commented on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-862040639
--
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.
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] SparkQA commented on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-862029828
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44361/
--
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.
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] zinking commented on a change in pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
zinking commented on a change in pull request #32914:
URL: https://github.com/apache/spark/pull/32914#discussion_r651701263
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/state/SymmetricHashJoinStateManager.scala
##########
@@ -319,15 +319,7 @@ class SymmetricHashJoinStateManager(
keyWithIndexToValueMetrics.numKeys, // represent each buffered row only once
keyToNumValuesMetrics.memoryUsedBytes + keyWithIndexToValueMetrics.memoryUsedBytes,
keyWithIndexToValueMetrics.customMetrics.map {
- case (s @ StateStoreCustomSumMetric(_, desc), value) =>
Review comment:
didn't quite get why it worths the effort, you have add code either here or in the case class. no difference to me.
or maybe you can save the effort by provide some default implementation.
--
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.
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] SparkQA commented on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-862007179
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44359/
--
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.
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] SparkQA commented on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-861995319
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44359/
--
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.
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] AmplabJenkins removed a comment on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-861233427
Can one of the admins verify this patch?
--
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.
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] SparkQA commented on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-861969627
**[Test build #139830 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139830/testReport)** for PR 32914 at commit [`5dddc66`](https://github.com/apache/spark/commit/5dddc6644c9e76f78ebd0d44a4c23ee80b0fd55c).
--
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.
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] AmplabJenkins commented on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-862019805
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/44359/
--
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.
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] SparkQA removed a comment on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-862041004
**[Test build #139839 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139839/testReport)** for PR 32914 at commit [`7211c4b`](https://github.com/apache/spark/commit/7211c4b407e5bf8353af3d2cbf6f072c79d5a175).
--
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.
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] HeartSaVioR commented on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-862001559
> https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139830
StackOverflowError happened while compiling... I'll retrigger again to see whether it's intermittent or not.
--
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.
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] SparkQA removed a comment on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-862002836
**[Test build #139833 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139833/testReport)** for PR 32914 at commit [`5dddc66`](https://github.com/apache/spark/commit/5dddc6644c9e76f78ebd0d44a4c23ee80b0fd55c).
--
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.
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] tdas commented on a change in pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
tdas commented on a change in pull request #32914:
URL: https://github.com/apache/spark/pull/32914#discussion_r652207235
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/state/SymmetricHashJoinStateManager.scala
##########
@@ -319,15 +319,7 @@ class SymmetricHashJoinStateManager(
keyWithIndexToValueMetrics.numKeys, // represent each buffered row only once
keyToNumValuesMetrics.memoryUsedBytes + keyWithIndexToValueMetrics.memoryUsedBytes,
keyWithIndexToValueMetrics.customMetrics.map {
- case (s @ StateStoreCustomSumMetric(_, desc), value) =>
Review comment:
it would be even better if with some scala magic the default implementation could be provided. but its not obvious to me how, because this is calling the copy constructor which is not available in any trait.
--
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.
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] AmplabJenkins removed a comment on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-862194355
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/139839/
--
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.
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] SparkQA commented on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-861980463
--
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.
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] HeartSaVioR commented on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-862001604
retest this, please
--
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.
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] AmplabJenkins removed a comment on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-862105282
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/44367/
--
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.
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] HeartSaVioR commented on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-861935874
--
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.
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] SparkQA commented on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-862017695
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44361/
--
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.
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] AmplabJenkins commented on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-861233427
Can one of the admins verify this patch?
--
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.
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] HeartSaVioR commented on a change in pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on a change in pull request #32914:
URL: https://github.com/apache/spark/pull/32914#discussion_r652223432
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/state/SymmetricHashJoinStateManager.scala
##########
@@ -319,15 +319,7 @@ class SymmetricHashJoinStateManager(
keyWithIndexToValueMetrics.numKeys, // represent each buffered row only once
keyToNumValuesMetrics.memoryUsedBytes + keyWithIndexToValueMetrics.memoryUsedBytes,
keyWithIndexToValueMetrics.customMetrics.map {
- case (s @ StateStoreCustomSumMetric(_, desc), value) =>
Review comment:
If we are also considering extension point from outside (like 3rd party state store provider) to add a new implementation of `StateStoreCustomMetric`, we should definitely not suppose the new implementation to be a case class having `desc` as field. (Even they have it, we can't use unless doing reflection.) I think this is the right way to do.
--
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.
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] AmplabJenkins commented on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-862105282
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/44367/
--
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.
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] SparkQA commented on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-861993839
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44358/
--
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.
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] HeartSaVioR commented on a change in pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on a change in pull request #32914:
URL: https://github.com/apache/spark/pull/32914#discussion_r652223432
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/state/SymmetricHashJoinStateManager.scala
##########
@@ -319,15 +319,7 @@ class SymmetricHashJoinStateManager(
keyWithIndexToValueMetrics.numKeys, // represent each buffered row only once
keyToNumValuesMetrics.memoryUsedBytes + keyWithIndexToValueMetrics.memoryUsedBytes,
keyWithIndexToValueMetrics.customMetrics.map {
- case (s @ StateStoreCustomSumMetric(_, desc), value) =>
Review comment:
If we are also considering extension point from outside (like 3rd party state store provider) to add a new implementation of `StateStoreCustomMetric`, we should definitely not suppose the new implementation to be a case class having `desc` as field. I think this is the right way to do.
--
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.
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] AmplabJenkins commented on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-861999568
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/44358/
--
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.
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] AmplabJenkins removed a comment on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-862019805
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/44359/
--
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.
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] AmplabJenkins commented on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-862194355
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/139839/
--
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.
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] SparkQA commented on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-861936755
**[Test build #139829 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139829/testReport)** for PR 32914 at commit [`1031593`](https://github.com/apache/spark/commit/10315931fff2ad06030cbc8e017f01e0be8593bc).
--
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.
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] SparkQA commented on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-862002836
**[Test build #139833 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139833/testReport)** for PR 32914 at commit [`5dddc66`](https://github.com/apache/spark/commit/5dddc6644c9e76f78ebd0d44a4c23ee80b0fd55c).
--
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.
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] HeartSaVioR commented on a change in pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on a change in pull request #32914:
URL: https://github.com/apache/spark/pull/32914#discussion_r652223432
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/state/SymmetricHashJoinStateManager.scala
##########
@@ -319,15 +319,7 @@ class SymmetricHashJoinStateManager(
keyWithIndexToValueMetrics.numKeys, // represent each buffered row only once
keyToNumValuesMetrics.memoryUsedBytes + keyWithIndexToValueMetrics.memoryUsedBytes,
keyWithIndexToValueMetrics.customMetrics.map {
- case (s @ StateStoreCustomSumMetric(_, desc), value) =>
Review comment:
If we are also considering extension point from outside (like 3rd party state store provider) to add a new implementation of `StateStoreCustomMetric`, we should definitely not suppose the new implementation to be a case class having `desc` as field. (Even they have it, we can't leverage it unless doing reflection.) I think this is the right way to do.
--
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.
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] HeartSaVioR commented on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-861935615
Indeed. I was about to comment on the missing spot but it was fixed so fast! Thanks for fixing the overall issue!
--
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.
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] SparkQA commented on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-862094960
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44367/
--
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.
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 pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-861416640
cc @HeartSaVioR @xuanyuanking FYI
--
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.
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] SparkQA removed a comment on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-861936755
**[Test build #139829 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139829/testReport)** for PR 32914 at commit [`1031593`](https://github.com/apache/spark/commit/10315931fff2ad06030cbc8e017f01e0be8593bc).
--
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.
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] AmplabJenkins commented on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-862069123
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/139829/
--
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.
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] AmplabJenkins removed a comment on pull request #32914: [SPARK-35763][SS] Add a new copy method to StateStoreCustomMetric
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32914:
URL: https://github.com/apache/spark/pull/32914#issuecomment-862069123
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/139829/
--
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.
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