public inbox for [email protected]  

help / color / mirror / Atom feed

From: Heikki Linnakangas <[email protected]>
To: Matthias van de Meent <[email protected]>
To: Thomas Munro <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Dilip Kumar <[email protected]>
Subject: Re: Bug: mdunlinkfiletag unlinks mainfork seg.0 instead of indicated fork+segment
Date: Tue, 7 Jul 2026 20:08:59 +0300
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAEze2WjfP95SL_Hsu7GzYXLnQyEsT49zOnNvbY_mBLCFiQra1g@mail.gmail.com>
References: <CAEze2WiWt+9+OnqW1g9rKz0gqxymmt=oe6pKAEDrutdfpDMpTw@mail.gmail.com>
	<CA+hUKGK3bbEsr8cpJXbtLhJChv7_DT2xmFcjPeVUuqQeTYdbSw@mail.gmail.com>
	<CAEze2WjfP95SL_Hsu7GzYXLnQyEsT49zOnNvbY_mBLCFiQra1g@mail.gmail.com>

On 21/12/2024 03:21, Matthias van de Meent wrote:
> On Sat, 21 Dec 2024 at 01:05, Thomas Munro <[email protected]> wrote:
>> The patch does not seem unreasonable and I'd like to help tidy this
>> up, but ... hmm, could we also consider going the other way?
>> register_unlink_segment(), mdunlinkfiletag() and the macro that
>> populates md.c's FileTag are internal to md.c, and we don't expect
>> external code to be pushing md.c SYNC_UNLINK_REQUEST requests into the
>> request queue (who would do that and what could the motivation
>> possibly be?)  Doesn't feel like a supported usage to me...  So my
>> question is: what bad thing would happen if we just renamed
>> register_unlink_segment() to register_unlink_tombstone() without
>> fork/seg arguments, to make it clear that it's not really a general
>> purpose unreliable segment unlink mechanism that we want anyone to
>> build more stuff on top of?
> 
> I just noticed I misinterpreted the conditions in mdunlinkfork, so
> that I thought it allowed a user to pass their own forknum into
> register_unlink_segment (as that is called with the user-provided
> forknum). Instead, that branch is only taken when forknum ==
> MAIN_FORKNUM, so I think you might be right that going in the other
> direction is more desirable.
> 
> In that case, something along the lines of the attached would then be
> better - it removes the fork and segno from register_unlink_segment's
> arguments (renamed to register_unlink_tombstone), and Asserts() that
> mdunlinkfiletag only receives a FileTag that contains expected values.

Renaming the function and the assertion makes sense to me. Committed to 
master, thanks!

- Heikki

view thread (5+ 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], [email protected], [email protected]
  Subject: Re: Bug: mdunlinkfiletag unlinks mainfork seg.0 instead of indicated fork+segment
  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