You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "1996fanrui (via GitHub)" <gi...@apache.org> on 2024/02/02 05:09:48 UTC
[PR] [FLINK-34336][test] Fix the bug that AutoRescalingITCase#testCheckpointRescalingWithKeyedAndNonPartitionedState may hang sometimes [flink]
1996fanrui opened a new pull request, #24248:
URL: https://github.com/apache/flink/pull/24248
## Purpose
AutoRescalingITCase#testCheckpointRescalingWithKeyedAndNonPartitionedState may hang in waitForRunningTasks(restClusterClient, jobID, parallelism2);
## Reason:
The job has 2 tasks(vertices), after calling updateJobResourceRequirements. The source parallelism isn't changed (It's parallelism) , and the FlatMapper+Sink is changed from parallelism to parallelism2.
So we expect the task number should be parallelism + parallelism2 instead of parallelism2.
## Why it can be passed for now?
Flink 1.19 supports the scaling cooldown, and the cooldown time is 30s by default. It means, flink job will rescale job 30 seconds after updateJobResourceRequirements is called.
So the running tasks are old parallelism when we call waitForRunningTasks(restClusterClient, jobID, parallelism2);.
IIUC, it cannot be guaranteed, and it's unexpected.
## How to reproduce this bug?
https://github.com/1996fanrui/flink/commit/ffd713e24d37db2c103e4cd4361d0cd916d0d2f6
1. Disable the cooldown
2. Sleep for a while before waitForRunningTasks
--
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] [FLINK-34336][test] Fix the bug that AutoRescalingITCase#testCheckpointRescalingWithKeyedAndNonPartitionedState may hang sometimes [flink]
Posted by "flinkbot (via GitHub)" <gi...@apache.org>.
flinkbot commented on PR #24248:
URL: https://github.com/apache/flink/pull/24248#issuecomment-1922845516
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "9732c01c33a38cb989d9fb73a5838329f35d8f7e",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "9732c01c33a38cb989d9fb73a5838329f35d8f7e",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 9732c01c33a38cb989d9fb73a5838329f35d8f7e 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] [FLINK-34336][test] Fix the bug that AutoRescalingITCase may hang sometimes [flink]
Posted by "XComp (via GitHub)" <gi...@apache.org>.
XComp commented on code in PR #24248:
URL: https://github.com/apache/flink/pull/24248#discussion_r1494741651
##########
flink-tests/src/test/java/org/apache/flink/test/checkpointing/AutoRescalingITCase.java:
##########
@@ -427,7 +427,8 @@ public void testCheckpointRescalingWithKeyedAndNonPartitionedState() throws Exce
restClusterClient.updateJobResourceRequirements(jobID, builder.build()).join();
- waitForRunningTasks(restClusterClient, jobID, parallelism2);
+ // Source is parallelism, the flatMapper & Sink is parallelism2
+ waitForRunningTasks(restClusterClient, jobID, parallelism + parallelism2);
Review Comment:
I checked that the shortening the cooldown phase works in PR #24246 . Not sure what I meant initially. :innocent:
--
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] [FLINK-34336][test] Fix the bug that AutoRescalingITCase may hang sometimes [flink]
Posted by "1996fanrui (via GitHub)" <gi...@apache.org>.
1996fanrui commented on PR #24248:
URL: https://github.com/apache/flink/pull/24248#issuecomment-1953382889
> LGTM 👍
Thanks @XComp for the review! Merging
> Can you create a 1.19 backport PR?
Sure, I will do it asap.
--
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] [FLINK-34336][test] Fix the bug that AutoRescalingITCase#testCheckpointRescalingWithKeyedAndNonPartitionedState may hang sometimes [flink]
Posted by "1996fanrui (via GitHub)" <gi...@apache.org>.
1996fanrui commented on PR #24248:
URL: https://github.com/apache/flink/pull/24248#issuecomment-1923016887
org.apache.flink.test.checkpointing.AutoRescalingITCase#testCheckpointRescalingNonPartitionedStateCausesException hangs as well.
I checked, all test jobs of AutoRescalingITCase have more than 1 job vertex. So the `desiredNumberOfRunningTasks` of all `waitForRunningTasks` should be `2 * parallelism2` or `p1 + p2`.
--
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] [FLINK-34336][test] Fix the bug that AutoRescalingITCase may hang sometimes [flink]
Posted by "XComp (via GitHub)" <gi...@apache.org>.
XComp commented on code in PR #24248:
URL: https://github.com/apache/flink/pull/24248#discussion_r1478245157
##########
flink-tests/src/test/java/org/apache/flink/test/checkpointing/AutoRescalingITCase.java:
##########
@@ -337,7 +337,7 @@ public void testCheckpointRescalingNonPartitionedStateCausesException() throws E
restClusterClient.updateJobResourceRequirements(jobID, builder.build()).join();
- waitForRunningTasks(restClusterClient, jobID, parallelism2);
+ waitForRunningTasks(restClusterClient, jobID, 2 * parallelism2);
Review Comment:
this is out-of-scope for FLINK-34336? Was it accidentally added?
##########
flink-tests/src/test/java/org/apache/flink/test/checkpointing/AutoRescalingITCase.java:
##########
@@ -427,7 +427,8 @@ public void testCheckpointRescalingWithKeyedAndNonPartitionedState() throws Exce
restClusterClient.updateJobResourceRequirements(jobID, builder.build()).join();
- waitForRunningTasks(restClusterClient, jobID, parallelism2);
+ // Source is parallelism, the flatMapper & Sink is parallelism2
+ waitForRunningTasks(restClusterClient, jobID, parallelism + parallelism2);
Review Comment:
Can't we also reduce the cooldown phase to make the test complete faster similarly to what you did in https://github.com/1996fanrui/flink/commit/ffd713e24d37db2c103e4cd4361d0cd916d0d2f6? I tried it locally but it didn't help, though. :thinking:
--
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] [FLINK-34336][test] Fix the bug that AutoRescalingITCase may hang sometimes [flink]
Posted by "1996fanrui (via GitHub)" <gi...@apache.org>.
1996fanrui commented on code in PR #24248:
URL: https://github.com/apache/flink/pull/24248#discussion_r1479228550
##########
flink-tests/src/test/java/org/apache/flink/test/checkpointing/AutoRescalingITCase.java:
##########
@@ -337,7 +337,7 @@ public void testCheckpointRescalingNonPartitionedStateCausesException() throws E
restClusterClient.updateJobResourceRequirements(jobID, builder.build()).join();
- waitForRunningTasks(restClusterClient, jobID, parallelism2);
+ waitForRunningTasks(restClusterClient, jobID, 2 * parallelism2);
Review Comment:
The last line calls the rescale api, IIUC, `waitForRunningTasks` should wait for all tasks of rescaled job.
The task number after rescaling should be `2 * parallelism2`.
Other tests are similar with here, but other tests wait for all task of rescaled job.
--
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] [FLINK-34336][test] Fix the bug that AutoRescalingITCase may hang sometimes [flink]
Posted by "1996fanrui (via GitHub)" <gi...@apache.org>.
1996fanrui commented on code in PR #24248:
URL: https://github.com/apache/flink/pull/24248#discussion_r1479228550
##########
flink-tests/src/test/java/org/apache/flink/test/checkpointing/AutoRescalingITCase.java:
##########
@@ -337,7 +337,7 @@ public void testCheckpointRescalingNonPartitionedStateCausesException() throws E
restClusterClient.updateJobResourceRequirements(jobID, builder.build()).join();
- waitForRunningTasks(restClusterClient, jobID, parallelism2);
+ waitForRunningTasks(restClusterClient, jobID, 2 * parallelism2);
Review Comment:
The last line calls the rescale api, IIUC, `waitForRunningTasks` should wait for all tasks of rescaled job.
The task number after rescaling should be `2 * parallelism2`.
That's why I think it's original bug, and it's out-of-scope for FLINK-34336.
Other tests are similar with here, but other tests wait for all task of rescaled job.
##########
flink-tests/src/test/java/org/apache/flink/test/checkpointing/AutoRescalingITCase.java:
##########
@@ -337,7 +337,7 @@ public void testCheckpointRescalingNonPartitionedStateCausesException() throws E
restClusterClient.updateJobResourceRequirements(jobID, builder.build()).join();
- waitForRunningTasks(restClusterClient, jobID, parallelism2);
+ waitForRunningTasks(restClusterClient, jobID, 2 * parallelism2);
Review Comment:
The last line calls the rescale api, IIUC, `waitForRunningTasks` should wait for all tasks of rescaled job.
The task number after rescaling should be `2 * parallelism2`.
That's why I think it's an original bug, and it's out-of-scope for FLINK-34336.
Other tests are similar with here, but other tests wait for all task of rescaled job.
--
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] [FLINK-34336][test] Fix the bug that AutoRescalingITCase may hang sometimes [flink]
Posted by "1996fanrui (via GitHub)" <gi...@apache.org>.
1996fanrui commented on code in PR #24248:
URL: https://github.com/apache/flink/pull/24248#discussion_r1479230252
##########
flink-tests/src/test/java/org/apache/flink/test/checkpointing/AutoRescalingITCase.java:
##########
@@ -427,7 +427,8 @@ public void testCheckpointRescalingWithKeyedAndNonPartitionedState() throws Exce
restClusterClient.updateJobResourceRequirements(jobID, builder.build()).join();
- waitForRunningTasks(restClusterClient, jobID, parallelism2);
+ // Source is parallelism, the flatMapper & Sink is parallelism2
+ waitForRunningTasks(restClusterClient, jobID, parallelism + parallelism2);
Review Comment:
> Can't we also reduce the cooldown phase to make the test complete faster similarly to what you did in [1996fanrui@ffd713e](https://github.com/1996fanrui/flink/commit/ffd713e24d37db2c103e4cd4361d0cd916d0d2f6)?
I have already reduced it in this PR : https://github.com/apache/flink/pull/24246, that's why I didn't do it here.
> I tried it locally but it didn't help, though. 🤔
Sorry, I don't understand. Do you mean the test duration is still long or test still fails?
- Before reducing the cooldown time, the duration of most of tests more than 30s.
- After reducing the cooldown time, the duration of most of tests less than 10s.
--
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] [FLINK-34336][test] Fix the bug that AutoRescalingITCase may hang sometimes [flink]
Posted by "1996fanrui (via GitHub)" <gi...@apache.org>.
1996fanrui commented on PR #24248:
URL: https://github.com/apache/flink/pull/24248#issuecomment-1933278564
https://github.com/apache/flink/pull/24246 is merged so far, I guess the CI of master branch may be stuck. This PR can fix it if CI is stuck on `waitForRunningTasks` of `AutoRescalingITCase`.
I start vacation since this afternoon, feel free to merge it if the change make sense.
BTW, I will check the github email every day on my vacation, so I still can address the comments.
--
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] [FLINK-34336][test] Fix the bug that AutoRescalingITCase may hang sometimes [flink]
Posted by "1996fanrui (via GitHub)" <gi...@apache.org>.
1996fanrui merged PR #24248:
URL: https://github.com/apache/flink/pull/24248
--
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