Page MenuHomePhabricator

Configurable garbage collection / revision retention policy in table schemas: add 'interval' policy support
Closed, ResolvedPublic

Description

Our table storage backend is generally modeled around revisioning and MVCC. This has many advantages for eventually consistent storage systems & distributed systems, but - if all versions are kept - can also result in a lot of storage used.

We have recently started to manually thin out our renders per revision (T94196) with an ad-hoc script. This is however fairly inefficient and operationally complex. Instead, we should implement a schema-configurable garbage collection policy in the table storage backend.

The schema property used to configure the policy could look like this:

{
     revisionRetentionPolicy: {
           type: 'latest',
           count: 1,
           grace_ttl: 86400
     }
}

The grace_ttl defines how long an entry is kept around after it was superseded. This is useful to let clients finish operations that depend on a specific revision, as described in T94422.

Here is another example keeping one entry every 24 hours (which could be a reasonable choice for Parsoid revision renders):

{
     revisionRetentionPolicy: {
           type: 'interval',
           interval: 86400,
           count: 1,
           grace_ttl: 86400
     }
}

Defaults could be:

  • 'latest' for logical tables that don't have an explicit timeuuid property defined in the last index position
  • 'all' (keep all revisions) for tables that have an explicit timeuuid property defined in the last index position

Event Timeline

GWicke raised the priority of this task from to Needs Triage.
GWicke updated the task description. (Show Details)
GWicke added subscribers: GWicke, mobrovac, Eevans.
GWicke renamed this task from RFC Configurable garbage collection policy in table schemas to RFC: Configurable garbage collection policy in table schemas.Mar 31 2015, 12:59 AM
GWicke set Security to None.

LGTM +1. The defaults are sane enough so that new applications / modules do not need to worry about these things.

In your second example, it's a bit unclear to me why use both interval and grace_ttl. Well, to be precise, I wonder if having both could lead to misconfigurations. Wouldn't it be easier to have just interval and then assume grace_ttl = interval, leading to keep count items / records for every interval seconds, and once any of them has been superseded, keep them around for interval seconds more.

Should we also have some kind of throttling policy? E.g. imagine a page gets updated / re-rendered every minute. Are we to keep 1440 copies until grace_ttl elapses? This seems like a waste of resources. (I don't have an idea to solve this, just putting it out there)

LGTM +1. The defaults are sane enough so that new applications / modules do not need to worry about these things.

In your second example, it's a bit unclear to me why use both interval and grace_ttl. Well, to be precise, I wonder if having both could lead to misconfigurations. Wouldn't it be easier to have just interval and then assume grace_ttl = interval, leading to keep count items / records for every interval seconds, and once any of them has been superseded, keep them around for interval seconds more.

I don't think that grace_ttl needs to be coupled to interval at all. It can very well be 0 or longer than the interval. In the case of 0, we'd only keep one copy within each interval (modulo maybe a short minimum TTL for secondary index stabilization), even if there were many renders within the interval. In the case of something > interval, the TTL on the data would be set higher than interval.

Should we also have some kind of throttling policy? E.g. imagine a page gets updated / re-rendered every minute. Are we to keep 1440 copies until grace_ttl elapses? This seems like a waste of resources. (I don't have an idea to solve this, just putting it out there)

The only way we could do this without compromising the guarantee of having a given tid around for client use within grace_ttl would be to refuse adding new renders if an update threshold is reached. In fact, we could do this by making the current If-Unmodified-Since check a bit fuzzy (return failure if the page was updated > threshold times in last interval & was updated slightly before the job time). However, that would of course miss some updates.

Eevans updated the task description. (Show Details)

I think the main considerations for secondary indexes are:

  • The minimum TTL should be long enough to let secondary index updates finish. Something like 10 seconds should be enough.
  • We currently version secondary index entries. A value that was matched by a previous tid revision, but no longer matches by the latest, is currently marked with the timeuuid of the revision that obsoleted it. This lets us (theoretically) support secondary index queries in the past. As we discussed before, we should probably change this so that old (no longer matching, _del set) index entries are deleted with grace_ttl whenever the corresponding main data row is deleted.
GWicke renamed this task from RFC: Configurable garbage collection policy in table schemas to RFC: Configurable garbage collection / revision retention policy in table schemas.Apr 3 2015, 12:25 AM
Eevans renamed this task from RFC: Configurable garbage collection / revision retention policy in table schemas to Configurable garbage collection / revision retention policy in table schemas.Apr 15 2015, 6:24 PM
Eevans triaged this task as Medium priority.Apr 15 2015, 7:16 PM

Basic support is done and live in production. We have not implemented the 'interval' policy yet, which would be preferable in the longer term.

GWicke renamed this task from Configurable garbage collection / revision retention policy in table schemas to Configurable garbage collection / revision retention policy in table schemas: add 'interval' policy support.Jun 29 2015, 5:36 PM
GWicke moved this task from Under discussion to Ready / next on the RESTBase board.

Here is a sketch of how the interval retention policy could work:

  • keep an array of candidate rows
  • scan backwards (just as the current policy does) over the previous revisions
  • if the render has no TTL set, and tid is in interval: add to candidate rows
  • else if the revision has a TTL set, or its tid is older than interval: set gc_grace_ttl on candidate rows encountered so far & return
  • else: do nothing & return

@GWicke Also, the first ever render of a revision is never deleted.

All PRs have been merged, released for restbase-mod-table-cassandra in version 0.7.10 and restbase-mod-table-sqlite version 0.1.4, so the ticket can be closed.