[jira] [Commented] (FLINK-13004) Correct the logic of needToCleanupState in KeyedProcessFunctionWithCleanupState
[
https://issues.apache.org/jira/browse/FLINK-13004?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16873994#comment-16873994
]
Jark Wu commented on FLINK-13004:
-
[~yunta] (y)
> Correct the logic of needToCleanupState in
> KeyedProcessFunctionWithCleanupState
> ---
>
> Key: FLINK-13004
> URL: https://issues.apache.org/jira/browse/FLINK-13004
> Project: Flink
> Issue Type: Bug
> Components: Table SQL / Planner
>Reporter: Yun Tang
>Assignee: Yun Tang
>Priority: Major
> Labels: pull-request-available
> Time Spent: 10m
> Remaining Estimate: 0h
>
> Current implementation of needToCleanupState in
> KeyedProcessFunctionWithCleanupState actually has potention bug:
> {code:java}
> protected Boolean needToCleanupState(Long timestamp) throws IOException {
>if (stateCleaningEnabled) {
> Long cleanupTime = cleanupTimeState.value();
> // check that the triggered timer is the last registered processing
> time timer.
> return null != cleanupTime && timestamp == cleanupTime;
>} else {
> return false;
>}
> }
> {code}
> Please note that it directly use "==" to judge whether *Long* type timestamp
> and cleanupTime equals. However, if that value is larger than 127L, the
> result would actually return false instead of wanted true.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (FLINK-13004) Correct the logic of needToCleanupState in KeyedProcessFunctionWithCleanupState
[
https://issues.apache.org/jira/browse/FLINK-13004?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16873815#comment-16873815
]
Yun Tang commented on FLINK-13004:
--
[~jark], actually, the {{timestamp}} passed in was wrapper {{Long.}} For the
comparison of wrapper and primitive value problem, you could refer to [question
here|https://stackoverflow.com/questions/19485818/what-are-not-2-long-variables-equal-with-operator-to-compare-in-java].
I am preparing a PR to fix this and found previous tests have been modified to
cater for this bug.
> Correct the logic of needToCleanupState in
> KeyedProcessFunctionWithCleanupState
> ---
>
> Key: FLINK-13004
> URL: https://issues.apache.org/jira/browse/FLINK-13004
> Project: Flink
> Issue Type: Bug
> Components: Table SQL / Planner
>Reporter: Yun Tang
>Assignee: Yun Tang
>Priority: Major
>
> Current implementation of needToCleanupState in
> KeyedProcessFunctionWithCleanupState actually has potention bug:
> {code:java}
> protected Boolean needToCleanupState(Long timestamp) throws IOException {
>if (stateCleaningEnabled) {
> Long cleanupTime = cleanupTimeState.value();
> // check that the triggered timer is the last registered processing
> time timer.
> return null != cleanupTime && timestamp == cleanupTime;
>} else {
> return false;
>}
> }
> {code}
> Please note that it directly use "==" to judge whether *Long* type timestamp
> and cleanupTime equals. However, if that value is larger than 127L, the
> result would actually return false instead of wanted true.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (FLINK-13004) Correct the logic of needToCleanupState in KeyedProcessFunctionWithCleanupState
[
https://issues.apache.org/jira/browse/FLINK-13004?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16873756#comment-16873756
]
Jark Wu commented on FLINK-13004:
-
I think the {{timestamp}} parameter should be primitive long. Thanks for
reporting this.
Could your elaborate more about why larger than 127L, the result would return
false?
> Correct the logic of needToCleanupState in
> KeyedProcessFunctionWithCleanupState
> ---
>
> Key: FLINK-13004
> URL: https://issues.apache.org/jira/browse/FLINK-13004
> Project: Flink
> Issue Type: Bug
> Components: Table SQL / Planner
>Reporter: Yun Tang
>Assignee: Yun Tang
>Priority: Major
>
> Current implementation of needToCleanupState in
> KeyedProcessFunctionWithCleanupState actually has potention bug:
> {code:java}
> protected Boolean needToCleanupState(Long timestamp) throws IOException {
>if (stateCleaningEnabled) {
> Long cleanupTime = cleanupTimeState.value();
> // check that the triggered timer is the last registered processing
> time timer.
> return null != cleanupTime && timestamp == cleanupTime;
>} else {
> return false;
>}
> }
> {code}
> Please note that it directly use "==" to judge whether *Long* type timestamp
> and cleanupTime equals. However, if that value is larger than 127L, the
> result would actually return false instead of wanted true.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
