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.

Sign up for GitHub

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

Jump to bottom

Trigger early commit when shutting down ingest V2 #5390

Closed
wants to merge 4 commits into from
Closed

Trigger early commit when shutting down ingest V2 #5390

wants to merge 4 commits into from

Conversation

rdettai
Copy link
Collaborator

@rdettai rdettai commented Sep 5, 2024 •
edited
Loading

Description

Currently the shutdown waits for the shards to be indexed without triggering an early commit, so the shutdown can take as long as the longest commit timeout.

How was this PR tested?

Integ tests

@rdettai rdettai self-assigned this Sep 5, 2024
@rdettai rdettai added the bug Something isn't working label Sep 5, 2024
@rdettai rdettai changed the title Trigger early commit timeout when shutting down ingest V2 Trigger early commit when shutting down ingest V2 Sep 5, 2024
@rdettai
Copy link
Collaborator Author

rdettai commented Sep 5, 2024

Gravitates around #5068

@rdettai rdettai requested a review from guilload September 5, 2024 15:45
@rdettai rdettai force-pushed the shutdown-early-commit-timeout branch from f1c4216 to d791662 Compare September 5, 2024 16:07
@rdettai rdettai marked this pull request as ready for review September 5, 2024 16:12
@rdettai rdettai force-pushed the shutdown-early-commit-timeout branch from d791662 to b04f0de Compare September 5, 2024 16:13
@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 5, 2024 •
edited
Loading

On SSD:

Average search latency is 0.993x that of the reference (lower is better).
Ref run id: 3290, ref commit: 6768d01
Link

On GCS:

Average search latency is 0.936x that of the reference (lower is better).
Ref run id: 3291, ref commit: 6768d01
Link

@rdettai rdettai force-pushed the shutdown-early-commit-timeout branch from f8137d3 to c1aa133 Compare September 9, 2024 10:09
@rdettai rdettai force-pushed the shutdown-early-commit-timeout branch from 23b14cc to e939ff2 Compare September 9, 2024 15:14
@rdettai rdettai mentioned this pull request Sep 10, 2024
Closed
@rdettai
Copy link
Collaborator Author

rdettai commented Sep 12, 2024 •
edited
Loading

As discussed internally, this new mechanism adds too much complexity. We will favor simpler workarounds for now:

  • increase termination grace periods in Helm charts and ECS configs
  • add a listener for second ctrl-c when running nodes in experimentation/tutorial contexts Enable force shutdown with 2nd Ctrl+C #5414
  • document shutdown behavior

@rdettai rdettai closed this Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@guilload guilload Awaiting requested review from guilload

Assignees

@rdettai rdettai

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@rdettai