Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MINOR: set test global timeout as 10 mins #15065

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

showuon
Copy link
Contributor

@showuon showuon commented Dec 22, 2023

As @stanislavkozlovski found here, there are some tests ran more than 1 ~ 2 hours, which make our CI build meet 8 hours timeout. We should set a timeout for each test as 10 mins using a junit5 feature here. If it exceeds 10m, it doesn't make sense to keep waiting and we should fail them, and investigate/improve them. This way, we can resolve the CI timeout issue.

If it exceeds the timeout, it'll fail with the exception:

Gradle Test Run :storage:test > Gradle Test Executor 21 > TransactionsWithTieredStoreTest > testDelayedFetchIncludesAbortedTransaction(String) > testDelayedFetchIncludesAbortedTransaction(String).quorum=zk FAILED
    java.util.concurrent.TimeoutException: testDelayedFetchIncludesAbortedTransaction(java.lang.String) timed out after 10 seconds
        at org.junit.jupiter.engine.extension.TimeoutExceptionFactory.create(TimeoutExceptionFactory.java:29)
        at org.junit.jupiter.engine.extension.SameThreadTimeoutInvocation.proceed(SameThreadTimeoutInvocation.java:58)
        at org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:156)

However, the stream tests are still using junit4, we can only set timeout for each test suite. I added timeout for KTableKTableLeftJoinTest separately as highlighted here.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@showuon
Copy link
Contributor Author

showuon commented Dec 22, 2023

@dajac @ijuma @jolshan @stanislavkozlovski , call for review.

@jolshan
Copy link
Contributor

jolshan commented Dec 27, 2023

Thanks @showuon for getting started with this. I noticed the first build still ran about 8h and hit an OOM issue. Looks like that will have to be addressed separately (which is ok). (edit, looks like the second build also hit OOM)

I was wondering if we saw any previously passing tests that were just 10 minutes so we hit the timeout. I didn't see any so far.

@dajac
Copy link
Contributor

dajac commented Jan 8, 2024

I like the idea! Have we been able to get a stable build with this?

@ijuma
Copy link
Contributor

ijuma commented Jan 8, 2024

This does actually work consistently? My experience with JUnit 5 timeout annotations has not been good, unfortunately (they often would simply not work at all). I haven't tried this approach though. It would be great if it did work.

@@ -13,3 +13,4 @@
# See the License for the specific language governing permissions and
# limitations under the License.
junit.jupiter.params.displayname.default = "{displayName}.{argumentsWithNames}"
junit.jupiter.execution.timeout.default = 10 m
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this sets a timeout of 10m on the lifecycle methods as well. While I think we should have a timeout on lifecycle methods too, perhaps we can decrease its value to say 5m (or even lesser if that seems apt)? WDYT?

Refer the junit.jupiter.execution.timeout.lifecycle.method.default prop for more information.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also stumbled upon Thread Mode which avoids pitfalls where the test code doesn't interrupt.

@gaurav-narula
Copy link
Contributor

Bumping this thread for more visibility.

Would be nice if we give this a shot since we still run into builds which time out out of the blue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants