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

Add pod lifecycle #104

Merged
merged 5 commits into from
Oct 14, 2024
Merged

Add pod lifecycle #104

merged 5 commits into from
Oct 14, 2024

Conversation

fredsig
Copy link
Contributor

@fredsig fredsig commented Oct 5, 2024

Introduced pod lifecycle. When using the AWS Load Balancer Controller, deployments of pods trigger 5xx gateway errors when doing config changes, since the target group registration/deregistration of pods is out of sync with kubernetes service network orchestration (when an indexer pod gets terminated with a SIGTERM, it's done immediately, not giving enough time to drain connections from the ALB's target group, triggering 5xx errors to the client). Introducing a sleep time on indexers can help avoid this during pod termination (as a pre-stop action).

Related: https://medium.com/@imprintpayments/mastering-the-challenges-of-using-alb-ingress-in-kubernetes-8c28a8f826c5

@fredsig
Copy link
Contributor Author

fredsig commented Oct 14, 2024

Thanks for unblocking the CI @guilload. I can see it failed lint as I didn't bump the version of this chart. If you are ok with this PR and it gets approved, what value you recommend to be changed? Let me know if there's something I can do. Thank you.

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.

LGTM. Thanks for linking the blog post. See also this ALB controller issue. Can you please bump the chart's version (patch) so we can merge this PR?

@fredsig
Copy link
Contributor Author

fredsig commented Oct 14, 2024

LGTM. Thanks for linking the blog post. See also this ALB controller issue. Can you please bump the chart's version (patch) so we can merge this PR?

Yep, this is exactly the original issue. I've bumped the Chart version to 0.7.2. Thank you!

@guilload guilload merged commit 1f95947 into quickwit-oss:main Oct 14, 2024
1 check passed
@fredsig fredsig deleted the add-pod-lifecycle branch October 15, 2024 08:24
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