Re: Incremental backups report progress as if they were full backups

Hi,

On Mon, Sep 28, 2026 at 10:30 PM Manu <manuelreyesbravo(at)gmail(dot)com> wrote:

> > A second consideration is that the patch makes GetFileBackupMethod()
> > run twice per file [...] I'm not sure whether this is something we
> > should be concerned about for clusters with a very large number of
> > files, or whether it would be preferable to cache the result
>
> I measured this. I put an INSTR_TIME around the size-only pass and
> logged it, then ran an incremental backup with and without passing ib to
> that pass, on clusters made of many empty tables (one file each), median
> of 12 runs:
>
> - 20,906 files: 37.4 ms without ib, 45.3 ms with it.
> - 100,906 files: 173.8 ms without ib, 204.0 ms with it.
>
> So the extra GetFileBackupMethod() adds about 0.3 microseconds per file
> (0.38 at 20k, 0.30 at 100k), ~30 ms at 100k files. It scales linearly
> and stays a small part of the size-only pass, which runs once before a
> backup that takes far longer. For what it's worth, I don't think
> caching the first pass is worth the extra code; the cost is not
> measurable against a real backup.
>

Thanks for the review, and especially for reproducing the issue and doing
the detailed measurements. Your results confirm that, in practice,
the additional cost of calling GetFileBackupMethod() twice is small.

> Since this changes the meaning of a documented field, I'd rather check
> > before going ahead: would this be acceptable, or would a separate
> > field be preferable?
>
> No strong opinion, but reusing the field reads right to me. The stated
> purpose of PROGRESS is to let the client tell how far along the stream
> is, which is the amount that will be sent; a separate field would leave
> the documented one reporting bytes that are never sent for an
> incremental backup.

I agree with this reasoning. I was guided by the same logic. A separate
field
would not remove the misleading value, it would just put the useful one
next to it.