You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2020/10/01 00:12:34 UTC
[GitHub] [beam] rohdesamuel opened a new pull request #12985: [BEAM-10994] Adds ability to log literal hot key in Batch Dataflow
rohdesamuel opened a new pull request #12985:
URL: https://github.com/apache/beam/pull/12985
The logging of the key is controlled by the DataflowPipelineOption `hotKeyLoggingEnabled`. When true and a hot key is detected from Dataflow, the literal hot key content will be logged to Cloud Logging. Otherwise, the default message without a key will be displayed.
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
- [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
Post-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
Lang | SDK | Dataflow | Flink | Samza | Spark | Twister2
--- | --- | --- | --- | --- | --- | ---
Go | [](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/) | ---
Java | [](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/)
Python | [](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Python_PVR_Flink_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/) | ---
XLang | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/) | ---
Pre-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
--- |Java | Python | Go | Website | Whitespace | Typescript
--- | --- | --- | --- | --- | --- | ---
Non-portable | [](https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonDocker_Cron/lastCompletedBuild/) <br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonDocs_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Whitespace_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Typescript_Cron/lastCompletedBuild/)
Portable | --- | [](https://ci-beam.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/) | --- | --- | --- | ---
See [.test-infra/jenkins/README](https://github.com/apache/beam/blob/master/.test-infra/jenkins/README.md) for trigger phrase, status and link of all Jenkins jobs.
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
[](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
----------------------------------------------------------------
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
[GitHub] [beam] rohdesamuel commented on a change in pull request #12985: [BEAM-10994] Adds ability to log literal hot key in Batch Dataflow
Posted by GitBox <gi...@apache.org>.
rohdesamuel commented on a change in pull request #12985:
URL: https://github.com/apache/beam/pull/12985#discussion_r504869807
##########
File path: runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/DataflowWorkProgressUpdater.java
##########
@@ -103,9 +112,17 @@ protected void reportProgressHelper() throws Exception {
if (result.getHotKeyDetection() != null
&& result.getHotKeyDetection().getUserStepName() != null) {
HotKeyDetection hotKeyDetection = result.getHotKeyDetection();
+
+ // The key set the in BatchModeExecutionContext is only set in the GroupingShuffleReader
+ // which is the correct key. The key is also translated into a Java object in the reader.
+ Object hotkey =
+ options.isHotKeyLoggingEnabled()
+ ? workItemStatusClient.getExecutionContext().getKey()
+ : null;
Review comment:
Gotcha, I changed the logging
----------------------------------------------------------------
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
[GitHub] [beam] lukecwik merged pull request #12985: [BEAM-10994] Adds ability to log literal hot key in Batch Dataflow
Posted by GitBox <gi...@apache.org>.
lukecwik merged pull request #12985:
URL: https://github.com/apache/beam/pull/12985
----------------------------------------------------------------
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
[GitHub] [beam] lukecwik commented on a change in pull request #12985: [BEAM-10994] Adds ability to log literal hot key in Batch Dataflow
Posted by GitBox <gi...@apache.org>.
lukecwik commented on a change in pull request #12985:
URL: https://github.com/apache/beam/pull/12985#discussion_r504302604
##########
File path: runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/DataflowWorkProgressUpdater.java
##########
@@ -103,9 +112,17 @@ protected void reportProgressHelper() throws Exception {
if (result.getHotKeyDetection() != null
&& result.getHotKeyDetection().getUserStepName() != null) {
HotKeyDetection hotKeyDetection = result.getHotKeyDetection();
+
+ // The key set the in BatchModeExecutionContext is only set in the GroupingShuffleReader
+ // which is the correct key. The key is also translated into a Java object in the reader.
+ Object hotkey =
+ options.isHotKeyLoggingEnabled()
+ ? workItemStatusClient.getExecutionContext().getKey()
+ : null;
Review comment:
That is a bug.
----------------------------------------------------------------
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
[GitHub] [beam] rohdesamuel commented on a change in pull request #12985: [BEAM-10994] Adds ability to log literal hot key in Batch Dataflow
Posted by GitBox <gi...@apache.org>.
rohdesamuel commented on a change in pull request #12985:
URL: https://github.com/apache/beam/pull/12985#discussion_r504183219
##########
File path: runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/DataflowWorkProgressUpdater.java
##########
@@ -103,9 +112,17 @@ protected void reportProgressHelper() throws Exception {
if (result.getHotKeyDetection() != null
&& result.getHotKeyDetection().getUserStepName() != null) {
HotKeyDetection hotKeyDetection = result.getHotKeyDetection();
+
+ // The key set the in BatchModeExecutionContext is only set in the GroupingShuffleReader
+ // which is the correct key. The key is also translated into a Java object in the reader.
+ Object hotkey =
+ options.isHotKeyLoggingEnabled()
+ ? workItemStatusClient.getExecutionContext().getKey()
+ : null;
Review comment:
In the BatchModeExecutionContext the getKey method's comment states "If there is not a currently defined key, returns null." Is this an error? Do you want me to update the comment?
----------------------------------------------------------------
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
[GitHub] [beam] lukecwik commented on pull request #12985: [BEAM-10994] Adds ability to log literal hot key in Batch Dataflow
Posted by GitBox <gi...@apache.org>.
lukecwik commented on pull request #12985:
URL: https://github.com/apache/beam/pull/12985#issuecomment-707907803
R: @lukecwik
----------------------------------------------------------------
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
[GitHub] [beam] lukecwik commented on a change in pull request #12985: [BEAM-10994] Adds ability to log literal hot key in Batch Dataflow
Posted by GitBox <gi...@apache.org>.
lukecwik commented on a change in pull request #12985:
URL: https://github.com/apache/beam/pull/12985#discussion_r504145753
##########
File path: runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/DataflowWorkProgressUpdater.java
##########
@@ -103,9 +112,17 @@ protected void reportProgressHelper() throws Exception {
if (result.getHotKeyDetection() != null
&& result.getHotKeyDetection().getUserStepName() != null) {
HotKeyDetection hotKeyDetection = result.getHotKeyDetection();
+
+ // The key set the in BatchModeExecutionContext is only set in the GroupingShuffleReader
+ // which is the correct key. The key is also translated into a Java object in the reader.
+ Object hotkey =
+ options.isHotKeyLoggingEnabled()
+ ? workItemStatusClient.getExecutionContext().getKey()
+ : null;
Review comment:
See my earlier comment on #12984 about assuming that `key == null` is the same thing as not present.
----------------------------------------------------------------
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