Re: trying again to get incremental backup

21.12.2023 15:07, Robert Haas wrote:
> On Wed, Dec 20, 2023 at 11:00 PM Alexander Lakhin <exclusion(at)gmail(dot)com> wrote:
>> I've found several typos/inconsistencies introduced with 174c48050 and
>> dc2123400. Maybe you would want to fix them, while on it?:
> That's an impressively long list of mistakes in something I thought
> I'd been careful about. Sigh.
>
> I don't suppose you could provide these corrections in the form of a
> patch? I don't really want to run these sed commands across the entire
> tree and then try to figure out what's what...

Please look at the attached patch; it corrects all 29 items ("recods"
fixed in two places), but maybe you find some substitutions wrong...

I've also observed that those commits introduced new warnings:
$ CC=gcc-12 CPPFLAGS="-Wtype-limits" ./configure -q && make -s -j8
reconstruct.c: In function ‘read_bytes’:
reconstruct.c:511:24: warning: comparison of unsigned expression in ‘< 0’ is always false [-Wtype-limits]
  511 |                 if (rb < 0)
      |                        ^
reconstruct.c: In function ‘write_reconstructed_file’:
reconstruct.c:650:40: warning: comparison of unsigned expression in ‘< 0’ is always false [-Wtype-limits]
  650 |                                 if (rb < 0)
      |                                        ^
reconstruct.c:662:32: warning: comparison of unsigned expression in ‘< 0’ is always false [-Wtype-limits]
  662 |                         if (wb < 0)

There are also two deadcode.DeadStores complaints from clang. First one is
about:
        /*
         * Align the wait time to prevent drift. This doesn't really matter,
         * but we'd like the warnings about how long we've been waiting to say
         * 10 seconds, 20 seconds, 30 seconds, 40 seconds ... without ever
         * drifting to something that is not a multiple of ten.
         */
        timeout_in_ms -=
            TimestampDifferenceMilliseconds(current_time, initial_time) %
            timeout_in_ms;
It looks like this timeout is really not used.

And the minor one (similar to many existing, maybe doesn't deserve fixing):
walsummarizer.c:808:5: warning: Value stored to 'summary_end_lsn' is never read [deadcode.DeadStores]
                                        summary_end_lsn = private_data->read_upto;
                                        ^ ~~~~~~~~~~~~~~~~~~~~~~~

>> Also, a comment above MaybeRemoveOldWalSummaries() basically repeats a
>> comment above redo_pointer_at_last_summary_removal declaration, but
>> perhaps it should say about removing summaries instead?
> Wow, yeah. Thanks, will fix.

Thank you for paying attention to it!

Best regards,
Alexander