Skip to content

improve readiness check for searchd and index freshness #657

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

Merged
merged 2 commits into from
Feb 13, 2025
Merged

improve readiness check for searchd and index freshness #657

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2c88003
improve readiness check for searchd and index freshness
ltclm Feb 13, 2025
40ea517
feedback adrien
ltclm Feb 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 11 additions & 6 deletions scripts/checker.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ searchd --status 1> /dev/null || exit 1

# check if index files are up-to-date
LAST_SYNC="/tmp/last_sync_finished.txt"
MAX_AGE=300 # max age in seconds should be the same interval as the cron settings
# max age in seconds, default value should be at least the same interval as the cron settings in docker-crontab, default 5 minutes (300 seconds)
# in productive systems it is better to use a higher value, e.g. 2x300s = 600s
MAX_AGE=${MAX_AGE:-300}

# check if index sync status file exists
if [[ ! -f "${LAST_SYNC}" ]]; then
Expand All @@ -22,13 +24,16 @@ if [[ ! -f "${LAST_SYNC}" ]]; then
exit 1
fi

# Calculate the time MAX_AGE seconds ago in seconds
five_mins_ago=$(( $(date +%s) - MAX_AGE ))
# Get the file's last modification time in seconds
file_mtime=$(stat -c %Y "${LAST_SYNC}")
# check if index-sync-rotate.sh is currently running with the lock file
# if a sync is currently running, further tests should not be executed
LOCK_FILE="/tmp/index-sync-rotate.sh"
Copy link
Member

Choose a reason for hiding this comment

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

Have you considered using flock? Using lock files correctly is surprisingly hard and that tools can help simplify things greatly. That said, I am lacking context on what exactly this script is supposed to be doing and how it's used to make a more specific recommendation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thank you for the feedback, yes the script index-sync-rotate.sh is using flock for the locking. :
https://github.com/geoadmin/service-search-sphinx/blob/develop-2025-03-12/scripts/index-sync-rotate.sh#L76

this script here is just checking if the lock is active which means than a sync is currently running.
this script here is just used for the readiness probe in kubernetes. one of the tests is to detect pods with outdated index files. inside the pod the index-sinc-rotate.sh is executed every 5m in order to sync (externally updated) sphinx index files into the local storage. as part of this sync the search service is being restarted with SIGHUP.

Copy link
Member

Choose a reason for hiding this comment

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

Cool. If that lock file is already managed by flock, its existence alone does not necessarily mean the lock is active. There are cases where the EXIT handler won't run. I think you want to test the return value of flock -n "${LOCK_FILE}" true.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for that advice! i have changed the test accordingly 👍

if [[ -f "${LOCK_FILE}" ]]; then
echo "$(basename "${LOCK_FILE}") is currently running. Exiting."
exit 0
fi

# check if index sync status file is up-to-date
if [[ $file_mtime -lt $five_mins_ago ]]; then
if [[ $(stat -c %Y "${LAST_SYNC}") -lt $(( $(date +%s) - MAX_AGE )) ]]; then
echo "index sync status file is outdated: ${LAST_SYNC}"
exit 1
fi
Loading