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

Add a configuration option to allow TTL-based expires on the slave #620

Closed
wants to merge 1 commit into from

Conversation

kovyrin
Copy link

@kovyrin kovyrin commented Aug 8, 2012

Add a configuration option slave-allow-key-expires to allow TTL-based expires on the slave. This could be very useful for non-readonly slaves where temporary data is created based on the replicated keys and it needs to be expired after a certain amount of time.

This code is used in production at LivingSocial.

… expires on the slave. This could be useful for non-readonly slaves where temporary data is created based on the replicated keys and it needs to be expired after a TTL.
@pietern
Copy link
Contributor

pietern commented Aug 8, 2012

Thanks for the pull request.

While I understand the use case, I'm not sure it is a good idea to pull this in. There already is a thin line between replication/key expiry/consistency semantics, so adding yet another variable in the mix may not be a good idea. Because the key space is shared, enabling this feature could lead to diverging data sets very easily.

On a higher level this could be solved by having a local-only "overlay" hash table per database. This will never replicate, have its own set of expire values, but will never be used by the replicating feed. A sort of copy-on-write mechanism, if you will. This could mean that slaves are always read-only, and writes to slaves will always be local. This may reduce some of the flexibility of the current approach, but does decrease the shoot-in-the-foot probability. Also, it would perfectly solve this use case.

What do you think?

@kovyrin
Copy link
Author

kovyrin commented Aug 8, 2012

Your approach sounds pretty good, the only issue is that I'm not sure how complicated such implementation would be, what to do on local/replicated key conflicts, etc. Anyways, if you think this patch should not be accepted, I do understand the reasons. We just needed it for the described use-case and had to do it this way since it was good enough for us.

@kovyrin kovyrin closed this Sep 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants