search

kubernetes-client / java

Official Java client library for kubernetes
http://kubernetes.io/
Apache License 2.0
3.57k stars 1.91k forks source link

[Umbrella Issue] Many tests use Thread.sleep instead of synchronization primitives #1223

Open brendandburns opened 4 years ago

brendandburns commented 4 years ago

Many of our tests simply have a Thread.sleep(duration) when testing multi-threaded code. This is inherently flaky.

Instead we should use synchronization classes (generally Semaphore) to synchronized the tests which is both faster in the general case and not flaky in the extreme case.

See for instance #1218 and #1219

brendandburns commented 4 years ago
yue9944882 commented 4 years ago

@brendandburns thanks for sorting out these potentially flaking tests.. will fix them later!

/assign

sarveshkaushal commented 4 years ago

I would like to help :-)

brendandburns commented 4 years ago

@sarveshkaushal we'd welcome the help. In most cases the Thread.sleep(foo) is to wait for some other thread to complete it's work. Instead, we should create a Semaphore, then acquire() one or more permits for the executions we're waiting for, and then release() them in the Thread, then attempt to acquire() in the test thread to implement the wait.

See an example PR #1219

The reason that Thread.sleep() is flaky is because in a CI/CD system like GitHub actions the machines can be massively overloaded, which leads to much longer delays than you might otherwise see when running tests, so the sleep(..) ends up not being long enough. You can always make the sleep longer, but that makes the tests run longer (and it's still flaky in extreme cases) so using synchronization primitives is (almost) always the right answer.

fejta-bot commented 3 years ago

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta. /lifecycle stale

fejta-bot commented 3 years ago

Stale issues rot after 30d of inactivity. Mark the issue as fresh with /remove-lifecycle rotten. Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta. /lifecycle rotten

brendandburns commented 3 years ago

/lifecycle frozen

Priya103 commented 3 years ago

If still available I would like to contribute

Vishal-thakur-01 commented 2 years ago

well i am new to java and i would like to get in touch with you guys and try to do my best in solving all the issues hope you all will understand my curiosity and get in touch with me and help me to grow and learn.........

Vyom-Yadav commented 2 years ago

@brendandburns I have one doubt, I was working on util/src/test/java/io/kubernetes/client/informer/cache/SharedProcessorTest.java, particularly on: https://github.com/kubernetes-client/java/blob/536f72556899a5a951a9d1d000f4b5073429e017/util/src/test/java/io/kubernetes/client/informer/cache/SharedProcessorTest.java#L27-L60

I can create a semaphore and acquire it, blocking the main thread, but how will I release so that blocked main thread will continue the execution, I looked at many tests, facing similar problem in many of them.

Edit- I cannot access ExecutorService instance variable which would have been useful over here, please suggest.

vinayak912002 commented 2 years ago

what are the things I need to learn in order to start contributiing to this project?

Mohdcode commented 2 years ago

what are the things I need to learn to start contributing to this project?

DSA, API, and Kubernetes or just contribute by documentation correction or making better changes

Vyom-Yadav commented 2 years ago

@brendandburns ping

tmrpavan commented 1 year ago

/assign