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 2021/02/15 05:40:00 UTC
[GitHub] [beam] baeminbo opened a new pull request #13993: Fix IllegalArgumentException in Interval
baeminbo opened a new pull request #13993:
URL: https://github.com/apache/beam/pull/13993
If a time correction happens, `endTime` can be before `startTime, which cause `IllegalArgumentException` (the end instant must be greater than the start instant) in `DataflowWorkerUnitClient.reportWorkItemStatus`.
Compute the elapsed time by `endTime.getMillis() - startTime.getMills()` which may be a negative value but I think it's ok because it's only logging purpose.
**Please** add a meaningful description for your change here
------------------------
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_VR_Dataflow_V2/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_Dataflow/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] pabloem commented on a change in pull request #13993: Fix IllegalArgumentException in Interval
Posted by GitBox <gi...@apache.org>.
pabloem commented on a change in pull request #13993:
URL: https://github.com/apache/beam/pull/13993#discussion_r577826147
##########
File path: runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/DataflowWorkUnitClient.java
##########
@@ -215,14 +214,15 @@ public WorkItemServiceState reportWorkItemStatus(WorkItemStatus workItemStatus)
&& DataflowWorkerLoggingMDC.getStageName() != null) {
DateTime startTime = stageStartTime.get();
if (startTime != null) {
- // This thread should have been tagged with the stage start time during getWorkItem(),
- Interval elapsed = new Interval(startTime, endTime);
+ // elapsed time can be negative by time correction
+ long elapsed = endTime.getMillis() - startTime.getMillis();
Review comment:
oh I see your explanation at the top. Fine.
----------------------------------------------------------------
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] baeminbo commented on pull request #13993: Fix IllegalArgumentException in Interval
Posted by GitBox <gi...@apache.org>.
baeminbo commented on pull request #13993:
URL: https://github.com/apache/beam/pull/13993#issuecomment-778959791
R: @pabloem Can you review this change?
----------------------------------------------------------------
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] pabloem merged pull request #13993: [BEAM-11832] Fix IllegalArgumentException in Interval
Posted by GitBox <gi...@apache.org>.
pabloem merged pull request #13993:
URL: https://github.com/apache/beam/pull/13993
----------------------------------------------------------------
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] pabloem commented on pull request #13993: Fix IllegalArgumentException in Interval
Posted by GitBox <gi...@apache.org>.
pabloem commented on pull request #13993:
URL: https://github.com/apache/beam/pull/13993#issuecomment-780745042
is there some research that went into this? Can you share some more info about 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] baeminbo commented on pull request #13993: Fix IllegalArgumentException in Interval
Posted by GitBox <gi...@apache.org>.
baeminbo commented on pull request #13993:
URL: https://github.com/apache/beam/pull/13993#issuecomment-781118798
I added BEAM-11832 for skewed metric issue by time correction.
For `IllegalArgumentException` at `DataflowWorkUnitClient`, I saw a couple of cases where some works were retried with the following error.
```
{
insertId: <REDACTED>
jsonPayload: {
exception: "java.lang.IllegalArgumentException: The end instant must be greater than the start instant
at org.joda.time.base.AbstractInterval.checkInterval(AbstractInterval.java:63)
at org.joda.time.base.BaseInterval.<init>(BaseInterval.java:94)
at org.joda.time.Interval.<init>(Interval.java:199)
at org.apache.beam.runners.dataflow.worker.DataflowWorkUnitClient.reportWorkItemStatus(DataflowWorkUnitClient.java:216)
at org.apache.beam.runners.dataflow.worker.WorkItemStatusClient.execute(WorkItemStatusClient.java:193)
at org.apache.beam.runners.dataflow.worker.WorkItemStatusClient.reportSuccess(WorkItemStatusClient.java:162)
at org.apache.beam.runners.dataflow.worker.BatchDataflowWorker.doWork(BatchDataflowWorker.java:382)
at org.apache.beam.runners.dataflow.worker.BatchDataflowWorker.getAndPerformWork(BatchDataflowWorker.java:306)
at org.apache.beam.runners.dataflow.worker.DataflowBatchWorkerHarness$WorkerThread.doWork(DataflowBatchWorkerHarness.java:140)
at org.apache.beam.runners.dataflow.worker.DataflowBatchWorkerHarness$WorkerThread.call(DataflowBatchWorkerHarness.java:120)
at org.apache.beam.runners.dataflow.worker.DataflowBatchWorkerHarness$WorkerThread.call(DataflowBatchWorkerHarness.java:107)
at java.util.concurrent.FutureTask.run(FutureTask.java:266)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
at java.lang.Thread.run(Thread.java:748)
"
job: <REDACTED>
logger: "org.apache.beam.runners.dataflow.worker.WorkItemStatusClient"
message: "Failure processing work item <REDACTED>;<REDACTED>;<REDACTED>: Uncaught exception occurred during work unit execution. This will be retried."
stage: <REDACTED>
thread: "25"
work: <REDACTED>
worker: <REDACTED>
}
labels: {
<REDACTED>
}
logName: "projects/<REDACTED>/logs/dataflow.googleapis.com%2Fworker"
receiveTimestamp: <REDACTED>
resource: {
labels: {
<REDACTED>
}
type: "dataflow_step"
}
severity: "ERROR"
timestamp: <REDACTED>
}
```
I confirmed there were time correction before the log above in `system` logs.
```
System clock wrong by -1.043293 seconds, adjustment started
System clock was stepped by -1.043293 seconds
```
----------------------------------------------------------------
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] pabloem commented on pull request #13993: Fix IllegalArgumentException in Interval
Posted by GitBox <gi...@apache.org>.
pabloem commented on pull request #13993:
URL: https://github.com/apache/beam/pull/13993#issuecomment-781496142
very interesting. Thanks Minbo
----------------------------------------------------------------
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] pabloem commented on pull request #13993: Fix IllegalArgumentException in Interval
Posted by GitBox <gi...@apache.org>.
pabloem commented on pull request #13993:
URL: https://github.com/apache/beam/pull/13993#issuecomment-780892314
can you add a TODO to fix the main issue that a date correction can skew our metrics?
----------------------------------------------------------------
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] pabloem commented on a change in pull request #13993: Fix IllegalArgumentException in Interval
Posted by GitBox <gi...@apache.org>.
pabloem commented on a change in pull request #13993:
URL: https://github.com/apache/beam/pull/13993#discussion_r577825729
##########
File path: runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/DataflowWorkUnitClient.java
##########
@@ -215,14 +214,15 @@ public WorkItemServiceState reportWorkItemStatus(WorkItemStatus workItemStatus)
&& DataflowWorkerLoggingMDC.getStageName() != null) {
DateTime startTime = stageStartTime.get();
if (startTime != null) {
- // This thread should have been tagged with the stage start time during getWorkItem(),
- Interval elapsed = new Interval(startTime, endTime);
+ // elapsed time can be negative by time correction
+ long elapsed = endTime.getMillis() - startTime.getMillis();
Review comment:
Can you explain how this is different from what we were doing before?
----------------------------------------------------------------
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