Re: cleanup patches for incremental backup

From: Nathan Bossart <nathandbossart(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: cleanup patches for incremental backup
Date: 2024-03-14 21:00:10
Message-ID: 20240314210010.GA3056455@nathanxps13
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Jan 24, 2024 at 12:05:15PM -0600, Nathan Bossart wrote:
> There might be an overflow risk in the cutoff time calculation, but I doubt
> that's the root cause of these failures:
>
> /*
> * Files should only be removed if the last modification time precedes the
> * cutoff time we compute here.
> */
> cutoff_time = time(NULL) - 60 * wal_summary_keep_time;

I've attached a short patch for fixing this overflow risk. Specifically,
it limits wal_summary_keep_time to INT_MAX / SECS_PER_MINUTE, just like
log_rotation_age.

I considering checking for overflow when we subtract the keep-time from the
result of time(2), but AFAICT there's only a problem if time_t is unsigned,
which Wikipedia leads me to believe is unusual [0], so I figured we might
be able to just wait this one out until 2038.

> Otherwise, I think we'll probably need to add some additional logging to
> figure out what is happening...

Separately, I suppose it's probably time to revert the temporary debugging
code adding by commit 5ddf997. I can craft a patch for that, too.

[0] https://en.wikipedia.org/wiki/Unix_time#Representing_the_number

--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

Attachment Content-Type Size
overflow_fix.patch text/x-diff 1.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tomas Vondra 2024-03-14 21:13:23 Re: JIT compilation per plan node
Previous Message Heikki Linnakangas 2024-03-14 20:59:50 Re: Parallel Bitmap Heap Scan reports per-worker stats in EXPLAIN ANALYZE