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

(PIE-1034) Ensure lockfile only deleted when appropriate #104

Conversation

gsparks
Copy link
Contributor

@gsparks gsparks commented Feb 11, 2022

The collect_api_events.rb script will check if another instance of the
script is running and exit if so. The problem arises when the exit is
caught so we can create the appropriate log messages and an ensure block
deletes the lockfile. In scenarios where the cron calls start to stack
on each other this ensure clause can delete lockfiles created by other
running instances of collect_api_events.rb.

To solve this we introduce a check before deleting the lockfile to
ensure that the current pid matches the pid in the lockfile. There is a
force parameter to override this check which is used by the
validate_command in the case that the pid referenced in the lockfile no
longer exists.

@gsparks gsparks requested a review from a team as a code owner February 11, 2022 19:45
The collect_api_events.rb script will check if another instance of the
script is running and exit if so. The problem arises when the exit is
caught so we can create the appropriate log messages and an ensure block
deletes the lockfile. In scenarios where the cron calls start to stack
on each other this ensure clause can delete lockfiles created by other
running instances of collect_api_events.rb.

To solve this we introduce a check before deleting the lockfile to
ensure that the current pid matches the pid in the lockfile. There is a
force parameter to override this check which is used by the
validate_command in the case that the pid referenced in the lockfile no
longer exists.
@gsparks gsparks force-pushed the PIE-1034_Ensure_only_current_lockfile_deleted branch from 420ec37 to 9166ebb Compare February 11, 2022 19:51
Copy link
Collaborator

@gregohardy gregohardy left a comment

Choose a reason for hiding this comment

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

Tests included ++

@gregohardy gregohardy merged commit 440d771 into puppetlabs:main Feb 14, 2022
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