You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "dmvk (via GitHub)" <gi...@apache.org> on 2023/03/08 14:04:13 UTC
[GitHub] [flink] dmvk opened a new pull request, #22134: [FLINK-31370] Prevent more timers from being fired if the StreamTask…
dmvk opened a new pull request, #22134:
URL: https://github.com/apache/flink/pull/22134
https://issues.apache.org/jira/browse/FLINK-31370
If the task is canceled while the watermark progresses, it may be stuck in the Cancelling state for a long time (e.g., when many windows are firing). This is closely related to [FLINK-20217](https://issues.apache.org/jira/browse/FLINK-20217), which might bring a more robust solution for checkpoint and cancellation code paths.
As a stopgap solution, we'll introduce a check allowing InternalTimerService to break out of the firing loop if the StreamTask has been marked as canceled.
*Performance consideration*: This adds a lookup to a volatile field on the per-timer code path.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] dmvk commented on pull request #22134: [FLINK-31370] Prevent more timers from being fired if the StreamTask…
Posted by "dmvk (via GitHub)" <gi...@apache.org>.
dmvk commented on PR #22134:
URL: https://github.com/apache/flink/pull/22134#issuecomment-1460402364
Thanks for the review, @pnowojski! I've added the same logic for the processing time timers. It would be super helpful if you could verify the change against benchmarks.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] pnowojski commented on a diff in pull request #22134: [FLINK-31370] Prevent more timers from being fired if the StreamTask…
Posted by "pnowojski (via GitHub)" <gi...@apache.org>.
pnowojski commented on code in PR #22134:
URL: https://github.com/apache/flink/pull/22134#discussion_r1129592490
##########
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/operators/InternalTimerServiceImpl.java:
##########
@@ -296,7 +302,9 @@ public void advanceWatermark(long time) throws Exception {
InternalTimer<K, N> timer;
- while ((timer = eventTimeTimersQueue.peek()) != null && timer.getTimestamp() <= time) {
+ while ((timer = eventTimeTimersQueue.peek()) != null
+ && timer.getTimestamp() <= time
+ && !cancellationContext.isCancelled()) {
Review Comment:
I would add the same check 10 lines above in the `onProcessingTimer()` method (and a test for 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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot commented on pull request #22134: [FLINK-31370] Prevent more timers from being fired if the StreamTask…
Posted by "flinkbot (via GitHub)" <gi...@apache.org>.
flinkbot commented on PR #22134:
URL: https://github.com/apache/flink/pull/22134#issuecomment-1460212176
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "4eac832e73fb0a8ca8ee44338f9ff30ca0dcfe34",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "4eac832e73fb0a8ca8ee44338f9ff30ca0dcfe34",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 4eac832e73fb0a8ca8ee44338f9ff30ca0dcfe34 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run azure` re-run the last Azure build
</details>
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] pnowojski commented on pull request #22134: [FLINK-31370] Prevent more timers from being fired if the StreamTask…
Posted by "pnowojski (via GitHub)" <gi...@apache.org>.
pnowojski commented on PR #22134:
URL: https://github.com/apache/flink/pull/22134#issuecomment-1461826990
I've started two benchmark runs:
http://codespeed.dak8s.net:8080/job/flink-benchmark-request/212/ just for `ProcessingTimerBenchmark` with JDK8
http://codespeed.dak8s.net:8080/job/flink-benchmark-request/213/ for all benchmarks with JDK11
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] dmvk merged pull request #22134: [FLINK-31370] Prevent more timers from being fired if the StreamTask…
Posted by "dmvk (via GitHub)" <gi...@apache.org>.
dmvk merged PR #22134:
URL: https://github.com/apache/flink/pull/22134
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org