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

Allow objects to be reaped sooner than leeway interval. #470

Closed
wants to merge 1 commit into from

Conversation

kellymclaughlin
Copy link
Contributor

Change how manifest delete marking works so that objects may be cleaned up prior to the expiration of the leeway interval. The current scheme works by writing the deleted or overwritten manifest version to a garbage collection bucket with a key that is representative of a timestamp in the future when the blocks of the object represented by the manifest are eligible to actually be deleted. The garbage collection daemon periodically checks the garbage collection bucket for keys that represent a time that is in the past and judges any such key as eligible to be reaped.

The problem with this is approach is that there is not a way to override the leeway interval in the case of a disk space shortage, bloated manifests, or situations where the system may be under stress. To improve this situation the process should be upended so that the keys written to the GC bucket are representative of the current time and not a future point in time and the enforcement of the leeway_interval should be done by the GC daemon. This would allow early cleanup of garbage to be accomplished even after a manifest is written to the GC bucket.

The goals of this work are as follows:

  • Change the keys written to the GC bucket to be representative of the current time instead of a time in the future
  • Move enforcement of the leeway_interval to the GC daemon
  • Provide a way to override the default leeway_interval from the riak-cs-gc tool

@ghost ghost assigned kellymclaughlin Aug 26, 2013
kellymclaughlin added a commit that referenced this pull request Aug 27, 2013
cleaned up prior to the expiration of the leeway interval.

Fixes #470

* Change the keys written to the GC bucket to be representative of the
  current time instead of a time in the future
* Move enforcement of the leeway_interval to the GC daemon
* Provide a way to override the default leeway_interval from the riak-cs-gc tool
kellymclaughlin added a commit that referenced this pull request Jan 3, 2014
cleaned up prior to the expiration of the leeway interval.

Fixes #470

* Change the keys written to the GC bucket to be representative of the
  current time instead of a time in the future
* Move enforcement of the leeway_interval to the GC daemon
* Provide a way to override the default leeway_interval from the riak-cs-gc tool
@kellymclaughlin
Copy link
Contributor Author

I plan to squash all 4 commits and any that result from code review, but I have left them for now to show the breakdown of the work.

@@ -155,7 +155,7 @@ idle(_S) ->
{history, {call, ?GCD_MODULE, resume, []}},
{history, {call, ?GCD_MODULE, set_interval, [infinity]}},
{{paused, idle}, {call, ?GCD_MODULE, pause, []}},
{fetching_next_batch, {call, ?GCD_MODULE, manual_batch, [[testing]]}}
{fetching_next_fileset, {call, ?GCD_MODULE, manual_batch, [[testing]]}}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The state transitions have changed a bit since this work began due to the addition of optional 2I pagination for GC.

@kellymclaughlin
Copy link
Contributor Author

This branch has been freshly rebased from develop to pull in some other GC-related changes such as optional 2I pagination for gathering items to GC.

There is still the issue of confusing and inconsistent terminology (e.g. batch, fileset), but I am deferring that to the forthcoming PR to add concurrency to the GC process.

@ghost ghost assigned reiddraper Jan 6, 2014
@reiddraper
Copy link
Contributor

One thing I've been working through in my head is what happens as you upgrade to this change. I wanted to make sure that as you upgrade, no garbage will be collected early. As I've worked it out, it is safe. In fact, one 'leeway' of garbage will be collected late (2x leeway). Here's how I came up with this:

Assume 24-hour leeway. Before upgrading, some garbage is created on day 0, and written to key 1 (lets assume keys with day resolution). Now, upgrade Riak CS. Keys written to day 1 won't be collected until day 2. Which means there will be a 48-hour leeway, but just for those keys written before the upgrade. Keys written after the upgrade will be written to now(). So after 24-hours, garbage will go back to being collected after 1x leeway. Does this seem correct?

@kellymclaughlin
Copy link
Contributor Author

Why are we adding Leeway? Should it be subtracted?

Yes it should. Fixed.

@kellymclaughlin
Copy link
Contributor Author

Quite nitpicky, but what about turning this into an error-tuple, instead of passing the 'EXIT' to later be pattern matched? Not something I feel strongly about.

Yes, I see your point, but I think I'd rather do that as a specific refactor of that script instead of throwing it in here.

@kellymclaughlin
Copy link
Contributor Author

Assume 24-hour leeway. Before upgrading, some garbage is created on day 0, and written to key 1 (lets assume keys with day resolution). Now, upgrade Riak CS. Keys written to day 1 won't be collected until day 2. Which means there will be a 48-hour leeway, but just for those keys written before the upgrade. Keys written after the upgrade will be written to now(). So after 24-hours, garbage will go back to being collected after 1x leeway. Does this seem correct?

This does seem correct. A possible workaround is to temporarily set a negative leeway time if that extra interval poses a real problem, but I suspect in most cases it will not be an issue.

@reiddraper
Copy link
Contributor

Great work: 👍

cleaned up prior to the expiration of the leeway interval.

Fixes #470

* Change the keys written to the GC bucket to be representative of the
  current time instead of a time in the future
* Move enforcement of the leeway_interval to the GC daemon
* Provide a way to override the default leeway_interval from the riak-cs-gc tool
@kellymclaughlin kellymclaughlin deleted the feature/leeway-interval-flexibility branch January 7, 2014 23:13
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