Hello Andres,
I think putting this into the control file is a seriously bad
idea. Postmaster interlocks against other postmasters running via
postmaster.pid.
Having a second interlock mechanism, in a different file, doesn't make
any sort of sense. Nor does it seem sane to have external tool
Hi,
On 2019-03-01 10:11:53 +0900, Michael Paquier wrote:
> One thing is that we don't protect a data folder to be started when it
> is in the process of being treated by an external tool, like
> pg_rewind, or pg_checksums. So having an extra flag in the control
> file, which can be used by extern
On Thu, Feb 28, 2019 at 01:07:23PM -0800, Andres Freund wrote:
> Huh? Postmaster.pid is written by the backend, pg_ctl just checks it to
> see if the backend has finished starting up etc. It's precisely what the
> backend uses to prevent two postmasters to start etc. It's also what say
> pg_resetwa
On Thu, Feb 28, 2019 at 04:09:32PM -0500, Joe Conway wrote:
> Committed and push that way.
Thanks for committing a fix.
> By the way, while looking at this, I noted at least a couple of places
> where OpenTransientFile() is being passed O_RDWR when the usage is
> pretty clearly intended to be rea
On 2/28/19 7:20 AM, Michael Paquier wrote:
> On Thu, Feb 28, 2019 at 07:11:04AM -0500, Joe Conway wrote:
>> Sure, will do. What are your thoughts on backpatching? This seems
>> unlikely to be a practical concern in the field, so my inclination is a
>> master only fix.
>
> I agree that this would u
Hi,
On 2019-02-28 09:54:48 +0100, Fabien COELHO wrote:
> > If we were to want to do more here, ISTM the right approach would use
> > the postmaster pid file, not the control file.
>
> ISTM that this just means re-inventing a manual poor-featured
> race-condition-prone lock API around another file
On Thu, Feb 28, 2019 at 07:11:04AM -0500, Joe Conway wrote:
> Sure, will do. What are your thoughts on backpatching? This seems
> unlikely to be a practical concern in the field, so my inclination is a
> master only fix.
I agree that this would unlikely become an issue as an error on the
control f
On 2/27/19 7:54 PM, Michael Paquier wrote:
> On Wed, Feb 27, 2019 at 07:45:11PM -0500, Joe Conway wrote:
>> It seems to me that OpenTransientFile() is more appropriate. Patch done
>> that way attached.
>
> Works for me, thanks for sending a patch! While on it, could you
> clean up the comment on
Hello Andres,
Note that my concern is not about the page size, but rather that as more
commands may change the cluster status by editing the control file, it would
be better that a postmaster does not start while a pg_rewind or enable
checksum or whatever is in progress, and currently there is
On Wed, Feb 27, 2019 at 07:45:11PM -0500, Joe Conway wrote:
> It seems to me that OpenTransientFile() is more appropriate. Patch done
> that way attached.
Works for me, thanks for sending a patch! While on it, could you
clean up the comment on top of get_controlfile()? "char" is mentioned
instea
On Wed, Feb 27, 2019 at 11:50:17AM +0100, Fabien COELHO wrote:
>> Shouldn't be necessary - the control file fits into a single page, and
>> writes of that size ought to always be atomic. And I also think
>> introducing flock usage for this would be quite disproportional.
There are static assertion
Hi,
On 2019-02-27 11:50:17 +0100, Fabien COELHO wrote:
> Note that my concern is not about the page size, but rather that as more
> commands may change the cluster status by editing the control file, it would
> be better that a postmaster does not start while a pg_rewind or enable
> checksum or wh
On 2/27/19 10:26 AM, Joe Conway wrote:
> On 2/27/19 2:47 AM, Michael Paquier wrote:
>> Hi all,
>> (CC-ing Joe as of dc7d70e)
> According to that comment BasicOpenFile does not seem to solve the issue
> you are pointing out (leaking of file descriptor on ERROR). Perhaps
> OpenTransientFile() is mor
On 2/27/19 2:47 AM, Michael Paquier wrote:
> Hi all,
> (CC-ing Joe as of dc7d70e)
>
> I was just looking at the offline checksum patch, and noticed some
> sloppy coding in controldata_utils.c. The control file gets opened in
> get_controlfile(), and if it generates an error then the file
> descri
However, while at it, there is also the question of whether the control file
should be locked when updated, eg with flock(2) to avoid race conditions
between concurrent commands. ISTM that there is currently not such thing in
the code, but that it would be desirable.
Shouldn't be necessary -
Hi,
On 2019-02-27 10:23:45 +0100, Fabien COELHO wrote:
> However, while at it, there is also the question of whether the control file
> should be locked when updated, eg with flock(2) to avoid race conditions
> between concurrent commands. ISTM that there is currently not such thing in
> the code,
Bonjour Michaël,
Thoughts?
None.
However, while at it, there is also the question of whether the control
file should be locked when updated, eg with flock(2) to avoid race
conditions between concurrent commands. ISTM that there is currently not
such thing in the code, but that it would be
Hi all,
(CC-ing Joe as of dc7d70e)
I was just looking at the offline checksum patch, and noticed some
sloppy coding in controldata_utils.c. The control file gets opened in
get_controlfile(), and if it generates an error then the file
descriptor remains open. As basic code rule in the backend we
18 matches
Mail list logo
