Re: [pbs-devel] [PATCH proxmox-backup] gc: attach context to index reader errors and ignore NotFound

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On September 8, 2020 1:18 pm, Stefan Reiter wrote:
> On 9/8/20 1:12 PM, Fabian Grünbichler wrote:
>> On September 8, 2020 11:18 am, Stefan Reiter wrote:
>>> Ignore NotFound errors during phase 1, this just means that a snapshot
>>> was forgotten or pruned between scanning for .fidx/.didx files and
>>> actually opening the index to touch the chunks.
>> 
>> I originally had a similar patch already lying around, but I am not sure
>> whether this is not too dangerous in the face of transient errors?
>> 
>> I'd much rather get to a point where we are sure that no concurrent
>> prune/forget operation can happen, and treat all errors as errors,
>> instead of treating all not found errors as benign 'must have happened
>> cause of concurrent actions'.
>> 
> 
> So no forget/prune during phase 1 of GC? That sounds like it would cause 
> quite some congestion.

or locking and touching group-wise, to reduce granularity and 
contention? or let prune/forget wait until GC phase 1 is over, by having 
a higher lock timeout?

phase 1 does not take too long here, but it probably depends a lot on 
datastore setup and size (special vdevs and enough RAM for caching 
probably help a lot here..)

we could also just mark them as deleted (touch $snapshot/.deleted) and 
let GC do the actual deletion of metadata as well, but that would be a 
much more involved change. added benefit that GC is now the only thing 
that deletes stuff (except for cleanup of aborted backup tasks, but that 
could also switch to that mechanism I guess).

> 
>> this is not pull, or download/restore, where we can just retry later -
>> if we skip the index here, all the chunks it referenced are up for
>> garbage collection unless they are saved by another index!
>> 
> 
> I do see where you're coming from, but what alternative is there? If the 
> index file is not found, we can't touch any referenced chunks anyway - 
> there are none for us to see.

the alternatives are

A) treat index files which we expected to read that have vanished as 
'must be benign', and continue GC
B) try to not have a scenario where that can happen benignly (e.g., 
because of a mutex between operations that delete indices and this phase 
of GC), so that we can know that it is an error and treat it as such

I'd like to choose B since it is the safe alternative, and this is the 
one path where having a bug could wipe out whole datastores, but if it's 
too involved then we have to go with A