On 8 July 2011 17:10, Heikki Linnakangas
wrote:
> I just committed the v8 of the patch, BTW, after fixing the comment typo you
> pointed out. Thanks!
Great, thanks.
Also, thank you Florian and Fujii.
--
Peter Geoghegan http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Tra
On 08.07.2011 18:56, Peter Geoghegan wrote:
On 8 July 2011 15:58, Florian Pflug wrote:
Also, none of the callers of WaitLatch() seems to actually inspect
the WL_POSTMASTER_DEATH bit of the result. We might want to make
their !PostmasterIsAlive() check conditional on the WL_POSTMASTER_DEATH
bit
On Jul8, 2011, at 17:56 , Peter Geoghegan wrote:
> On 8 July 2011 15:58, Florian Pflug wrote:
>> SyncRepWaitForLSN() says
>> /*
>> * Wait on latch for up to 60 seconds. This allows us to check for
>> * postmaster death regularly while waiting. Note that timeout here
>> * does not necessaril
On 8 July 2011 15:58, Florian Pflug wrote:
> SyncRepWaitForLSN() says
> /*
> * Wait on latch for up to 60 seconds. This allows us to check for
> * postmaster death regularly while waiting. Note that timeout here
> * does not necessarily release from loop.
> */
> WaitLatch(&MyProc->waitLa
On Jul8, 2011, at 14:40 , Heikki Linnakangas wrote:
> On 08.07.2011 13:58, Florian Pflug wrote:
>> On Jul8, 2011, at 11:57 , Peter Geoghegan wrote:
>>> On 7 July 2011 19:15, Robert Haas wrote:
> I'm not concerned about the possibility of spurious extra cycles of
> auxiliary process event l
On 08.07.2011 16:11, Peter Geoghegan wrote:
Incidentally, I like that this removes the amDirectChild argument to
PostmasterIsAlive() - an added benefit.
amDirectChild==false has actually been dead code for years. But the new
pipe method would work for a non-direct child too as long as the pipe
On 8 July 2011 13:40, Heikki Linnakangas
wrote:
> I put the burden on the callers. Removing the return value from WaitLatch()
> altogether just makes life unnecessarily difficult for callers that could
> safely use that information, even if you sometimes get spurious wakeups. In
> particular, the
On 08.07.2011 13:58, Florian Pflug wrote:
On Jul8, 2011, at 11:57 , Peter Geoghegan wrote:
On 7 July 2011 19:15, Robert Haas wrote:
I'm not concerned about the possibility of spurious extra cycles of
auxiliary process event loops - should I be?
A tight loop would be bad, but an occasional sp
On Jul8, 2011, at 11:57 , Peter Geoghegan wrote:
> On 7 July 2011 19:15, Robert Haas wrote:
>>> I'm not concerned about the possibility of spurious extra cycles of
>>> auxiliary process event loops - should I be?
>>
>> A tight loop would be bad, but an occasional spurious wake-up seems harmless.
On 7 July 2011 19:15, Robert Haas wrote:
>> I'm not concerned about the possibility of spurious extra cycles of
>> auxiliary process event loops - should I be?
>
> A tight loop would be bad, but an occasional spurious wake-up seems harmless.
We should also assert !PostmasterIsAlive() from within
On Thu, Jul 7, 2011 at 1:41 PM, Peter Geoghegan wrote:
> I now think that we shouldn't change the return value format from the
> most recent revisions of the patch (i.e. returning a bitfield). We
> should leave it as-is, while documenting that it's possible, although
> extremely unlikely, for it t
I now think that we shouldn't change the return value format from the
most recent revisions of the patch (i.e. returning a bitfield). We
should leave it as-is, while documenting that it's possible, although
extremely unlikely, for it to incorrectly report Postmaster death, and
that clients therefor
On 5 July 2011 07:49, Heikki Linnakangas
wrote:
> Good point, and testing shows that that is exactly what happens at least on
> Linux (see attached test program). So, as the code stands, the children will
> go into a busy loop until the grandparent calls waitpid(). That's not good.
>
> In that lig
On 05.07.2011 00:42, Florian Pflug wrote:
On Jul4, 2011, at 23:11 , Peter Geoghegan wrote:
On 4 July 2011 17:36, Florian Pflug wrote:
Btw, with the death-watch / life-sign / whatever infrastructure in place,
shouldn't PostmasterIsAlive() be using that instead of getppid() / kill(0)?
Hmm, may
On Tue, Jul 5, 2011 at 1:36 AM, Florian Pflug wrote:
> On Jul4, 2011, at 17:53 , Heikki Linnakangas wrote:
>>> Under Linux, select() may report a socket file descriptor as "ready
>>> for
>>> reading", while nevertheless a subsequent read blocks. This could
>>> for
>>> example
On 4 July 2011 22:42, Florian Pflug wrote:
> If we do expect such event, we should close the hole instead of asserting.
> If we don't, then what's the point of the assert.
You can say the same thing about any assertion. I'm not going to
attempt to close the hole because I don't believe that there
On Jul4, 2011, at 23:11 , Peter Geoghegan wrote:
> On 4 July 2011 17:36, Florian Pflug wrote:
>> On Jul4, 2011, at 17:53 , Heikki Linnakangas wrote:
Under Linux, select() may report a socket file descriptor as "ready
for
reading", while nevertheless a subsequent read b
On 4 July 2011 16:53, Heikki Linnakangas
wrote:
> Ok, here's a new patch, addressing the issues Fujii raised, and with a bunch
> of stylistic changes of my own. Also, I committed a patch to remove
> silent_mode, so the fork_process() changes are now gone. I'm going to sleep
> over this and review
On Jul4, 2011, at 17:53 , Heikki Linnakangas wrote:
>> Under Linux, select() may report a socket file descriptor as "ready for
>> reading", while nevertheless a subsequent read blocks. This could for
>> example happen when data has arrived but upon examination has wrong
>>
Ok, here's a new patch, addressing the issues Fujii raised, and with a
bunch of stylistic changes of my own. Also, I committed a patch to
remove silent_mode, so the fork_process() changes are now gone. I'm
going to sleep over this and review once again tomorrow, and commit if
it still looks goo
On Thu, Jun 30, 2011 at 5:47 AM, Peter Geoghegan wrote:
> I don't think that the way I've phrased my error messages is
> inconsistent with that style guide, excepty perhaps the pipe()
> reference, but if you feel it's important to try and use "could not",
> I have no objections.
I like Fujii's r
On 30 June 2011 08:58, Heikki Linnakangas
wrote:
> Here's a WIP patch with some mostly cosmetic changes I've done this far. I
> haven't tested the Windows code at all yet. It seems that no-one is
> objecting to removing silent_mode altogether, so I'm going to do that before
> committing this patc
On 30.06.2011 09:36, Fujii Masao wrote:
On Sat, Jun 25, 2011 at 10:41 AM, Peter Geoghegan wrote:
Attached is patch that addresses Fujii's third and most recent set of concerns.
Thanks for updating the patch!
I think that Heikki is currently taking another look at my work,
because he indicat
On Sat, Jun 25, 2011 at 10:41 AM, Peter Geoghegan wrote:
> Attached is patch that addresses Fujii's third and most recent set of
> concerns.
Thanks for updating the patch!
> I think that Heikki is currently taking another look at my work,
> because he indicates in a new message to the list a sh
Attached is patch that addresses Fujii's third and most recent set of concerns.
--
Peter Geoghegan http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 4952d22..bfe6bc
On 24 June 2011 12:30, Fujii Masao wrote:
> +#ifndef WIN32
> + /*
> + * Initialise mechanism that allows waiting latch clients
> + * to wake on postmaster death, to finish their
> + * remaining business
> + */
> + InitPostmasterDeathWatchHandle();
> +#endif
On Wed, Jun 22, 2011 at 9:11 PM, Peter Geoghegan wrote:
>> rc = WaitForMultipleObjects(numevents, events, FALSE,
>> (timeout >= 0) ?
>> (timeout / 1000) : INFINITE);
>> - if (rc == WAIT_FAILED)
>> +
Attached patch addresses Fujii's more recent concerns.
On 22 June 2011 04:54, Fujii Masao wrote:
> +WaitLatch(volatile Latch *latch, int wakeEvents, long timeout)
> +WaitLatchOrSocket(volatile Latch *latch, int wakeEvents, pgsocket
> sock, long timeout)
>
> If 'wakeEvent' is zero, we cannot get
On Tue, Jun 21, 2011 at 6:22 PM, Peter Geoghegan wrote:
> Thanks for giving this your attention Fujii. Attached patch addresses
> your concerns.
Thanks for updating the patch! I have a few comments;
+WaitLatch(volatile Latch *latch, int wakeEvents, long timeout)
+WaitLatchOrSocket(volatile Latch
Thanks for giving this your attention Fujii. Attached patch addresses
your concerns.
On 20 June 2011 05:53, Fujii Masao wrote:
> 'hifd' should be initialized to 'selfpipe_readfd' before the above
> 'if' block. Otherwise,
> 'hifd = postmaster_alive_fds[POSTMASTER_FD_WATCH]' might have no effect.
On Mon, Jun 20, 2011 at 1:00 AM, Peter Geoghegan wrote:
> I took another look at this this evening, and realised that my
> comments could be a little clearer.
>
> Attached revision cleans them up a bit.
Since I'm not familiar with Windows, I haven't read the code related
to Windows. But
the follo
I took another look at this this evening, and realised that my
comments could be a little clearer.
Attached revision cleans them up a bit.
--
Peter Geoghegan http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services
diff --git a/src/backend/access/transam/xlog
On 16 June 2011 16:30, Heikki Linnakangas
wrote:
> This patch breaks silent_mode=on. In silent_mode, postmaster forks early on,
> to detach from the controlling tty. It uses fork_process() for that, which
> with patch closes the write end of the postmaster-alive pipe, but that's
> wrong because th
Excerpts from Peter Geoghegan's message of jue jun 16 08:42:39 -0400 2011:
> On 16 June 2011 13:15, Heikki Linnakangas
> wrote:
>
> > Hmm, I'm not sure having the pid in that error message is too useful in the
> > first place. The process was just spawned, and it will die at that error.
> > When
This patch breaks silent_mode=on. In silent_mode, postmaster forks early
on, to detach from the controlling tty. It uses fork_process() for that,
which with patch closes the write end of the postmaster-alive pipe, but
that's wrong because the child becomes the postmaster process.
On a stylisti
On 16 June 2011 15:27, Heikki Linnakangas
wrote:
> I don't understand that comment. Why can't e.g postmaster death happen at
> the same time as a latch is set? I think the code is fine as it is, we just
> need to document that if there are several events that would wake up
> WaitLatch(), we make
Peter Geoghegan wrote:
--- 247,277
* do that), and the select() will return immediately.
*/
drainSelfPipe();
! if (latch->is_set && (wakeEvents & WL_LATCH_SET))
! {
! result |= WL_LATCH_SET;
!
On 16 June 2011 13:15, Heikki Linnakangas
wrote:
> Hmm, I'm not sure having the pid in that error message is too useful in the
> first place. The process was just spawned, and it will die at that error.
> When you try to debug that sort of error, what you would compare the pid
> with? And you can
On 16.06.2011 15:07, Peter Geoghegan wrote:
I had another quick look-over this patch, and realised that I made a
minor mistake:
+void
+ReleasePostmasterDeathWatchHandle(void)
+{
+ /* MyProcPid won't have been set yet */
+ Assert(PostmasterPid != getpid());
+ /* Please don't ask
I had another quick look-over this patch, and realised that I made a
minor mistake:
+void
+ReleasePostmasterDeathWatchHandle(void)
+{
+ /* MyProcPid won't have been set yet */
+ Assert(PostmasterPid != getpid());
+ /* Please don't ask twice */
+ Assert(postmaster_alive_fds[
On Thu, May 26, 2011 at 11:58 AM, Peter Geoghegan wrote:
> Attached revision doesn't use any threads or pipes on win32. It's far
> neater there. I'm still seeing that "lagger" process (which is an
> overstatement) at times, so I guess it is normal. On Windows, there is
> no detailed PS output, so
On 26 May 2011 11:22, Heikki Linnakangas
wrote:
> The Unix-stuff looks good to me at a first glance.
Good.
> There's one reference left to "life sign" in comments. (FWIW, I don't have a
> problem with that terminology myself)
Should have caught that one. Removed.
> Looking at the MSDN docs aga
On 24.05.2011 23:43, Peter Geoghegan wrote:
Attached is the latest revision of the latch implementation that
monitors postmaster death, plus the archiver client that now relies on
that new functionality and thereby works well without a tight
PostmasterIsAlive() polling loop.
The Unix-stuff look
f the
net commit latency. So I'd think this is worth fixing.
Thanks,
--Ganesh
On Thu, 23 Sep 2010, Simon Riggs wrote:
Date: Thu, 23 Sep 2010 06:56:38 -0700
From: Simon Riggs
To: Ganesh Venkitachalam
Cc: "pgsql-hackers@postgresql.org"
Subject: Re: [HACKERS] Latch implementation
On Wed, 2010-09-22 at 13:31 -0700, Ganesh Venkitachalam-1 wrote:
> Hi,
>
> I've been playing around with measuring the latch implementation in 9.1,
> and here are the results of a ping-pong test with 2 processes signalling
> and waiting on the latch. I did three variations (linux 2.6.18, nehalem
On 22/09/10 23:31, Ganesh Venkitachalam-1 wrote:
I've been playing around with measuring the latch implementation in 9.1,
and here are the results of a ping-pong test with 2 processes signalling
and waiting on the latch. I did three variations (linux 2.6.18, nehalem
processor).
One is the curren
On Wed, Sep 22, 2010 at 4:31 PM, Ganesh Venkitachalam-1
wrote:
> I've been playing around with measuring the latch implementation in 9.1, and
> here are the results of a ping-pong test with 2 processes signalling and
> waiting on the latch. I did three variations (linux 2.6.18, nehalem
> processor
47 matches
Mail list logo
