-
Notifications
You must be signed in to change notification settings - Fork 50
add preStop hook for autoscaling down #142
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -121,6 +121,46 @@ spec: | |||||||||||||||||||||||||||||||||||||||||||||||||||||
successThreshold: {{ .Values.probes.ingester.config.readinessProbe.successThreshold | default 1 }} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
failureThreshold: {{ .Values.probes.ingester.config.readinessProbe.failureThreshold | default 3 }} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
{{- end }} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
{{- if .Values.autoscaling.ingester.enabled }} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
lifecycle: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
preStop: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
exec: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
command: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
- /bin/sh | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
- -c | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
- | | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Get credentials from environment variables | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
USER_EMAIL="$ZO_ROOT_USER_EMAIL" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
USER_PASSWORD="$ZO_ROOT_USER_PASSWORD" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Create base64 encoded credentials for Authorization header | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
AUTH_HEADER=$(echo -n "${USER_EMAIL}:${USER_PASSWORD}" | base64) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Disable the node first | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
echo "Disabling ingester node..." | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
curl -X PUT "http://localhost:{{ .Values.config.ZO_HTTP_PORT }}/node/enable?value=false" \ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
-H "Authorization: Basic ${AUTH_HEADER}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
# returns 200 if successful and "true" if the node is disabled | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Flush all data from memory to WAL. This does not flush data from ingester to s3. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
echo "Flushing data from ingester..." | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
curl -X PUT "http://localhost:{{ .Values.config.ZO_HTTP_PORT }}/node/flush" \ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
-H "Authorization: Basic ${AUTH_HEADER}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
# returns 200 if successful and "true" if the node is flushed | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The curl commands lack error handling. If the API calls fail, the script continues without knowing if the operations succeeded, which could lead to data loss.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
# We need another API to check if all the data has been moved to s3 or /flush should become async and move files to s3 as well | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
# e.g /node/wal_status | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Need to build this API. Until then, we will wait for 900 seconds. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Wait for 900 seconds after flush to ensure data is moved to s3 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
# 15 minutes for now, since file movement to s3 may take up to 10 minutes | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
echo "Waiting 900 seconds to flush data..." | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
sleep 900 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The sleep duration of 900 seconds is a magic number that should be made configurable through values.yaml to allow for environment-specific tuning. Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
echo "Pre-stop hook completed" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
{{- end }} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
resources: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
{{- toYaml .Values.resources.ingester | nindent 12 }} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
envFrom: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1015,14 +1015,14 @@ probes: | |
timeoutSeconds: 5 | ||
successThreshold: 1 | ||
failureThreshold: 3 | ||
terminationGracePeriodSeconds: 30 | ||
terminationGracePeriodSeconds: 1200 # 20 minutes for now, since we are using pre-stop hook to flush data andit takes up to 10 minutes to flush data to s3 | ||
prabhatsharma marked this conversation as resolved.
Show resolved
Hide resolved
|
||
livenessProbe: | ||
initialDelaySeconds: 10 | ||
periodSeconds: 10 | ||
timeoutSeconds: 5 | ||
successThreshold: 1 | ||
failureThreshold: 3 | ||
terminationGracePeriodSeconds: 30 | ||
terminationGracePeriodSeconds: 1200 # 20 minutes for now, since we are using pre-stop hook to flush data andit takes up to 10 minutes to flush data to s3 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a spelling error in the comment: 'andit' should be 'and it'. Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||
querier: | ||
enabled: false | ||
config: | ||
|
Uh oh!
There was an error while loading. Please reload this page.