Skip to content

Remove readiness probe in favor of startup probe - #185

Open
nadav-govari wants to merge 1 commit into
mainfrom
nadav/readiness
Open

Remove readiness probe in favor of startup probe#185
nadav-govari wants to merge 1 commit into
mainfrom
nadav/readiness

Conversation

@nadav-govari

Copy link
Copy Markdown
Contributor

We decided to remove the readiness probe in quickwit-oss/quickwit#6673 and use a startup probe instead. In that PR, the startup endpoint is aliased to the readiness endpoint for backwards compatibility. This removes the readiness probe itself, and modifies the startup probe.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 256f136609

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +150 to 153
path: /health/readyz
port: rest
failureThreshold: 12
periodSeconds: 5

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Raise the startup probe budget for readyz checks

When /health/readyz takes longer than 60 seconds to return success (for example during a slow metastore/object-store dependency recovery or a cold cluster restart), this new startupProbe kills the container after 12 failures at 5s intervals; the old readinessProbe would simply keep the pod out of endpoints until the dependency recovered. Since the same readyz startupProbe defaults are repeated for the other Quickwit components, a transient >60s readiness delay can turn upgrades into CrashLoopBackOff instead of delayed readiness; please give the startup probe a much larger failureThreshold/period or keep readiness semantics separate.

Useful? React with 👍 / 👎.

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.

1 participant