From: Tom Lane <tgl@sss.pgh.pa.us>
To: Pavel Borisov <pashkin.elfe@gmail.com>
cc: Maxim Orlov <orlovmg@gmail.com>, Julien Rouhaud <rjuju123@gmail.com>,
        Nathan Bossart <nathandbossart@gmail.com>,
        pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: remove reset_shared()
In-reply-to: 
 <CALT9ZEH_v33xfJpNjY+BtrYpyvDqPZ8xHCdzjbK21S8XGAUJeQ@mail.gmail.com>
References: <20220329221702.GA559657@nathanxps13>
 <20220330011942.fpouca2laocok7ng@jrouhaud>
 <CACG=ezYS+m1LATFiVfKGkgD3AsEW8O9b6NbZPeks6sSr2G73DQ@mail.gmail.com>
 <CALT9ZEH_v33xfJpNjY+BtrYpyvDqPZ8xHCdzjbK21S8XGAUJeQ@mail.gmail.com>
Comments: In-reply-to Pavel Borisov <pashkin.elfe@gmail.com>
	message dated "Fri, 15 Jul 2022 16:48:54 +0400"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <3885874.1657989251.1@sss.pgh.pa.us>
Date: Sat, 16 Jul 2022 12:34:11 -0400
Message-ID: <3885875.1657989251@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/3885875.1657989251%40sss.pgh.pa.us>
Precedence: bulk

Pavel Borisov <pashkin.elfe@gmail.com> writes:
> I see the proposed patch as uncontroversial and good enough to be
> committed. It will make the code a little clearer. Personally, I don't like
> leaving functions that are just wrappers for another and called only once.

Yeah, I like this for a different reason: just a couple days ago I was
comparing the postmaster's startup sequence to that used in standalone
mode (in postgres.c) and was momentarily confused because one had
reset_shared() where the other had CreateSharedMemoryAndSemaphores().

Looking in our git history, it seems that reset_shared() used to embed
slightly more knowledge, but it sure looks pretty pointless now.

> But I think that if there's a question of code readability it's not bad to
> restore the comments on the purpose of a call that were originally in the
> code.

Actually I think you chose the wrong place to move the comment to.
It applies to the initial postmaster start, because what it's pointing
out is that we'll probably choose the same IPC keys as any previous
run did.  If we felt the need to enforce that during a crash restart,
we surely could do so directly.

Pushed after fiddling with the comments.

			regards, tom lane