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

Fixing test cases caused by enabling merge-on-refresh by default in Lucene 9.3 #4

Closed

Conversation

reta
Copy link

@reta reta commented Jun 9, 2022

Signed-off-by: Andriy Redko [email protected]

Description

Fixing test cases caused by enabling merge-on-refresh by default in Lucene 9.3. A better fix will be done as part of opensearch-project#3553.

Issues Resolved

Part of opensearch-project#3553

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.

nknize and others added 2 commits June 8, 2022 10:46
Upgrades to latest snapshot of lucene 9.3

Signed-off-by: Nicholas Walter Knize <[email protected]>
logger.warn(
"The {} is enabled but {} is set to 0, merge on flush will not be activated",
IndexSettings.INDEX_MERGE_ON_FLUSH_ENABLED.getKey(),
IndexSettings.INDEX_MERGE_ON_FLUSH_MAX_FULL_FLUSH_MERGE_WAIT_TIME.getKey()
);
}
} else {
iwc.setMaxFullFlushMergeWaitMillis(0);
Copy link
Owner

Choose a reason for hiding this comment

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

I think we want to retain the 500ms default here per the commit message?

Copy link
Owner

Choose a reason for hiding this comment

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

I updated the testcase to 0ms, though, so we can retain the behavior of not exposing aborted docs.

Copy link
Author

Choose a reason for hiding this comment

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

No, we explicitly disable merge on refresh since the setting is set to false

@reta reta closed this Jun 9, 2022
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.

2 participants