Skip to content

Mitigate testDynamicFiltering* flakiness 2 #18468

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

Conversation

ssheikin
Copy link
Contributor

testDynamicFiltering* reuses global QueryTracker state which can pruneExpiredQueries()
BaseJdbcConnectorTest is huge. Test methods are executed in parallel. There could be more than query.max-history queries between executeWithQueryId and SqlQueryManager.getFullQueryInfo invocations.

Executing testDynamicFiltering* methods even in almost single threaded environment (executing these methods after all other in the test class) eliminates this issue.

    private static final int LAST = 1000;
    @Test(priority = LAST)

Retrying assertions mitigates this issue as well.
So adding @flaky to make CI even more solid.

Description

This is an alternative to #18323

Additional context and related issues

Release notes

(x) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text:

# Section
* Fix some things. ({issue}`issuenumber`)

@ssheikin
Copy link
Contributor Author

@kokosing @wendigo @hashhar Please approve.

@ssheikin ssheikin force-pushed the ssheikin/38/trino/flaky_testDynamicFiltering_2 branch from fec9e86 to 30ff958 Compare August 1, 2023 08:27
testDynamicFiltering* reuses global QueryTracker state which can
pruneExpiredQueries()
BaseJdbcConnectorTest is huge. Test methods are executed in parallel.
There could be more than `query.max-history` queries between
`executeWithQueryId` and `SqlQueryManager.getFullQueryInfo` invocations.

Executing `testDynamicFiltering*` methods even in almost single threaded
environment (executing these methods after all other in the test class)
eliminates this issue.
```
    private static final int LAST = 1000;
    @test(priority = LAST)
```

Retrying assertions mitigates this issue as well.
So adding @flaky to make CI even more solid.
@ssheikin
Copy link
Contributor Author

ssheikin commented Aug 2, 2023

Closing this PR in favour of #18323
which does not have TestNG priority magic.

@ssheikin ssheikin closed this Aug 2, 2023
@ssheikin ssheikin deleted the ssheikin/38/trino/flaky_testDynamicFiltering_2 branch April 12, 2024 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

1 participant