public inbox for [email protected]  
help / color / mirror / Atom feed
From: David Steele <[email protected]>
To: Robert Haas <[email protected]>
To: Dilip Kumar <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: [email protected] <[email protected]>
Subject: Re: trying again to get incremental backup
Date: Thu, 19 Oct 2023 15:18:20 -0400
Message-ID: <[email protected]> (raw)
In-Reply-To: <CA+TgmoZNVeBzoqDL8xvr-nkaepq815jtDR4nJzPew7=3iEuM1g@mail.gmail.com>
References: <CA+TgmoYOYZfMCyOXFyC-P+-mdrZqm5pP2N7S-r0z3_402h9rsA@mail.gmail.com>
	<[email protected]>
	<CA+TgmoaF-TnXK9D5m3m1pTj6p5u0DiyzKuPZM1pqPGKzXhQw1w@mail.gmail.com>
	<[email protected]>
	<CA+TgmoaTZPKzcTW0pf30n+_uQtrEKRWB8qoOgkmnVh+QK2Wn_A@mail.gmail.com>
	<[email protected]>
	<CA+TgmoZoHNqx=zconLLbV+8=gNhv7g5ocXd-Mee97wvTkKc-6A@mail.gmail.com>
	<CAFiTN-sx4JS-2tKe-Vv8otWHAp3MaSTt678a7d4EdaSzBJgpBg@mail.gmail.com>
	<CA+Tgmoa_Gq2Qc9dh-WiZEUyF-nk3XULuQR-zFY2vOGNWRCZjkQ@mail.gmail.com>
	<CA+TgmoYw=8CnKEPQX9qk2estSTCaB7OGcnVM8oa-hAePG2O4Yw@mail.gmail.com>
	<CA+TgmoZNVeBzoqDL8xvr-nkaepq815jtDR4nJzPew7=3iEuM1g@mail.gmail.com>

On 10/19/23 12:05, Robert Haas wrote:
> On Wed, Oct 4, 2023 at 4:08 PM Robert Haas <[email protected]> wrote:
>> Clearly there's a good amount of stuff to sort out here, but we've
>> still got quite a bit of time left before feature freeze so I'd like
>> to have a go at it. Please let me know your thoughts, if you have any.
> 
> Apparently, nobody has any thoughts, but here's an updated patch set
> anyway. The main change, other than rebasing, is that I did a bunch
> more documentation work on the main patch (0005). I'm much happier
> with it now, although I expect it may need more adjustments here and
> there as outstanding design questions get settled.
> 
> After some thought, I think that it should be fine to commit 0001 and
> 0002 as independent refactoring patches, and I plan to go ahead and do
> that pretty soon unless somebody objects.

0001 looks pretty good to me. The only thing I find a little troublesome 
is the repeated construction of file names with/without segment numbers 
in ResetUnloggedRelationsInDbspaceDir(), .e.g.:

+			if (segno == 0)
+				snprintf(dstpath, sizeof(dstpath), "%s/%u",
+						 dbspacedirname, relNumber);
+			else
+				snprintf(dstpath, sizeof(dstpath), "%s/%u.%u",
+						 dbspacedirname, relNumber, segno);


If this happened three times I'd definitely want a helper function, but 
even with two I think it would be a bit nicer.

0002 is definitely a good idea. FWIW pgBackRest does this conversion but 
also errors if it does not succeed. We have never seen a report of this 
error happening in the wild, so I think it must be pretty rare if it 
does happen.

Regards,
-David






view thread (22+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: trying again to get incremental backup
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox