You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "pnowojski (via GitHub)" <gi...@apache.org> on 2024/08/20 11:52:10 UTC
[PR] (1.19 backport) [FLINK-35886][task] Fix watermark idleness timeout accounting when subtask is backpressured/blocked [flink]
pnowojski opened a new pull request, #25223:
URL: https://github.com/apache/flink/pull/25223
this is a backport of https://github.com/apache/flink/pull/25167
--
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
Re: [PR] (1.19 backport) [FLINK-35886][task] Fix watermark idleness timeout accounting when subtask is backpressured/blocked [flink]
Posted by "flinkbot (via GitHub)" <gi...@apache.org>.
flinkbot commented on PR #25223:
URL: https://github.com/apache/flink/pull/25223#issuecomment-2298676411
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "2f64401e30c571adf949de9b705b531f8a267256",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "2f64401e30c571adf949de9b705b531f8a267256",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 2f64401e30c571adf949de9b705b531f8a267256 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
Re: [PR] (1.19 backport) [FLINK-35886][task] Fix watermark idleness timeout accounting when subtask is backpressured/blocked [flink]
Posted by "rkhachatryan (via GitHub)" <gi...@apache.org>.
rkhachatryan commented on code in PR #25223:
URL: https://github.com/apache/flink/pull/25223#discussion_r1723771139
##########
pom.xml:
##########
@@ -2355,6 +2355,10 @@ under the License.
<exclude>@org.apache.flink.annotation.Experimental</exclude>
<exclude>@org.apache.flink.annotation.Internal</exclude>
<!-- MARKER: start exclusions; these will be wiped by tools/releasing/update_japicmp_configuration.sh -->
+ <!-- New method has been added to the interface, but this interface shouldn't be implemented by users, only used/called, so this doesn't break API compatibility. -->
+ <exclude>org.apache.flink.api.common.eventtime.WatermarkGeneratorSupplier$Context</exclude>
+ <!-- Base interface has been extracted from the existing Clock class. This interface shouldn't affect user code. -->
+ <exclude>org.apache.flink.util.clock.Clock</exclude>
Review Comment:
It would be nice to return System clock from the new method (`WatermarkGeneratorSupplier.Context#getInputActivityClock`) to avoid breaking existing implementations.
I think it's not impossible that there are custom implementations of this interface.
--
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
Re: [PR] (1.19 backport) [FLINK-35886][task] Fix watermark idleness timeout accounting when subtask is backpressured/blocked [flink]
Posted by "pnowojski (via GitHub)" <gi...@apache.org>.
pnowojski merged PR #25223:
URL: https://github.com/apache/flink/pull/25223
--
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
Re: [PR] (1.19 backport) [FLINK-35886][task] Fix watermark idleness timeout accounting when subtask is backpressured/blocked [flink]
Posted by "pnowojski (via GitHub)" <gi...@apache.org>.
pnowojski commented on code in PR #25223:
URL: https://github.com/apache/flink/pull/25223#discussion_r1728928197
##########
pom.xml:
##########
@@ -2355,6 +2355,10 @@ under the License.
<exclude>@org.apache.flink.annotation.Experimental</exclude>
<exclude>@org.apache.flink.annotation.Internal</exclude>
<!-- MARKER: start exclusions; these will be wiped by tools/releasing/update_japicmp_configuration.sh -->
+ <!-- New method has been added to the interface, but this interface shouldn't be implemented by users, only used/called, so this doesn't break API compatibility. -->
+ <exclude>org.apache.flink.api.common.eventtime.WatermarkGeneratorSupplier$Context</exclude>
+ <!-- Base interface has been extracted from the existing Clock class. This interface shouldn't affect user code. -->
+ <exclude>org.apache.flink.util.clock.Clock</exclude>
Review Comment:
Actually this `WatermarkGeneratorSupplier.Context#getInputActivityClock`, turns out to be a problem and user code actually implements on its own `WatermarkGeneratorSupplier.Context` interface. For example our Kafka connector does this here: https://github.com/apache/flink-connector-kafka/blob/main/flink-connector-kafka/src/main/java/org/apache/flink/streaming/connectors/kafka/internals/AbstractFetcher.java#L438-L439
Due to this reason, I don't see an easy way how to backport this bug fix to release 1.19 and 1.20 and I would suggest to only fix the bug in 2.0. Especially given that there doesn't seem to be a large interest in this bug fix (it hasn't been reported before). I will revert my changes for 1.19 and 1.20
--
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
Re: [PR] (1.19 backport) [FLINK-35886][task] Fix watermark idleness timeout accounting when subtask is backpressured/blocked [flink]
Posted by "pnowojski (via GitHub)" <gi...@apache.org>.
pnowojski commented on code in PR #25223:
URL: https://github.com/apache/flink/pull/25223#discussion_r1723182222
##########
pom.xml:
##########
@@ -2355,6 +2355,10 @@ under the License.
<exclude>@org.apache.flink.annotation.Experimental</exclude>
<exclude>@org.apache.flink.annotation.Internal</exclude>
<!-- MARKER: start exclusions; these will be wiped by tools/releasing/update_japicmp_configuration.sh -->
+ <!-- New method has been added to the interface, but this interface shouldn't be implemented by users, only used/called, so this doesn't break API compatibility. -->
+ <exclude>org.apache.flink.api.common.eventtime.WatermarkGeneratorSupplier$Context</exclude>
+ <!-- Base interface has been extracted from the existing Clock class. This interface shouldn't affect user code. -->
+ <exclude>org.apache.flink.util.clock.Clock</exclude>
Review Comment:
Please pay attention to this
--
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
Re: [PR] (1.19 backport) [FLINK-35886][task] Fix watermark idleness timeout accounting when subtask is backpressured/blocked [flink]
Posted by "pnowojski (via GitHub)" <gi...@apache.org>.
pnowojski commented on code in PR #25223:
URL: https://github.com/apache/flink/pull/25223#discussion_r1724488059
##########
pom.xml:
##########
@@ -2355,6 +2355,10 @@ under the License.
<exclude>@org.apache.flink.annotation.Experimental</exclude>
<exclude>@org.apache.flink.annotation.Internal</exclude>
<!-- MARKER: start exclusions; these will be wiped by tools/releasing/update_japicmp_configuration.sh -->
+ <!-- New method has been added to the interface, but this interface shouldn't be implemented by users, only used/called, so this doesn't break API compatibility. -->
+ <exclude>org.apache.flink.api.common.eventtime.WatermarkGeneratorSupplier$Context</exclude>
+ <!-- Base interface has been extracted from the existing Clock class. This interface shouldn't affect user code. -->
+ <exclude>org.apache.flink.util.clock.Clock</exclude>
Review Comment:
This context is provided by Flink to operators/functions, so I don't see any legitimate way how user should be interested in implementing it's own context. This could only happen if someone forks Flink and extends it somehow in this area AFAIU. I will merge it as is.
--
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