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/01/04 06:36:14 UTC
[PR] [hotfix][autoscaler] Fix the StandaloneAutoscalerExecutorTest.testScaling fails occasionally due to race condition [flink-kubernetes-operator]
1996fanrui opened a new pull request, #746:
URL: https://github.com/apache/flink-kubernetes-operator/pull/746
Sorry for this fix!
After #744 is merged, I rebased it. During developing other features, I found `StandaloneAutoscalerExecutorTest.testScaling fails occasionally due to race condition`.
Here is my fork repo, it fails with :
https://github.com/1996fanrui/flink-kubernetes-operator/actions/runs/7405774151/job/20149240482#step:5:22232
```
Error: Failures:
Error: StandaloneAutoscalerExecutorTest.testScaling:93
Actual and expected should have same size but actual size is:
1
while expected size is:
2
Actual was:
[org.apache.flink.autoscaler.event.TestingEventCollector$Event@6b580b88,
org.apache.flink.autoscaler.event.TestingEventCollector$Event@6d91790b]
Expected was:
[9fe5243cd23e0727710fcd0b4d423970, 9eec17388e32cb02ec205ac9f638e2fe]
```
From this log, we know the reason is race condition. The Actual was has 2 events, but when we check the actual size, it was one.
## Why it fails occasionally?
https://github.com/apache/flink-kubernetes-operator/pull/744 supports multiple threads for autoscaler standalone, and the scaling control loop thread doesn't wait for the actual scaling of all jobs.
So I introduced the `CountDownLatch` to let test wait all scaling are finished. But the `eventHandler.handleEvent` is called inside of the `scalingSingleJob`. We should call `countDownLatch.countDown();` after `scalingSingleJob`.
--
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] [hotfix][autoscaler] Fix the StandaloneAutoscalerExecutorTest.testScaling fails occasionally due to race condition [flink-kubernetes-operator]
Posted by "1996fanrui (via GitHub)" <gi...@apache.org>.
1996fanrui merged PR #746:
URL: https://github.com/apache/flink-kubernetes-operator/pull/746
--
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] [hotfix][autoscaler] Fix the StandaloneAutoscalerExecutorTest.testScaling fails occasionally due to race condition [flink-kubernetes-operator]
Posted by "1996fanrui (via GitHub)" <gi...@apache.org>.
1996fanrui commented on PR #746:
URL: https://github.com/apache/flink-kubernetes-operator/pull/746#issuecomment-1876570597
> And If you have a bit of concern on the stability of the fix, you could use @RepeatedTest annotations from repeated test cases locally to test whether this fix is stable.
Thanks @RocMarshal for your review and suggestion! After this PR, I have already used `@RepeatedTest ` to check this test. It works well for now.
--
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] [hotfix][autoscaler] Fix the StandaloneAutoscalerExecutorTest.testScaling fails occasionally due to race condition [flink-kubernetes-operator]
Posted by "RocMarshal (via GitHub)" <gi...@apache.org>.
RocMarshal commented on code in PR #746:
URL: https://github.com/apache/flink-kubernetes-operator/pull/746#discussion_r1441407436
##########
flink-autoscaler-standalone/src/test/java/org/apache/flink/autoscaler/standalone/StandaloneAutoscalerExecutorTest.java:
##########
@@ -62,30 +62,38 @@ void testScaling(boolean throwExceptionWhileScale) throws Exception {
Collections.synchronizedList(new ArrayList<JobAutoScalerContext<JobID>>());
var eventCollector = new TestingEventCollector<JobID, JobAutoScalerContext<JobID>>();
- final Configuration conf = new Configuration();
+ final var conf = new Configuration();
conf.set(CONTROL_LOOP_PARALLELISM, 1);
var countDownLatch = new CountDownLatch(jobList.size());
+
+ final var jobAutoScaler =
+ new JobAutoScaler<JobID, JobAutoScalerContext<JobID>>() {
+ @Override
+ public void scale(JobAutoScalerContext<JobID> context) {
+ actualScaleContexts.add(context);
+ if (exceptionKeys.contains(context.getJobKey())) {
+ throw new RuntimeException("Excepted exception.");
+ }
+ }
+
+ @Override
+ public void cleanup(JobID jobKey) {
+ fail("Should be called.");
+ }
+ };
+
try (var autoscalerExecutor =
new StandaloneAutoscalerExecutor<>(
- conf,
- () -> jobList,
- eventCollector,
- new JobAutoScaler<>() {
- @Override
- public void scale(JobAutoScalerContext<JobID> context) {
- actualScaleContexts.add(context);
- countDownLatch.countDown();
- if (exceptionKeys.contains(context.getJobKey())) {
- throw new RuntimeException("Excepted exception.");
- }
- }
+ conf, () -> jobList, eventCollector, jobAutoScaler) {
+ @Override
+ protected void scalingSingleJob(JobAutoScalerContext<JobID> jobContext) {
+ super.scalingSingleJob(jobContext);
+ countDownLatch.countDown();
+ }
+ }) {
- @Override
- public void cleanup(JobID jobKey) {
- fail("Should be called.");
- }
- })) {
autoscalerExecutor.scaling();
+ // Wait for all scalings are finished.
Review Comment:
```suggestion
// Wait for all scalings to go finished.
```
--
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