Improve GC performance

[guilload]

Splits sometimes need to be deleted:

• because they were replaced after a merge.
• because they were uploaded, but some failure prevented their publication. (In that case they are in the Staged state)
• because they only contain documents out of the retention period.

We do never delete splits right away, to avoid interferring with in-flight queries. Instead we change their state to MarkForDelete. The GC then periodically delete the splits that have been in the MarkForDelete splits for more than a given grace period.

Delete here means "delete them from the storage", then "delete them from the metastore".
The current GC is pretty much stateless. Periodically it wakes up, and queries the metastore.

On a large volume (50 indexes) with partitioning, our current approach to GC has proved to be too inefficient.

There is a lot of room for improvement for our GC:

• push down the predicates that allow computing the list of splits candidate for deletion, add indexes, SELECT split_id etc.
• LIMIT 100?
• incremental GC
• batch delete request at the storage level

Also it would be very helpful to improve our observability.

[ChillFish8]

A couple of observations and thoughts while debugging this:

• Adding a limit and trying to incrementally run the deletes presents a difficult problem around how you actually catch up to the rate of deletes being produced, situations where you can catch up tend to not have the original performance issue in the first place although it probably wouldn't hurt to have, it doesn't solve the main issue which is the effects on other services when trying to keep up with a large number of deletes being produced at high load.
• Creating a index on the split_state doesn't appear to have much of an affect on the performance, although I think this is some what to be expected since the cardinality is so low.

[fulmicoton]

Adding a limit and trying to incrementally run the deletes presents a difficult problem around how you actually catch up to the rate of deletes being produced, situations where you can catch up tend to not have the original performance issue in the first place although it probably wouldn't hurt to have, it doesn't solve the main issue which is the effects on other services when trying to keep up with a large number of deletes being produced at high load.

Right now the GC loop look like

loop {
  gc()
  wait(duration)
}

it could be something like

loop {
     let splits_to_gc_possibly_remaining = gc_with_limit();
     if !splits_to_gc_possibly_remaining {
         wait(duration)
     }
  }
}

[fulmicoton]

Adding missing information: the problem was observed on a deployment that was not using the grpc metastore bridge.
All 50 indexers had an open connection to postgresql, writing in a concurrent manner

[fulmicoton]

I wonder if we should add a table listing the splits that are marked to be deleted.

I think the highest source of deletes are merges.

Retention works on mature splits, so the cardinality is reduced.

Deleting in bulk is slow in postgresql, and holds row level lock.
That way the deletes created because of merge would delete 9 rows at a time...
Possibly making the deletion from the main table way more liquid.

[guilload]

A few things regarding optimizing our schema/indexes/triggers:

• there is a trigger that updates the index update_timestamp each time we update/delete a split; that might be the culprit.
• PostgreSQL supports partial indexes, we can add partial index on split_state for the Staged and MarkForDeletion states
• the splits table has no index on index_id; that's probably a must-have for deployments with many indexes.

[ChillFish8]

there is a trigger that updates the index update_timestamp each time we update/delete a split; that might be the culprit.

I agree, reading into this it appears the triggers are notorious for causing performance issues when a large number of changes occur in bulk.

PostgreSQL supports partial indexes, we can add a partial index on split_state for the Staged and MarkForDeletion states

I've tested this but due to the low cardinality of the split_states postgres' the query planner appears to still use a sequential scan when analysing.
(attempted both with a index on just split_state and also split_state, update_timetsamp)

the splits table has no index on index_id; that's probably a must-have for deployments with many indexes.

I shall give this a try and maybe with a combination of the other indexes to see what success we have on a high number of indexes.

[guilload]

Did you add a partial index or a "regular" index on split_state?

CREATE INDEX splits__split_state_ix ON splits (split_state) WHERE split_state IN ('Staged', 'MarkedForDeletion')

[ChillFish8]

I've attempted both but neither seems to convince the query planner that it's worth using when working with one index (maybe we can convince it if there's a high number? Will try that next) 😞

delete from splits where split_state = 'MarkedForDeletion';

produces the following explanation, seemingly with and without the index being present:

[guilload]

Yeah, it's only going to manifest with a high number of splits that span multiple pages, where few splits are marked for deletion and few pages contain splits marked for deletion. Tricky.