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

[BUG] HeapBasedRateTrackerTest are unreliable #2193

Closed
peternied opened this issue Oct 25, 2022 · 1 comment · Fixed by #3934
Closed

[BUG] HeapBasedRateTrackerTest are unreliable #2193

peternied opened this issue Oct 25, 2022 · 1 comment · Fixed by #3934
Assignees
Labels
bug Something isn't working flaky-test Flaky Test issue good first issue These are recommended starting points for newcomers looking to make their first contributions. triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable.

Comments

@peternied
Copy link
Member

peternied commented Oct 25, 2022

What is the bug?
The tests in HeapBasedRateTrackerTest [1] expect behavior based on elapsed time vs System.currentTimeMillis() [2]. During test execution there is no guarantee that Thread.Sleep(...) times are anywhere near expected. This tests are unreliable as has been seen when attempting to onboard MacOS and Windows platforms.

How can one reproduce the bug?
Running the following command on a resource constrained machine many times until the tests fail
./gradlew test --tests org.opensearch.security.auth.limiting.HeapBasedRateTrackerTest

How should this be fixed?
The way time is calculated show be supplied by a TimeProvider that can be mocked during unit tests for reliability

[1] https://github.com/opensearch-project/security/blob/main/src/test/java/org/opensearch/security/auth/limiting/HeapBasedRateTrackerTest.java#L27
[2] https://github.com/opensearch-project/security/blob/main/src/main/java/org/opensearch/security/util/ratetracking/HeapBasedRateTracker.java#L88

@peternied peternied added bug Something isn't working untriaged Require the attention of the repository maintainers and may need to be prioritized labels Oct 25, 2022
peternied added a commit to peternied/security that referenced this issue Oct 25, 2022
Tracked with opensearch-project#2193

Signed-off-by: Peter Nied <petern@amazon.com>
peternied added a commit to peternied/security that referenced this issue Oct 25, 2022
Tracked with opensearch-project#2193

Signed-off-by: Peter Nied <petern@amazon.com>
peternied added a commit that referenced this issue Oct 28, 2022
Add CI for Windows and MacOS platforms

Due to the increase in the number of platforms, I've separated the newer integration tests into their own workflow.  Until retries have been enabled they will automatically pass - but still run and report logs.  As soon as we have full confidence we will allow them to start blocking pull requests from merging.  #2184

Switch the gradle commands to be platform agnostic via the `gradle/gradle-build-action@v2`, dropping the 'clean' step to the build which allows us to reuse the gradle cache - if we see any problems pulling in more recent snapshots we can disable this setting quickly.

Found and fixed an issued with config value replacement via environment variables, long story short Windows and MacOS allow for many more characters that are used in the unix environment variable landscape.  Added new tests to cover these interesting scenarios as well.

Found an encoding issue with user names from config files, still unclear of the source of this issue, be it test setup specific or a problem in the broader OpenSearch ecosystem, disabling the `testSpecialUsernames` until we can dive deeper. #2194

Disabled the HeapBasedRateTrackerTests - it was depending on system timing and was very brittle if the system under test experienced any undo load, created follow up issue #2193

Fixed a test issue in testDlsWithMinDocCountZeroAggregations where there was a random chance for a test failure, easier to find intermittent tests when they are run so often.

OpenSSL has open questions - while it is supported for our Linux JDK11 builds, it seems like a stopgap measure.  I've disabled the tests on windows environment while we determine if we should support OpenSSL at all on Windows JDK11.  #2195

Signed-off-by: Peter Nied <petern@amazon.com>
peternied added a commit that referenced this issue Oct 28, 2022
Add CI for Windows and MacOS platforms

Due to the increase in the number of platforms, I've separated the newer integration tests into their own workflow.  Until retries have been enabled they will automatically pass - but still run and report logs.  As soon as we have full confidence we will allow them to start blocking pull requests from merging.  #2184

Switch the gradle commands to be platform agnostic via the `gradle/gradle-build-action@v2`, dropping the 'clean' step to the build which allows us to reuse the gradle cache - if we see any problems pulling in more recent snapshots we can disable this setting quickly.

Found and fixed an issued with config value replacement via environment variables, long story short Windows and MacOS allow for many more characters that are used in the unix environment variable landscape.  Added new tests to cover these interesting scenarios as well.

Found an encoding issue with user names from config files, still unclear of the source of this issue, be it test setup specific or a problem in the broader OpenSearch ecosystem, disabling the `testSpecialUsernames` until we can dive deeper. #2194

Disabled the HeapBasedRateTrackerTests - it was depending on system timing and was very brittle if the system under test experienced any undo load, created follow up issue #2193

Fixed a test issue in testDlsWithMinDocCountZeroAggregations where there was a random chance for a test failure, easier to find intermittent tests when they are run so often.

OpenSSL has open questions - while it is supported for our Linux JDK11 builds, it seems like a stopgap measure.  I've disabled the tests on windows environment while we determine if we should support OpenSSL at all on Windows JDK11.  #2195

Signed-off-by: Peter Nied <petern@amazon.com>
(cherry picked from commit a57fd0a)
peternied added a commit that referenced this issue Oct 28, 2022
Add CI for Windows and MacOS platforms

Due to the increase in the number of platforms, I've separated the newer integration tests into their own workflow.  Until retries have been enabled they will automatically pass - but still run and report logs.  As soon as we have full confidence we will allow them to start blocking pull requests from merging.  #2184

Switch the gradle commands to be platform agnostic via the `gradle/gradle-build-action@v2`, dropping the 'clean' step to the build which allows us to reuse the gradle cache - if we see any problems pulling in more recent snapshots we can disable this setting quickly.

Found and fixed an issued with config value replacement via environment variables, long story short Windows and MacOS allow for many more characters that are used in the unix environment variable landscape.  Added new tests to cover these interesting scenarios as well.

Found an encoding issue with user names from config files, still unclear of the source of this issue, be it test setup specific or a problem in the broader OpenSearch ecosystem, disabling the `testSpecialUsernames` until we can dive deeper. #2194

Disabled the HeapBasedRateTrackerTests - it was depending on system timing and was very brittle if the system under test experienced any undo load, created follow up issue #2193

Fixed a test issue in testDlsWithMinDocCountZeroAggregations where there was a random chance for a test failure, easier to find intermittent tests when they are run so often.

OpenSSL has open questions - while it is supported for our Linux JDK11 builds, it seems like a stopgap measure.  I've disabled the tests on windows environment while we determine if we should support OpenSSL at all on Windows JDK11.  #2195

Signed-off-by: Peter Nied <petern@amazon.com>
(cherry picked from commit a57fd0a)
@stephen-crawford
Copy link
Contributor

@peternied for this issue did you only see errors during the Mac and Window launch? I am looking at it but have not gotten the test to fail yet.

peternied added a commit that referenced this issue Oct 31, 2022
Add CI for Windows and MacOS platforms

Due to the increase in the number of platforms, I've separated the newer integration tests into their own workflow.  Until retries have been enabled they will automatically pass - but still run and report logs.  As soon as we have full confidence we will allow them to start blocking pull requests from merging.  #2184

Switch the gradle commands to be platform agnostic via the `gradle/gradle-build-action@v2`, dropping the 'clean' step to the build which allows us to reuse the gradle cache - if we see any problems pulling in more recent snapshots we can disable this setting quickly.

Found and fixed an issued with config value replacement via environment variables, long story short Windows and MacOS allow for many more characters that are used in the unix environment variable landscape.  Added new tests to cover these interesting scenarios as well.

Found an encoding issue with user names from config files, still unclear of the source of this issue, be it test setup specific or a problem in the broader OpenSearch ecosystem, disabling the `testSpecialUsernames` until we can dive deeper. #2194

Disabled the HeapBasedRateTrackerTests - it was depending on system timing and was very brittle if the system under test experienced any undo load, created follow up issue #2193

Fixed a test issue in testDlsWithMinDocCountZeroAggregations where there was a random chance for a test failure, easier to find intermittent tests when they are run so often.

OpenSSL has open questions - while it is supported for our Linux JDK11 builds, it seems like a stopgap measure.  I've disabled the tests on windows environment while we determine if we should support OpenSSL at all on Windows JDK11.  #2195

Signed-off-by: Peter Nied <petern@amazon.com>
(cherry picked from commit a57fd0a)
@cwperks cwperks added good first issue These are recommended starting points for newcomers looking to make their first contributions. sprint backlog and removed untriaged Require the attention of the repository maintainers and may need to be prioritized sprint backlog labels Oct 31, 2022
@peternied peternied added the triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable. label Oct 31, 2022
stephen-crawford pushed a commit to stephen-crawford/security that referenced this issue Nov 10, 2022
…nsearch-project#2205)

Add CI for Windows and MacOS platforms

Due to the increase in the number of platforms, I've separated the newer integration tests into their own workflow.  Until retries have been enabled they will automatically pass - but still run and report logs.  As soon as we have full confidence we will allow them to start blocking pull requests from merging.  opensearch-project#2184

Switch the gradle commands to be platform agnostic via the `gradle/gradle-build-action@v2`, dropping the 'clean' step to the build which allows us to reuse the gradle cache - if we see any problems pulling in more recent snapshots we can disable this setting quickly.

Found and fixed an issued with config value replacement via environment variables, long story short Windows and MacOS allow for many more characters that are used in the unix environment variable landscape.  Added new tests to cover these interesting scenarios as well.

Found an encoding issue with user names from config files, still unclear of the source of this issue, be it test setup specific or a problem in the broader OpenSearch ecosystem, disabling the `testSpecialUsernames` until we can dive deeper. opensearch-project#2194

Disabled the HeapBasedRateTrackerTests - it was depending on system timing and was very brittle if the system under test experienced any undo load, created follow up issue opensearch-project#2193

Fixed a test issue in testDlsWithMinDocCountZeroAggregations where there was a random chance for a test failure, easier to find intermittent tests when they are run so often.

OpenSSL has open questions - while it is supported for our Linux JDK11 builds, it seems like a stopgap measure.  I've disabled the tests on windows environment while we determine if we should support OpenSSL at all on Windows JDK11.  opensearch-project#2195

Signed-off-by: Peter Nied <petern@amazon.com>
(cherry picked from commit a57fd0a)
stephen-crawford pushed a commit to stephen-crawford/security that referenced this issue Nov 10, 2022
Add CI for Windows and MacOS platforms

Due to the increase in the number of platforms, I've separated the newer integration tests into their own workflow.  Until retries have been enabled they will automatically pass - but still run and report logs.  As soon as we have full confidence we will allow them to start blocking pull requests from merging.  opensearch-project#2184

Switch the gradle commands to be platform agnostic via the `gradle/gradle-build-action@v2`, dropping the 'clean' step to the build which allows us to reuse the gradle cache - if we see any problems pulling in more recent snapshots we can disable this setting quickly.

Found and fixed an issued with config value replacement via environment variables, long story short Windows and MacOS allow for many more characters that are used in the unix environment variable landscape.  Added new tests to cover these interesting scenarios as well.

Found an encoding issue with user names from config files, still unclear of the source of this issue, be it test setup specific or a problem in the broader OpenSearch ecosystem, disabling the `testSpecialUsernames` until we can dive deeper. opensearch-project#2194

Disabled the HeapBasedRateTrackerTests - it was depending on system timing and was very brittle if the system under test experienced any undo load, created follow up issue opensearch-project#2193

Fixed a test issue in testDlsWithMinDocCountZeroAggregations where there was a random chance for a test failure, easier to find intermittent tests when they are run so often.

OpenSSL has open questions - while it is supported for our Linux JDK11 builds, it seems like a stopgap measure.  I've disabled the tests on windows environment while we determine if we should support OpenSSL at all on Windows JDK11.  opensearch-project#2195

Signed-off-by: Peter Nied <petern@amazon.com>
Signed-off-by: Stephen Crawford <steecraw@amazon.com>
stephen-crawford pushed a commit to stephen-crawford/security that referenced this issue Nov 10, 2022
…nsearch-project#2205)

Add CI for Windows and MacOS platforms

Due to the increase in the number of platforms, I've separated the newer integration tests into their own workflow.  Until retries have been enabled they will automatically pass - but still run and report logs.  As soon as we have full confidence we will allow them to start blocking pull requests from merging.  opensearch-project#2184

Switch the gradle commands to be platform agnostic via the `gradle/gradle-build-action@v2`, dropping the 'clean' step to the build which allows us to reuse the gradle cache - if we see any problems pulling in more recent snapshots we can disable this setting quickly.

Found and fixed an issued with config value replacement via environment variables, long story short Windows and MacOS allow for many more characters that are used in the unix environment variable landscape.  Added new tests to cover these interesting scenarios as well.

Found an encoding issue with user names from config files, still unclear of the source of this issue, be it test setup specific or a problem in the broader OpenSearch ecosystem, disabling the `testSpecialUsernames` until we can dive deeper. opensearch-project#2194

Disabled the HeapBasedRateTrackerTests - it was depending on system timing and was very brittle if the system under test experienced any undo load, created follow up issue opensearch-project#2193

Fixed a test issue in testDlsWithMinDocCountZeroAggregations where there was a random chance for a test failure, easier to find intermittent tests when they are run so often.

OpenSSL has open questions - while it is supported for our Linux JDK11 builds, it seems like a stopgap measure.  I've disabled the tests on windows environment while we determine if we should support OpenSSL at all on Windows JDK11.  opensearch-project#2195

Signed-off-by: Peter Nied <petern@amazon.com>
(cherry picked from commit a57fd0a)
Signed-off-by: Stephen Crawford <steecraw@amazon.com>
stephen-crawford pushed a commit to stephen-crawford/security that referenced this issue Nov 10, 2022
…nsearch-project#2205)

Add CI for Windows and MacOS platforms

Due to the increase in the number of platforms, I've separated the newer integration tests into their own workflow.  Until retries have been enabled they will automatically pass - but still run and report logs.  As soon as we have full confidence we will allow them to start blocking pull requests from merging.  opensearch-project#2184

Switch the gradle commands to be platform agnostic via the `gradle/gradle-build-action@v2`, dropping the 'clean' step to the build which allows us to reuse the gradle cache - if we see any problems pulling in more recent snapshots we can disable this setting quickly.

Found and fixed an issued with config value replacement via environment variables, long story short Windows and MacOS allow for many more characters that are used in the unix environment variable landscape.  Added new tests to cover these interesting scenarios as well.

Found an encoding issue with user names from config files, still unclear of the source of this issue, be it test setup specific or a problem in the broader OpenSearch ecosystem, disabling the `testSpecialUsernames` until we can dive deeper. opensearch-project#2194

Disabled the HeapBasedRateTrackerTests - it was depending on system timing and was very brittle if the system under test experienced any undo load, created follow up issue opensearch-project#2193

Fixed a test issue in testDlsWithMinDocCountZeroAggregations where there was a random chance for a test failure, easier to find intermittent tests when they are run so often.

OpenSSL has open questions - while it is supported for our Linux JDK11 builds, it seems like a stopgap measure.  I've disabled the tests on windows environment while we determine if we should support OpenSSL at all on Windows JDK11.  opensearch-project#2195

Signed-off-by: Peter Nied <petern@amazon.com>
(cherry picked from commit a57fd0a)
Signed-off-by: Stephen Crawford <steecraw@amazon.com>
peternied added a commit to peternied/security that referenced this issue Dec 1, 2022
Add CI for Windows and MacOS platforms

Due to the increase in the number of platforms, I've separated the newer integration tests into their own workflow.  Until retries have been enabled they will automatically pass - but still run and report logs.  As soon as we have full confidence we will allow them to start blocking pull requests from merging.  opensearch-project#2184

Switch the gradle commands to be platform agnostic via the `gradle/gradle-build-action@v2`, dropping the 'clean' step to the build which allows us to reuse the gradle cache - if we see any problems pulling in more recent snapshots we can disable this setting quickly.

Found and fixed an issued with config value replacement via environment variables, long story short Windows and MacOS allow for many more characters that are used in the unix environment variable landscape.  Added new tests to cover these interesting scenarios as well.

Found an encoding issue with user names from config files, still unclear of the source of this issue, be it test setup specific or a problem in the broader OpenSearch ecosystem, disabling the `testSpecialUsernames` until we can dive deeper. opensearch-project#2194

Disabled the HeapBasedRateTrackerTests - it was depending on system timing and was very brittle if the system under test experienced any undo load, created follow up issue opensearch-project#2193

Fixed a test issue in testDlsWithMinDocCountZeroAggregations where there was a random chance for a test failure, easier to find intermittent tests when they are run so often.

OpenSSL has open questions - while it is supported for our Linux JDK11 builds, it seems like a stopgap measure.  I've disabled the tests on windows environment while we determine if we should support OpenSSL at all on Windows JDK11.  opensearch-project#2195

Signed-off-by: Peter Nied <petern@amazon.com>
stephen-crawford pushed a commit to stephen-crawford/security that referenced this issue Dec 2, 2022
Add CI for Windows and MacOS platforms

Due to the increase in the number of platforms, I've separated the newer integration tests into their own workflow.  Until retries have been enabled they will automatically pass - but still run and report logs.  As soon as we have full confidence we will allow them to start blocking pull requests from merging.  opensearch-project#2184

Switch the gradle commands to be platform agnostic via the `gradle/gradle-build-action@v2`, dropping the 'clean' step to the build which allows us to reuse the gradle cache - if we see any problems pulling in more recent snapshots we can disable this setting quickly.

Found and fixed an issued with config value replacement via environment variables, long story short Windows and MacOS allow for many more characters that are used in the unix environment variable landscape.  Added new tests to cover these interesting scenarios as well.

Found an encoding issue with user names from config files, still unclear of the source of this issue, be it test setup specific or a problem in the broader OpenSearch ecosystem, disabling the `testSpecialUsernames` until we can dive deeper. opensearch-project#2194

Disabled the HeapBasedRateTrackerTests - it was depending on system timing and was very brittle if the system under test experienced any undo load, created follow up issue opensearch-project#2193

Fixed a test issue in testDlsWithMinDocCountZeroAggregations where there was a random chance for a test failure, easier to find intermittent tests when they are run so often.

OpenSSL has open questions - while it is supported for our Linux JDK11 builds, it seems like a stopgap measure.  I've disabled the tests on windows environment while we determine if we should support OpenSSL at all on Windows JDK11.  opensearch-project#2195

Signed-off-by: Peter Nied <petern@amazon.com>
wuychn pushed a commit to ochprince/security that referenced this issue Mar 16, 2023
…nsearch-project#2205)

Add CI for Windows and MacOS platforms

Due to the increase in the number of platforms, I've separated the newer integration tests into their own workflow.  Until retries have been enabled they will automatically pass - but still run and report logs.  As soon as we have full confidence we will allow them to start blocking pull requests from merging.  opensearch-project#2184

Switch the gradle commands to be platform agnostic via the `gradle/gradle-build-action@v2`, dropping the 'clean' step to the build which allows us to reuse the gradle cache - if we see any problems pulling in more recent snapshots we can disable this setting quickly.

Found and fixed an issued with config value replacement via environment variables, long story short Windows and MacOS allow for many more characters that are used in the unix environment variable landscape.  Added new tests to cover these interesting scenarios as well.

Found an encoding issue with user names from config files, still unclear of the source of this issue, be it test setup specific or a problem in the broader OpenSearch ecosystem, disabling the `testSpecialUsernames` until we can dive deeper. opensearch-project#2194

Disabled the HeapBasedRateTrackerTests - it was depending on system timing and was very brittle if the system under test experienced any undo load, created follow up issue opensearch-project#2193

Fixed a test issue in testDlsWithMinDocCountZeroAggregations where there was a random chance for a test failure, easier to find intermittent tests when they are run so often.

OpenSSL has open questions - while it is supported for our Linux JDK11 builds, it seems like a stopgap measure.  I've disabled the tests on windows environment while we determine if we should support OpenSSL at all on Windows JDK11.  opensearch-project#2195

Signed-off-by: Peter Nied <petern@amazon.com>
(cherry picked from commit a57fd0a)
@cwperks cwperks added the flaky-test Flaky Test issue label Jan 4, 2024
@peternied peternied self-assigned this Jan 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working flaky-test Flaky Test issue good first issue These are recommended starting points for newcomers looking to make their first contributions. triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants