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

Fix flaky test InternalEngineTests.testLastRefreshCheckpoint #9365

Merged
merged 1 commit into from
Aug 16, 2023

Conversation

ankitkala
Copy link
Member

Description

We saw an assertion failure here where currentOngoingRefreshCheckpoint was -1 and lastRefreshedCheckpoint was 0. This was not reproducible after 50K test iterations.

com.carrotsearch.randomizedtesting.UncaughtExceptionError: Captured an uncaught exception in thread: Thread[id=649, name=Thread-197, state=RUNNABLE, group=TGRP-InternalEngineTests]
	at __randomizedtesting.SeedInfo.seed([B448FCE4648CA4EB:7A3CB282E14976E1]:0)
Caused by: java.lang.AssertionError: 
Expected: a value equal to or greater than <0L>
     but: <-1L> was less than <0L>
	at __randomizedtesting.SeedInfo.seed([B448FCE4648CA4EB]:0)
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:18)
	at org.junit.Assert.assertThat(Assert.java:964)
	at org.junit.Assert.assertThat(Assert.java:930)
	at org.opensearch.index.engine.InternalEngineTests.lambda$testLastRefreshCheckpoint$84(InternalEngineTests.java:6643)

Within the LastRefreshedCheckpointListener(here) we always update lastRefreshedCheckpoint based on currentOngoingRefreshCheckpoint so this above failure condition can never occur. Except we're also invoking the updateRefreshedCheckpoint from refresh which isn't required IMO. Ideal fix would be to remove that but that is more intrusive and there isn't enough context on why it was added in the first place.
Thus the easier fix in this PR is to ensure that we also update pendingCheckpoint whenever we're updating refreshedCheckpoint to ensure consistency.

Related Issues

Resolves #9029

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@opensearch-trigger-bot
Copy link
Contributor

Compatibility status:

Checks if related components are compatible with change 0a7eade

Incompatible components

Incompatible components: [https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/security-analytics.git]

Skipped components

Compatible components

Compatible components: [https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/performance-analyzer.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git]

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.index.ShardIndexingPressureIT.testShardIndexingPressureTrackingDuringBulkWrites

@codecov
Copy link

codecov bot commented Aug 15, 2023

Codecov Report

Merging #9365 (a352a42) into main (9dca96d) will decrease coverage by 0.01%.
Report is 7 commits behind head on main.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##               main    #9365      +/-   ##
============================================
- Coverage     71.15%   71.15%   -0.01%     
- Complexity    57452    57463      +11     
============================================
  Files          4777     4777              
  Lines        270705   270706       +1     
  Branches      39565    39565              
============================================
- Hits         192629   192619      -10     
- Misses        61909    61923      +14     
+ Partials      16167    16164       -3     
Files Changed Coverage Δ
...va/org/opensearch/index/engine/InternalEngine.java 74.80% <100.00%> (+0.40%) ⬆️

... and 466 files with indirect coverage changes

@mch2 mch2 added the backport 2.x Backport to 2.x branch label Aug 16, 2023
@mch2 mch2 merged commit f971e5b into opensearch-project:main Aug 16, 2023
opensearch-trigger-bot bot pushed a commit that referenced this pull request Aug 16, 2023
Signed-off-by: Ankit Kala <[email protected]>
(cherry picked from commit f971e5b)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
kotwanikunal pushed a commit that referenced this pull request Aug 16, 2023
…9395)

(cherry picked from commit f971e5b)

Signed-off-by: Ankit Kala <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
austintlee pushed a commit to austintlee/OpenSearch that referenced this pull request Aug 25, 2023
kkmr pushed a commit to kkmr/OpenSearch that referenced this pull request Aug 28, 2023
kaushalmahi12 pushed a commit to kaushalmahi12/OpenSearch that referenced this pull request Sep 12, 2023
brusic pushed a commit to brusic/OpenSearch that referenced this pull request Sep 25, 2023
shiv0408 pushed a commit to Gaurav614/OpenSearch that referenced this pull request Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Backport to 2.x branch skip-changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] org.opensearch.index.engine.InternalEngineTests.testLastRefreshCheckpoint is flaky
2 participants