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

Modify lock to allow simultaneous transfers to different hosts. #14

Merged
merged 1 commit into from
Feb 3, 2023

Conversation

william-stearns
Copy link
Contributor

Instead of a single lock limiting us to one transfer at a time, change the lock so each destination host has its own lock. This allows multiple transfers to be kicked off at the same time as long as they're to different machines. (Example use: 2 ACH servers at 2 different data centers)

Copy link
Contributor

@ethack ethack left a comment

Choose a reason for hiding this comment

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

I didn't test, but the code looks good to me. I can't think of any issues other than resource contention this behavior change could cause. If that turns out to be a problem this could be put behind a --parallel flag or similar.

@william-stearns
Copy link
Contributor Author

As a brief explanation:

  • The "flock" was originally placed there to give a lock to the transfer so that we couldn't have 2 copies of it running at any one time. (If we have 2, we'd send each file to the remote server twice, slowing down the transfer even more.)
  • The new lock statement allows one lock per remote ssh host. In a case where a sensor will be sending logs to more than one AC-Hunter or RITA system (such as when one has a primary and a backup/disaster recovery AC-Hunter system), it's possible to have transfers running to both systems simultaneously. Both can be kicked off from cron at the same time.
  • This change directly benefits one of our larger customers who needs the transfers to happen simultaneously.

@Zalgo2462
Copy link
Contributor

This has been tested in a deployment and found to be effective. The change is simple, and it shouldn't affect most users. LGTM

@Zalgo2462 Zalgo2462 merged commit 3af1d45 into master Feb 3, 2023
@Zalgo2462 Zalgo2462 deleted the wls_allow_multiple_transfers branch February 3, 2023 03:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs review request Customer request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants