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

Fix actor shutdown to await merges #4741

Merged
merged 2 commits into from
Mar 22, 2024
Merged

Fix actor shutdown to await merges #4741

merged 2 commits into from
Mar 22, 2024

Conversation

rdettai
Copy link
Collaborator

@rdettai rdettai commented Mar 14, 2024 •
edited by fmassot
Loading

Description

Improve the actor shutdown on AWS Lambda runs so that merges go through. Fix #4612

How was this PR tested?

I ran the Quickwit Lambda stack with the mock data generator for a few weeks with both:

  • a 100k element per batch generator
  • a 10 element per batch generator
    Both ran without any error and all merges were properly performed.

This test also led to opening #4734 as Cloudwatch logs costs where excessively high.

@rdettai rdettai self-assigned this Mar 14, 2024
@rdettai rdettai requested a review from fmassot March 14, 2024 19:00
@rdettai rdettai linked an issue Mar 14, 2024 that may be closed by this pull request
Make sure long merges are not cancelled on the Lambda indexer #4612
Closed
@guilload
Copy link
Member

Can you revert the changes in code-generated files? I wonder how this is not breaking CI.

@rdettai rdettai force-pushed the improve-lambda-merges branch from a6d1f15 to 950e2cb Compare March 15, 2024 10:03
@rdettai
Copy link
Collaborator Author

rdettai commented Mar 15, 2024

Done!

@rdettai rdettai requested a review from trinity-1686a March 21, 2024 09:14
trinity-1686a
trinity-1686a reviewed Mar 21, 2024
View reviewed changes
Copy link
Contributor

@trinity-1686a trinity-1686a left a comment

Choose a reason for hiding this comment

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

looks sensible, but i lack knowledge about this part of the codebase (and in particular cdk stuff) to be very confident in my assessment

distribution/lambda/cdk/stacks/services/quickwit_service.py Show resolved Hide resolved
quickwit/quickwit-lambda/src/environment.rs Show resolved Hide resolved
quickwit/quickwit-lambda/src/searcher/handler.rs Outdated Show resolved Hide resolved
quickwit/quickwit-lambda/src/searcher/search.rs Outdated Show resolved Hide resolved
quickwit/quickwit-lambda/src/indexer/ingest/mod.rs Outdated Show resolved Hide resolved
@rdettai rdettai force-pushed the improve-lambda-merges branch from 950e2cb to 0dce794 Compare March 21, 2024 15:59
@rdettai rdettai force-pushed the improve-lambda-merges branch from 4f9cde0 to a3de045 Compare March 22, 2024 13:56
@rdettai rdettai enabled auto-merge (squash) March 22, 2024 13:56
@rdettai rdettai disabled auto-merge March 22, 2024 13:58
@rdettai rdettai enabled auto-merge (squash) March 22, 2024 13:58
@rdettai rdettai merged commit dc5453e into main Mar 22, 2024
4 checks passed
@rdettai rdettai deleted the improve-lambda-merges branch March 22, 2024 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@trinity-1686a trinity-1686a trinity-1686a approved these changes

@fmassot fmassot Awaiting requested review from fmassot

Assignees

@rdettai rdettai

Labels
aws-lambda
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Make sure long merges are not cancelled on the Lambda indexer
3 participants
@rdettai @guilload @trinity-1686a