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

Close first engine instance before creating second #1457

Merged
merged 2 commits into from
Oct 28, 2021

Conversation

andrross
Copy link
Member

@andrross andrross commented Oct 27, 2021

Description

This fixes a test that appears to fail for certain test seeds. The following is a test run that will fail without this fix:

  2> REPRODUCE WITH: ./gradlew ':server:test' --tests "org.opensearch.index.engine.InternalEngineTests.testEngineCreationWithCustomTranslogDeletePolicy" -Dtests.seed=E3E5AAD95ABD299B -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=vi -Dtests.timezone=Europe/Astrakhan -Druntime.java=11
  2> java.io.IOException: could not remove the following files (in the order of attempts):
       /home/ubuntu/OpenSearch/server/build/testrun/test/temp/org.opensearch.index.engine.InternalEngineTests_E3E5AAD95ABD299B-001/translog-primary-001/translog-2.tlog: java.io.IOException: access denied: /home/ubuntu/OpenSearch/server/build/testrun/test/temp/org.opensearch.index.engine.InternalEngineTests_E3E5AAD95ABD299B-001/translog-primary-001/translog-2.tlog
       /home/ubuntu/OpenSearch/server/build/testrun/test/temp/org.opensearch.index.engine.InternalEngineTests_E3E5AAD95ABD299B-001/translog-primary-001/translog.ckp: java.io.IOException: access denied: /home/ubuntu/OpenSearch/server/build/testrun/test/temp/org.opensearch.index.engine.InternalEngineTests_E3E5AAD95ABD299B-001/translog-primary-001/translog.ckp
       /home/ubuntu/OpenSearch/server/build/testrun/test/temp/org.opensearch.index.engine.InternalEngineTests_E3E5AAD95ABD299B-001/translog-primary-001/translog-1.tlog: java.io.IOException: access denied: /home/ubuntu/OpenSearch/server/build/testrun/test/temp/org.opensearch.index.engine.InternalEngineTests_E3E5AAD95ABD299B-001/translog-primary-001/translog-1.tlog
       /home/ubuntu/OpenSearch/server/build/testrun/test/temp/org.opensearch.index.engine.InternalEngineTests_E3E5AAD95ABD299B-001/translog-primary-001: java.nio.file.DirectoryNotEmptyException: /home/ubuntu/OpenSearch/server/build/testrun/test/temp/org.opensearch.index.engine.InternalEngineTests_E3E5AAD95ABD299B-001/translog-primary-001
        at __randomizedtesting.SeedInfo.seed([E3E5AAD95ABD299B:4F3E3027C1E8F03B]:0)
        at org.opensearch.core.internal.io.IOUtils.rm(IOUtils.java:220)
        at org.opensearch.index.translog.Translog.createEmptyTranslog(Translog.java:2027)
        at org.opensearch.index.translog.Translog.createEmptyTranslog(Translog.java:1999)
        at org.opensearch.index.translog.Translog.createEmptyTranslog(Translog.java:1989)
        at org.opensearch.index.engine.EngineTestCase.createEngine(EngineTestCase.java:538)
        at org.opensearch.index.engine.EngineTestCase.createEngine(EngineTestCase.java:527)
        at org.opensearch.index.engine.InternalEngineTests.testEngineCreationWithCustomTranslogDeletePolicy(InternalEngineTests.java:3838)

Issues Resolved

None

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

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.

When creating the second instance of an InternalEngine using the same
translog config of the default InternalEngine instance, the second
instance will attempt to delete all the existing translog files. I found
a deterministic test failure when running with the seed
`E3E6AAD95ABD299B`. This change to use a distinct location for the
translog seems appropriate in any case, though I can't explain why this
test doesn't deterministically fail for all test seeds.

Signed-off-by: Andrew Ross <andrross@amazon.com>
@opensearch-ci-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Wrapper Validation success aa7f004

@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed aa7f004

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Precommit success aa7f004

As opposed to creating a second engine instance with a different
translog location, just close the first one before creating the second.

Signed-off-by: Andrew Ross <andrross@amazon.com>
@andrross andrross changed the title Use different translog location for second engine instance Close first engine instance before creating second Oct 28, 2021
@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed e2eb1b0

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Wrapper Validation success e2eb1b0

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Precommit success e2eb1b0

Copy link
Contributor

@adnapibar adnapibar left a comment

Choose a reason for hiding this comment

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

Thanks :)

@adnapibar
Copy link
Contributor

start gradle check

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure e2eb1b0
Log 845

Reports 845

@adnapibar
Copy link
Contributor

* What went wrong:
Execution failed for task ':qa:repository-multi-version:v1.2.0#Step1OldClusterTest'.
> `node{:qa:repository-multi-version:v1.2.0-old-0}` failed to wait for ports files after 120000 MILLISECONDS

Retrying

@adnapibar
Copy link
Contributor

start gradle check

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure e2eb1b0
Log 847

Reports 847

@adnapibar
Copy link
Contributor

Failed test not reproducible

./gradlew ':server:test' --tests "org.opensearch.monitor.fs.FsHealthServiceTests.testFailsHealthOnHungIOBeyondHealthyTimeout" -Dtests.seed=EF63616AF7732FA7 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=zh-Hant-HK -Dtests.timezone=Europe/Bucharest -Druntime.java=17

@adnapibar
Copy link
Contributor

start gradle check

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure e2eb1b0
Log 849

Reports 849

@adnapibar
Copy link
Contributor

start gradle check

1 similar comment
@adnapibar
Copy link
Contributor

start gradle check

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success e2eb1b0
Log 850

Reports 850

@adnapibar adnapibar merged commit 12789f8 into opensearch-project:main Oct 28, 2021
adnapibar added a commit that referenced this pull request Oct 29, 2021
…in EnginePlugin (#1404) (#1424)

* Add extension point for custom TranslogDeletionPolicy in EnginePlugin. (#1404)

This commit adds a method that can be used to provide a custom TranslogDeletionPolicy from within plugins that implement the EnginePlugin interface. This enables plugins to provide a custom deletion policy with the current limitation that only one plugin can override the policy. An exception will be thrown if more than one plugin overrides the policy.

Signed-off-by: Rabi Panda <adnapibar@gmail.com>

* Close first engine instance before creating second (#1457)

When creating the second instance of an InternalEngine using the same translog config of the default InternalEngine instance, the second instance will attempt to delete all the existing translog files. I found
a deterministic test failure when running with the seed `E3E6AAD95ABD299B`. As opposed to creating a second engine instance with a different translog location, just close the first one before creating the second.

Signed-off-by: Andrew Ross <andrross@amazon.com>

Co-authored-by: Andrew Ross <andrross@amazon.com>
@andrross andrross deleted the fix-engine-test branch November 5, 2021 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants