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

Issue/5474 merge pipeline finalization #5475

Merged
merged 3 commits into from
Oct 10, 2024

Conversation

fulmicoton
Copy link
Contributor

@fulmicoton fulmicoton commented Oct 4, 2024

Before this PR, when the last indexing pipeline
is shutdown, the merge pipeline associated to an index
would be abruptly shutdown too.

This PR makes two changes to this behavior.
First, we wait for ongoing or pending merges to
be executed before shutting down the pipeline.

Second, merge policy get the opportunity to offer
a list of extra merges to run.

This functionality is introduced to help users
who migrated from elasticsearch and rely on a daily
indexes.

Closes #5474

Copy link

github-actions bot commented Oct 4, 2024

On SSD:

Average search latency is 0.995x that of the reference (lower is better).
Ref run id: 3700, ref commit: 2dcc696
Link

On GCS:

Average search latency is 1.06x that of the reference (lower is better).
Ref run id: 3701, ref commit: 2dcc696
Link

@fulmicoton fulmicoton force-pushed the issue/5474-merge-pipeline-finalization branch 13 times, most recently from 4c16e96 to 1bed4b1 Compare October 9, 2024 07:20
@fulmicoton fulmicoton marked this pull request as ready for review October 9, 2024 07:22
@fulmicoton fulmicoton force-pushed the issue/5474-merge-pipeline-finalization branch 7 times, most recently from 9a50f0f to 2dd11bd Compare October 9, 2024 09:24
@fulmicoton fulmicoton force-pushed the issue/5474-merge-pipeline-finalization branch from 2dd11bd to ff62bc5 Compare October 9, 2024 09:25
Before this PR, when the last indexing pipeline
is shutdown, the merge pipeline associated to an index
would be abruptly shutdown too.

This PR makes two changes to this behavior.
First, we wait for ongoing or pending merges to
be executed before shutting down the pipeline.

Second, merge policy get the opportunity to offer
a list of extra merges to run.

This functionality is introduced to help users
who migrated from elasticsearch and rely on a daily
indexes.

Closes #5474
@fulmicoton fulmicoton force-pushed the issue/5474-merge-pipeline-finalization branch from ff62bc5 to d0ce172 Compare October 9, 2024 10:32
Copy link
Member

@guilload guilload left a comment

Choose a reason for hiding this comment

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

The abundant comments made the PR super easy to review. Thanks!

quickwit/quickwit-indexing/src/actors/merge_pipeline.rs Outdated Show resolved Hide resolved
quickwit/quickwit-indexing/src/actors/merge_pipeline.rs Outdated Show resolved Hide resolved
Co-authored-by: Adrien Guillo <adrien@quickwit.io>
@fulmicoton fulmicoton force-pushed the issue/5474-merge-pipeline-finalization branch from 3c67952 to 7966e7e Compare October 10, 2024 05:53
@fulmicoton fulmicoton force-pushed the issue/5474-merge-pipeline-finalization branch from 7966e7e to 129f93f Compare October 10, 2024 06:48
@fulmicoton fulmicoton merged commit 47c1bf3 into main Oct 10, 2024
4 of 5 checks passed
@fulmicoton fulmicoton deleted the issue/5474-merge-pipeline-finalization branch October 10, 2024 08:06
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.

Merge pipeline stops abruptly after the last indexing pipeline terminates
2 participants