Thread.sleep() method is replaced with Awaitility

[shreyanshR7]

#7154@nastra

[nk1506]

IMHO, we should use https://github.com/apache/iceberg/blob/master/api/src/test/java/org/apache/iceberg/TestHelpers.java#L57
Or create a utility with parameter as duration where callable is always returning true.

[shreyanshR7]

Oh i see, the code uses a while loop checks the current time until the condition is met.But its asked to replace Thread.sleep method with awaitility, should i implement your suggestion as it don't uses awaitility.@nk1506

[nk1506]

I think idea here is to replace all the thread usage with Awaitility. But we need to understand why the thread is sleeping first.

1. If there is intentional sleep to add some delay. It should be handled with corresponding poll-delay and timestamp diff check.
2. If there is a sleep for another process to be finished, it should check for the corresponding job.
   Make sure pollDelay , atLeast, atMost are being configured correctly.

[shreyanshR7]

Thanks for the suggestion @nk1506 ,I'll update that

[shreyanshR7]

I've made the changes@nk1506

[nastra]

The goal of #7154 is to convert Thread.sleep usages to Awaitility where it makes sense. We don't want to blindly just replace all Thread.sleep usage. We need to understand why the sleep is used in the first place.

Typically, Thread.sleep is used to wait for an async condition to happen. Rather than waiting the max sleep time, we want to wait less than that, by checking a success condition that indicates we can proceed.

Here is a good example where the test was made more stable by the use of Awaitility:

iceberg/core/src/test/java/org/apache/iceberg/actions/TestCommitService.java

Lines 99 to 102 in cff2ff9

	Awaitility.await()
	.atMost(5, TimeUnit.SECONDS)
	.pollInSameThread()
	.untilAsserted(() -> assertThat(commitService.completedRewritesAllCommitted()).isTrue());

We wait at most 5 seconds until the given assertion is met.

For this PR that means a good starting point might be

iceberg/flink/v1.17/flink/src/test/java/org/apache/iceberg/flink/source/TestStreamingMonitorFunction.java

Line 153 in f9f85e7

Thread.sleep(1000L);

, where there's a 1s sleep and then a check. That's the kind of things we want to replace with Awaitility

[shreyanshR7]

I've made the changes :)@nastra

[nastra]

does it make sense here to switch to awaitility?

[nastra]

does it make sense here to switch to awaitility?

[nastra]

why are these changes here necessary?

[shreyanshR7]

wait let me check....

[shreyanshR7]

I tried to fix the issues :) @nastra

[nastra]

@shreyanshR7 there are still many unrelated changes to files. Can you please fix all of those?

[shreyanshR7]

Let me check, I will try my best to undo these unrelated changes, these changes may be occurred due to adding awaitility method. sorry for inconvenience @nastra

[shreyanshR7]

@nastra I made another pull request addressing the issue with some more clean changes, please consider that.