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

Honor max segment size during only_expunge_deletes #10036

Merged

Conversation

msfroh
Copy link
Collaborator

@msfroh msfroh commented Sep 13, 2023

Description

There are latency-sensitive users for whom the default 20% allowable accumulation of deletes is too high, so they force merge with "only_expunge_deletes".

Unfortunately, this has ignored the max segment size, so despite the word "only" in there, they get the terrible side-effect of producing segments larger than the max segment size. These oversized segments are unlikely to participate in future merges, until they have so many deletes that they could drop back below the max segment size, likely making the whole "too many deletes" problem even worse.

This commit fixes that so that "only_expunge_deletes" ONLY EXPUNGES DELETES without creating oversized segments.

Related Issues

Resolves #7644

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.

@github-actions github-actions bot added enhancement Enhancement or improvement to existing feature or request Indexing Indexing, Bulk Indexing and anything related to indexing labels Sep 13, 2023
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Sep 13, 2023

Compatibility status:

Checks if related components are compatible with change 7d265b4

Incompatible components

Skipped components

Compatible components

Compatible components: [https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/custom-codecs.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/performance-analyzer.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/reporting.git]

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@codecov
Copy link

codecov bot commented Sep 13, 2023

Codecov Report

Merging #10036 (df7f441) into main (70a582f) will increase coverage by 0.12%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##               main   #10036      +/-   ##
============================================
+ Coverage     71.06%   71.18%   +0.12%     
- Complexity    58103    58171      +68     
============================================
  Files          4825     4825              
  Lines        274024   274024              
  Branches      39926    39926              
============================================
+ Hits         194730   195075     +345     
+ Misses        62978    62604     -374     
- Partials      16316    16345      +29     
Files Changed Coverage Δ
.../opensearch/index/OpenSearchTieredMergePolicy.java 100.00% <100.00%> (+3.33%) ⬆️

... and 497 files with indirect coverage changes

Signed-off-by: Michael Froh <froh@amazon.com>
@msfroh msfroh force-pushed the only_expunge_deletes_but_for_real branch from d2a375f to 7d265b4 Compare September 16, 2023 00:32
It's challenging to create a real unit test for this change without
constructing a real Lucene index, but at least this invokes the changed
code.

Signed-off-by: Michael Froh <froh@amazon.com>
@msfroh msfroh force-pushed the only_expunge_deletes_but_for_real branch from 7d265b4 to df7f441 Compare September 16, 2023 00:38
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

Copy link
Member

@andrross andrross left a comment

Choose a reason for hiding this comment

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

I'm convinced by your reasoning, @msfroh. We should document this behavior change, but it appears we don't have documentation for the force_merge API at all. I created an issue here to track that: opensearch-project/documentation-website#5057

@msfroh msfroh merged commit 1dde018 into opensearch-project:main Sep 23, 2023
13 checks passed
sarthakaggarwal97 pushed a commit to sarthakaggarwal97/OpenSearch that referenced this pull request Sep 24, 2023
brusic pushed a commit to brusic/OpenSearch that referenced this pull request Sep 25, 2023
…t#10036)

Signed-off-by: Ivan Brusic <ivan.brusic@flocksafety.com>
vikasvb90 pushed a commit to vikasvb90/OpenSearch that referenced this pull request Oct 10, 2023
@msfroh msfroh added the backport 2.x Backport to 2.x branch label Oct 12, 2023
@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch/backport-2.x
# Create a new branch
git switch --create backport/backport-10036-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 1dde018a1aa9cb606c274a4119348de285b80a65
# Push it to GitHub
git push --set-upstream origin backport/backport-10036-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-10036-to-2.x.

msfroh added a commit to msfroh/OpenSearch that referenced this pull request Oct 12, 2023
…t#10036)

(cherry picked from commit 1dde018)
Signed-off-by: Michael Froh <froh@amazon.com>
msfroh added a commit to msfroh/OpenSearch that referenced this pull request Oct 16, 2023
…t#10036)

(cherry picked from commit 1dde018)
Signed-off-by: Michael Froh <froh@amazon.com>
msfroh added a commit to msfroh/OpenSearch that referenced this pull request Oct 16, 2023
…t#10036)

(cherry picked from commit 1dde018)
Signed-off-by: Michael Froh <froh@amazon.com>
msfroh added a commit to msfroh/OpenSearch that referenced this pull request Oct 18, 2023
…t#10036)

(cherry picked from commit 1dde018)
Signed-off-by: Michael Froh <froh@amazon.com>
msfroh added a commit to msfroh/OpenSearch that referenced this pull request Oct 23, 2023
…t#10036)

(cherry picked from commit 1dde018)
Signed-off-by: Michael Froh <froh@amazon.com>
msfroh added a commit to msfroh/OpenSearch that referenced this pull request Oct 24, 2023
…t#10036)

(cherry picked from commit 1dde018)
Signed-off-by: Michael Froh <froh@amazon.com>
msfroh added a commit that referenced this pull request Oct 24, 2023
(cherry picked from commit 1dde018)
Signed-off-by: Michael Froh <froh@amazon.com>
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 backport-failed enhancement Enhancement or improvement to existing feature or request Indexing Indexing, Bulk Indexing and anything related to indexing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Force merge with only_expunge_deletes should honor max segment size
3 participants