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

fix disabling redbeat_lock feature #102

Closed
wants to merge 1 commit into from

Conversation

bertosantamaria
Copy link

In situations where RedBeat lock is not needed we are unable to disable this feature. This PR allows user to disable lock by omitting or assigning None in the config.
#101

@sibson
Copy link
Owner

sibson commented Nov 14, 2018

Without looking deeper, I'm not sure how the Config is handled. Does this change the default behaviour? I'm concerned existing users relying on the lock will suddenly find it stopped working for them if they didn't explicitly enable it.

@bertosantamaria
Copy link
Author

@sibson, I see your point. What if there was another config key that allowed toggling the use of the redbeat_lock_key. Something like:

  1. If you set "redbeat_use_lock" to False then you disable RedBeat's lock feature regardless of what you set for "redbeat_lock_key".
  2. If you set "redbeat_use_lock" to True or omit it completely from your config then RedBeat will use your value of "redbeat_lock_key" or default to using "redbeat_key_prefix"+":lock" for the lock (current behavior.)
#config keys and values
redbeat_lock_key: user supplied | redbeat_key_prefix + ':lock'
redbeat_use_lock: True|False

Pros:

  • keeps the code backwards compatible
  • allows users to actually disable the lock
  • uses lock by default

Cons:

  • adds another key to the config to control the lock which could be confusing (will make docs clear)

@sibson
Copy link
Owner

sibson commented Nov 16, 2018

That seems like a reasonable solution. I wish I'd done it right the first time :)

Would it be more clear to call it readbeat_ensure_only_one or redbeat_singleton? I've no attachment to any particular name.

@joekohlsdorf
Copy link
Contributor

This PR is in conflict with the current README and doesn't address that change:

RedBeat uses a distributed lock to prevent multiple instances running. To disable this feature, set:
redbeat_lock_key = None

I also strongly advise against disabling locking by default which this change does.
A second config key is not necessary to fix this. The real bug is that overwriting default configuration values does not work as expected.

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.

3 participants