Skip to content

Against race conditions and duplicate cron entries#10

Merged
pastakhov merged 2 commits intomasterfrom
MBSD-354/fix-simultaneously-running
Jul 9, 2025
Merged

Against race conditions and duplicate cron entries#10
pastakhov merged 2 commits intomasterfrom
MBSD-354/fix-simultaneously-running

Conversation

@pastakhov
Copy link
Copy Markdown
Contributor

  • File Locking: Uses flock to ensure only one update process runs at a time
  • Atomic File Operations: Uses mv instead of cat for atomic file updates
  • Duplicate Prevention: Tracks processed jobs to prevent duplicate cron entries
  • Unique Temp Files: Uses process-specific temporary files to prevent conflicts
  • Proper Cleanup: Automatic cleanup of temporary files on script exit

- **File Locking**: Uses `flock` to ensure only one update process runs at a time
- **Atomic File Operations**: Uses `mv` instead of `cat` for atomic file updates
- **Duplicate Prevention**: Tracks processed jobs to prevent duplicate cron entries
- **Unique Temp Files**: Uses process-specific temporary files to prevent conflicts
- **Proper Cleanup**: Automatic cleanup of temporary files on script exit
Comment thread startup.sh Outdated
# Use flock to ensure only one update process runs at a time
(
flock -x 9 || {
log "Another update is already running, skipping this event"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This failover won't be triggered unless you provide -n parameter because -x waits for the lock to release by default

But we probably don't want to have this failover in the first place, otherwise it would discard the events after the one that triggred the update so it's possible that the contab ends up in not desired state. Example with this failover in place:

  • An event received for labels update
  • The update of the crontab started
  • Labels updated again shortly until the crontab update is completed
  • The last even is discarded and you end up with crontab state not matching the labels

So I would not add -n but instead remove the failover || {...} so it would not confuse anyone looking at it

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks, you are right.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jul 9, 2025

🐳 The image based on d2671039 commit has been built with 20250709-10 tag as ghcr.io/wikiteq/cron:20250709-10

@pastakhov pastakhov merged commit 2da693f into master Jul 9, 2025
2 checks passed
@pastakhov pastakhov deleted the MBSD-354/fix-simultaneously-running branch July 9, 2025 14:39
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