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
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.
On second thought, it is reasonable for the patch to be evaluated
44 matches
Mail list logo
