Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-13 Thread Heikki Linnakangas
On 13/09/10 20:43, Jeff Davis wrote: On Mon, 2010-09-13 at 09:10 +0300, Heikki Linnakangas wrote: but we should be consistent and document that: (a) it shouldn't happen (b) that it's just a sanity check and we're ignoring the race Would this be sufficient? --- a/src/backend/port/unix_la

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-13 Thread Jeff Davis
On Mon, 2010-09-13 at 09:10 +0300, Heikki Linnakangas wrote: > > but we should be consistent and document that: > > (a) it shouldn't happen > > (b) that it's just a sanity check and we're ignoring the race > > Would this be sufficient? > > --- a/src/backend/port/unix_latch.c > +++ b/src/backe

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-12 Thread Heikki Linnakangas
On 12/09/10 20:13, Jeff Davis wrote: On Sun, 2010-09-12 at 12:29 -0400, Tom Lane wrote: ... why throw an ERROR there if it can't happen (or indicates an inconsistent state when it does happen)? Are you suggesting that an Assert would be sufficient? I'm not too picky about whether it's Assert

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-12 Thread Jeff Davis
On Sun, 2010-09-12 at 14:12 -0400, Tom Lane wrote: > Uh, this is nonsense. You have to have something like these functions > to support transferring ownership of a latch from one process to > another, which is required at least for the walreceiver usage. Oh, I see. It's needed to know where to se

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-12 Thread Tom Lane
Jeff Davis writes: > However, that also means that the whole concept of OwnLatch/DisownLatch > is entirely redundant, and only there for asserts because it doesn't do > anything else. That seems a little strange to me, as well, so (at > minimum) it should be documented that the functions really ha

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-12 Thread Jeff Davis
On Sun, 2010-09-12 at 10:13 -0700, Jeff Davis wrote: > I'm not too picky about whether it's Assert, ERROR, or PANIC (Asserts > aren't available in production systems, which I assume is why elog was > used); but we should be consistent and document that: > (a) it shouldn't happen > (b) that it's j

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-12 Thread Jeff Davis
On Sun, 2010-09-12 at 12:29 -0400, Tom Lane wrote: > > ... why throw an ERROR there if it can't happen (or > > indicates an inconsistent state when it does happen)? > > Are you suggesting that an Assert would be sufficient? > I'm not too picky about whether it's Assert, ERROR, or PANIC (Asserts

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-12 Thread Tom Lane
Jeff Davis writes: > I glanced at the code, and I see (in OwnLatch()): > + if (latch->owner_pid != 0) > + elog(ERROR, "latch already owned"); > + latch->owner_pid = MyProcPid; > But it looks like there may be a race there. Yeah, that error check is only intended to cat

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-12 Thread Jeff Davis
On Sat, 2010-09-11 at 19:15 +0300, Heikki Linnakangas wrote: > Committed. I'll take a look at making walreceiver respond quickly when > WAL arrives in the standby, using latches, but that shouldn't interfere > with what you're doing. I glanced at the code, and I see (in OwnLatch()): + if

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-11 Thread Heikki Linnakangas
On 11/09/10 18:02, Tom Lane wrote: Heikki Linnakangas writes: Barring any last-minute objections, I'll commit this in the next few days. This patch doesn't affect walreceiver yet; I think the next step is to use the latches to eliminate the polling loop in walreceiver too, so that as soon as a

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-11 Thread Tom Lane
Heikki Linnakangas writes: > On 06/09/10 19:27, Heikki Linnakangas wrote: >> It seems that NumSharedLatches() is entirely wrongly placed if it's in >> the win32 specific code! That needs to be somewhere shared, so people find >> it, > Yeah. There's a notice of that in OwnLatch(), but it would be

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-11 Thread Heikki Linnakangas
On 06/09/10 19:27, Heikki Linnakangas wrote: On 06/09/10 17:18, Tom Lane wrote: BTW, on reflection the AcquireLatch/ReleaseLatch terminology seems a bit ill chosen: ReleaseLatch sounds way too much like something that would just unlock or clear the latch. Perhaps OwnLatch/DisownLatch, or somethi

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-09 Thread Markus Wanner
On 09/08/2010 08:01 PM, Heikki Linnakangas wrote: Yeah, there isn't much you can do about it. Perhaps you could set a "mayday flag" (a global boolean variable) if it fails, and check that in the main loop, elogging a warning there instead. But I don't think we need to go to such lengths, realisti

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-09 Thread Markus Wanner
On 09/08/2010 08:18 PM, Tom Lane wrote: Considering that we know that major platforms such as FreeBSD have changed their implementations *very* recently, it seems foolish to assume that an executable built on a machine with corrected pselect could not be run on one with an older implementation.

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-08 Thread Heikki Linnakangas
On 08/09/10 23:07, Martijn van Oosterhout wrote: On Mon, Sep 06, 2010 at 07:24:59PM +0200, Markus Wanner wrote: Do I understand correctly that the purpose of this patch is to work around the brokenness of select() on very few platforms? Or is there any additional feature that plain signals don't

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-08 Thread Martijn van Oosterhout
On Mon, Sep 06, 2010 at 07:24:59PM +0200, Markus Wanner wrote: > Do I understand correctly that the purpose of this patch is to work > around the brokenness of select() on very few platforms? Or is there any > additional feature that plain signals don't give us? If the issue is just that selec

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-08 Thread Tom Lane
Martijn van Oosterhout writes: > If the issue is just that select() doesn't get interrupted and we don't > care about a couple of syscalls, would it not be better to simply use > sigaction to turn on SA_RESTART just prior to the select() and turn it > off just after. Or are these systems so broken

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-08 Thread Tom Lane
Heikki Linnakangas writes: > On 08/09/10 20:36, Markus Wanner wrote: >> It should be possible to reliably determine the platforms that provide >> an atomic pselect(). For those, I'm hesitant to use a "trick", where >> pselect() clearly provides a simpler and more "official" alternative. >> Especia

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-08 Thread Heikki Linnakangas
On 08/09/10 20:36, Markus Wanner wrote: On 09/06/2010 11:03 PM, Tom Lane wrote: I don't entirely see the point of opening ourselves up to the risk of using a pselect that's not safe under the hood. It should be possible to reliably determine the platforms that provide an atomic pselect(). For

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-08 Thread Markus Wanner
Hi, On 09/06/2010 11:03 PM, Tom Lane wrote: I don't entirely see the point of opening ourselves up to the risk of using a pselect that's not safe under the hood. It should be possible to reliably determine the platforms that provide an atomic pselect(). For those, I'm hesitant to use a "trick

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-07 Thread Markus Wanner
On 09/07/2010 09:06 AM, Heikki Linnakangas wrote: Setting a latch that's already set is very fast, so you want to keep it set until the last moment. See the coding in walsender for example, it goes to some lengths to avoid clearing the latch until it's very sure there's no more work for it to do.

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-07 Thread Heikki Linnakangas
On 06/09/10 20:24, Markus Wanner wrote: On 09/06/2010 06:27 PM, Heikki Linnakangas wrote: + * It's important to reset the latch*before* checking if there's work to + * do. Otherwise, if someone sets the latch between the check and the + * ResetLatch call, you will miss it and Wait will block.

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-07 Thread Heikki Linnakangas
On 06/09/10 23:10, Markus Wanner wrote: Good. How about syscall overhead? One more write operation to the self-pipe per signal from within the signal handler and one read to actually clear the 'ready' state of the pipe from the waiter portion of the code, right? Right. Do we plan to replace a

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-06 Thread Tom Lane
Markus Wanner writes: > AFAICT the custom select() implementation we are using for Windows could > easily be changed to mimic pselect() instead. Thus most reasonably > up-to-date Linux distributions plus Windows certainly provide a workable > pselect() syscall. Would it be worth using pselect()

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-06 Thread Markus Wanner
On 09/06/2010 08:46 PM, Tom Lane wrote: Well, it's not defined in the Single Unix Spec, which is our customary reference for portability. FWIW, I bet the self-pipe trick isn't mentioned there, either... any good precedence that it actually works as expected on all of the target platforms? Exi

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-06 Thread Tom Lane
Markus Wanner writes: > Is pselect() really as unportable as stated in the patch? What platforms > have problems with pselect()? Well, it's not defined in the Single Unix Spec, which is our customary reference for portability. Also, it's alleged that some platforms have it but in a form that's

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-06 Thread Markus Wanner
Hi, On 09/06/2010 06:27 PM, Heikki Linnakangas wrote: Here's an updated patch, with all the issues reported this far fixed, except for that naming issue, and Fujii's suggestion to use poll() instead of select() where available. I've also polished it quite a bit, improving comments etc. Magnus, c

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-06 Thread Heikki Linnakangas
On 06/09/10 17:18, Tom Lane wrote: Heikki Linnakangas writes: I think we have just a terminology issue. What you're describing is exactly how it works now, if you just s/InitLatch/AcquireLatch. No, it isn't. What I'm suggesting requires breaking InitLatch into two operations. We also need

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-06 Thread Tom Lane
Heikki Linnakangas writes: > On 03/09/10 21:50, Tom Lane wrote: >> Well, in that case what we need to do is presume that the latch object >> has a continuing existence but the owner/receiver can come and go. >> I would suggest that InitLatch needs to initialize the object into a >> valid but unown

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-05 Thread Heikki Linnakangas
On 03/09/10 21:50, Tom Lane wrote: Heikki Linnakangas writes: On 03/09/10 21:16, Tom Lane wrote: It's probably not too unreasonable to assume that pid_t assignment is atomic. But I'm still thinking that we have bigger problems than that if there are really cases where SetLatch can execute at

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-05 Thread Simon Riggs
On Fri, 2010-09-03 at 18:24 -0400, Tom Lane wrote: > Now the HS case likewise appears to be set up so that the signal can > only directly interrupt ProcWaitForSignal, so I think the core issue > is > whether any deadlock situations are possible. Given that this gets > called from a low-level place

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-03 Thread Robert Haas
On Fri, Sep 3, 2010 at 6:24 PM, Tom Lane wrote: > Now the HS case likewise appears to be set up so that the signal can > only directly interrupt ProcWaitForSignal, so I think the core issue is > whether any deadlock situations are possible.  Given that this gets > called from a low-level place lik

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-03 Thread Tom Lane
Robert Haas writes: > Color me confused; I may need to backpedal rapidly here. I had > thought that what Tom was complaining about was the fact that the > signal handler was taking LWLocks, which I would have thought to be > totally unsafe. Well, it's unsafe if the signal could interrupt mainlin

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-03 Thread Tom Lane
Heikki Linnakangas writes: > A safer approach would be to just PGSemaphoreUnlock() in the signal > handler, and do all the other processing outside it. I don't see any particularly good reason to assume that PGSemaphoreUnlock is safe either: you're still talking about nested semop operations. T

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-03 Thread Robert Haas
On Fri, Sep 3, 2010 at 4:20 PM, Heikki Linnakangas wrote: > Maybe that's ok, if I'm reading the deadlock checker code correctly, it also > calls semop() to increment the another process' semaphore, and the deadlock > checker can be invoked from a signal handler while in semop() to wait on our > pr

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-03 Thread Heikki Linnakangas
On 03/09/10 19:38, Robert Haas wrote: On Fri, Sep 3, 2010 at 12:10 PM, Tom Lane wrote: Robert Haas writes: On Fri, Sep 3, 2010 at 10:07 AM, Tom Lane wrote: [ shrug... ] I stated before that the Hot Standby patch is doing utterly unsafe things in signal handlers. Simon rejected that. I am

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-03 Thread Robert Haas
On Fri, Sep 3, 2010 at 3:11 PM, Ron Mayer wrote: > Tom Lane wrote: >> Robert Haas writes: >>> On Fri, Sep 3, 2010 at 10:07 AM, Tom Lane wrote: [ shrug... ]  I stated before that the Hot Standby patch is doing utterly unsafe things in signal handlers.  Simon rejected that. I am wai

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-03 Thread Ron Mayer
Tom Lane wrote: > Robert Haas writes: >> On Fri, Sep 3, 2010 at 10:07 AM, Tom Lane wrote: >>> [ shrug... ] I stated before that the Hot Standby patch is doing >>> utterly unsafe things in signal handlers. Simon rejected that. >>> I am waiting for irrefutable evidence to emerge from the field >>

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-03 Thread Tom Lane
Heikki Linnakangas writes: > On 03/09/10 21:16, Tom Lane wrote: >> It's probably not too unreasonable to assume that pid_t assignment is >> atomic. But I'm still thinking that we have bigger problems than that >> if there are really cases where SetLatch can execute at approximately >> the same ti

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-03 Thread Heikki Linnakangas
On 03/09/10 21:16, Tom Lane wrote: Heikki Linnakangas writes: WaitLatch had to set the pid on the Latch struct to allow other processes to send the signal. Another process could call SetLatch and read the pid field, while WaitLatch is just setting it. I think we'll have to put a spinlock there,

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-03 Thread Tom Lane
Heikki Linnakangas writes: > On 03/09/10 17:51, Tom Lane wrote: >> If there is *any* possibility of that happening then you have far worse >> problems than whether the field is atomically readable or not: the >> behavior will be unpredictable at just slightly larger timescales. > Each Walsender n

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-03 Thread Heikki Linnakangas
On 03/09/10 17:51, Tom Lane wrote: Heikki Linnakangas writes: On 02/09/10 23:13, Tom Lane wrote: Also, using sig_atomic_t for owner_pid is entirely not sane. Hmm, true, it doesn't need to be set from signal handler, but is there an atomicity problem if one process calls ReleaseLatch while a

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-03 Thread Robert Haas
On Fri, Sep 3, 2010 at 12:10 PM, Tom Lane wrote: > Robert Haas writes: >> On Fri, Sep 3, 2010 at 10:07 AM, Tom Lane wrote: >>> [ shrug... ]  I stated before that the Hot Standby patch is doing >>> utterly unsafe things in signal handlers.  Simon rejected that. >>> I am waiting for irrefutable ev

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-03 Thread Tom Lane
Robert Haas writes: > On Fri, Sep 3, 2010 at 10:07 AM, Tom Lane wrote: >> [ shrug... ]  I stated before that the Hot Standby patch is doing >> utterly unsafe things in signal handlers.  Simon rejected that. >> I am waiting for irrefutable evidence to emerge from the field >> (and am very confiden

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-03 Thread Robert Haas
On Fri, Sep 3, 2010 at 10:07 AM, Tom Lane wrote: > Fujii Masao writes: >> On Fri, Sep 3, 2010 at 11:08 AM, Tom Lane wrote: >>> elog(FATAL) is *certainly* not a better idea.  I think there's really >>> nothing that can be done, you just have to silently ignore the error. > >> Hmm.. some functions

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-03 Thread Tom Lane
Heikki Linnakangas writes: > On 02/09/10 23:13, Tom Lane wrote: >>> (Yeah, I realize the caller >>> could look into the latch to find that out, but callers really ought to >>> treat latches as opaque structs.) > Hmm, maybe we need a TestLatch function to check if a latch is set. +1. It could be

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-03 Thread Tom Lane
Fujii Masao writes: > On Fri, Sep 3, 2010 at 11:08 AM, Tom Lane wrote: >> elog(FATAL) is *certainly* not a better idea.  I think there's really >> nothing that can be done, you just have to silently ignore the error. > Hmm.. some functions called by a signal handler use elog(FATAL), e.g., > Reco

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-03 Thread Heikki Linnakangas
On 02/09/10 23:13, Tom Lane wrote: The WaitLatch ...timeout API could use a bit of refinement. I'd suggest defining negative timeout as meaning wait forever, so that timeout = 0 can be used for "check but don't wait". Also, it seems like the function shouldn't just return void but should return

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-02 Thread Fujii Masao
On Fri, Sep 3, 2010 at 11:08 AM, Tom Lane wrote: > Fujii Masao writes: >>> +               * XXX: Is it safe to elog(ERROR) in a signal handler? >>> >>> No, it isn't. > >> We should use elog(FATAL) or check proc_exit_inprogress, instead? > > elog(FATAL) is *certainly* not a better idea.  I think

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-02 Thread Tom Lane
Fujii Masao writes: >> + * XXX: Is it safe to elog(ERROR) in a signal handler? >> >> No, it isn't. > We should use elog(FATAL) or check proc_exit_inprogress, instead? elog(FATAL) is *certainly* not a better idea. I think there's really nothing that can be done, you just have to s

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-02 Thread Fujii Masao
On Fri, Sep 3, 2010 at 5:13 AM, Tom Lane wrote: > Does ReleaseLatch() have any actual use-case, and if so what would it be? > I think we could do without it. In Unix, probably we can live without that. But in Windows, we need to free SharedEventHandles slot for upcoming process using a latch when

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-02 Thread Tom Lane
Heikki Linnakangas writes: > Ok, here's an updated patch with WaitLatchOrSocket that let's you do that. Minor code review items: s/There is three/There are three/ in unix_latch.c header comment The header comment points out the correct usage of ResetLatch, but I think it should also emphasize t

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-02 Thread Heikki Linnakangas
On 02/09/10 06:46, Fujii Masao wrote: On Wed, Sep 1, 2010 at 4:11 PM, Heikki Linnakangas wrote: The obvious next question is how to wait for multiple sockets and a latch at the same time? Perhaps we should have a select()-like interface where you can pass multiple file descriptors. Then again,

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-01 Thread Fujii Masao
On Wed, Sep 1, 2010 at 4:11 PM, Heikki Linnakangas wrote: > The obvious next question is how to wait for multiple sockets and a latch at > the same time? Perhaps we should have a select()-like interface where you > can pass multiple file descriptors. Then again, looking at the current > callers of

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-09-01 Thread Heikki Linnakangas
On 31/08/10 15:47, Fujii Masao wrote: On Fri, Aug 27, 2010 at 4:39 PM, Fujii Masao wrote: /* * XXX: Should we invent an API to wait for data coming from the * client connection too? It's not critical, but we could then * eliminate the timeout altogether and go to sleep for good. */ Ye

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-08-31 Thread Bruce Momjian
Tom Lane wrote: > Greg Smith writes: > > Tom Lane wrote: > >> Well, yes they are. They cause unnecessary process wakeups and thereby > >> consume cycles even when the database is idle. See for example a > >> longstanding complaint here: > >> https://bugzilla.redhat.com/show_bug.cgi?id=252129 >

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-08-31 Thread Fujii Masao
On Fri, Aug 27, 2010 at 4:39 PM, Fujii Masao wrote: >> /* >>  * XXX: Should we invent an API to wait for data coming from the >>  * client connection too? It's not critical, but we could then >>  * eliminate the timeout altogether and go to sleep for good. >>  */ > > Yes, it would be very helpful

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-08-31 Thread Fujii Masao
On Tue, Aug 31, 2010 at 4:06 PM, Heikki Linnakangas wrote: > Here's a 2nd version of the "latch" patch. Now with a Windows > implementation. Comments welcome. Seems good. Two minor comments: > rc = WaitForSingleObject(latch->event, timeout / 1000); > if (rc == WAIT_FAILED) > { > ereport(E

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-08-31 Thread Heikki Linnakangas
Here's a 2nd version of the "latch" patch. Now with a Windows implementation. Comments welcome. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com diff --git a/configure b/configure index bd9b347..432cd58 100755 --- a/configure +++ b/configure @@ -27773,6 +27773,13 @@ _ACEOF

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-08-28 Thread Greg Smith
Tom Lane wrote: Greg Smith writes: ... The only clear case where this is always a win is when the system it totally idle. If you'll climb down off that horse for a moment: yeah, the idle case is *exactly* what they're complaining about. I wasn't on a horse here--I saw the specific c

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-08-27 Thread Tom Lane
Greg Smith writes: > Tom Lane wrote: >> Well, yes they are. They cause unnecessary process wakeups and thereby >> consume cycles even when the database is idle. See for example a >> longstanding complaint here: >> https://bugzilla.redhat.com/show_bug.cgi?id=252129 > ... The only clear case whe

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-08-27 Thread Tom Lane
Robert Haas writes: > On Fri, Aug 27, 2010 at 5:54 PM, Greg Smith wrote: >> The way the background writer wakes up periodically to absorb fsync requests >> is already way too infrequent on a busy system. > Maybe instead of a fixed-duration sleep we could wake it up when it > needs to do somethin

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-08-27 Thread Robert Haas
On Fri, Aug 27, 2010 at 5:54 PM, Greg Smith wrote: > Tom Lane wrote: >> >> Well, yes they are.  They cause unnecessary process wakeups and thereby >> consume cycles even when the database is idle.  See for example a >> longstanding complaint here: >> https://bugzilla.redhat.com/show_bug.cgi?id=252

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-08-27 Thread Greg Smith
Tom Lane wrote: Well, yes they are. They cause unnecessary process wakeups and thereby consume cycles even when the database is idle. See for example a longstanding complaint here: https://bugzilla.redhat.com/show_bug.cgi?id=252129 If we're going to go to the trouble of having a mechanism like

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-08-27 Thread Heikki Linnakangas
On 27/08/10 10:39, Fujii Masao wrote: On Thu, Aug 26, 2010 at 7:40 PM, Heikki Linnakangas wrote: There's two kinds of latches, local and global. Local latches can only be set from the same process - allowing you to replace pg_usleep() with something that is always interruptible by signals (by

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-08-27 Thread Fujii Masao
On Thu, Aug 26, 2010 at 7:40 PM, Heikki Linnakangas wrote: > Here's a first attempt at implementing that. To demonstrate how it works, I > modified walsender to use the new latch facility, also to respond quickly to > SIGHUP and SIGTERM. Great! > There's two kinds of latches, local and global. L

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-08-26 Thread Heikki Linnakangas
On 24/08/10 02:44, Tom Lane wrote: Heikki Linnakangas writes: [ "latch" proposal ] This seems reasonably clean as far as signal conditions generated internally to Postgres go, but I remain unclear on how it helps for response to actual signals. You can can set the latch in the signal handle

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-08-24 Thread Heikki Linnakangas
On 24/08/10 04:08, Alvaro Herrera wrote: Excerpts from Tom Lane's message of lun ago 23 19:44:02 -0400 2010: Heikki Linnakangas writes: [ "latch" proposal ] This seems reasonably clean as far as signal conditions generated internally to Postgres go, but I remain unclear on how it helps for

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-08-23 Thread Alvaro Herrera
Excerpts from Tom Lane's message of lun ago 23 19:44:02 -0400 2010: > Heikki Linnakangas writes: > > [ "latch" proposal ] > > This seems reasonably clean as far as signal conditions generated > internally to Postgres go, but I remain unclear on how it helps for > response to actual signals. Thi

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-08-23 Thread Tom Lane
Heikki Linnakangas writes: > On 20/08/10 17:28, Tom Lane wrote: >> Well, yes they are. They cause unnecessary process wakeups and thereby >> consume cycles even when the database is idle. See for example a >> longstanding complaint here: >> https://bugzilla.redhat.com/show_bug.cgi?id=252129 >>

Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-08-23 Thread Heikki Linnakangas
On 20/08/10 17:28, Tom Lane wrote: [ It's way past time to change the thread title ] Heikki Linnakangas writes: On 20/08/10 16:24, Tom Lane wrote: You keep on proposing solutions that only work for walsender :-(. Well yes, the other places where we use pg_usleep() are not really a problem

Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-08-20 Thread Tom Lane
[ It's way past time to change the thread title ] Heikki Linnakangas writes: > On 20/08/10 16:24, Tom Lane wrote: >> You keep on proposing solutions that only work for walsender :-(. > Well yes, the other places where we use pg_usleep() are not really a > problem as is. Well, yes they are. Th

Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!

2010-08-20 Thread Heikki Linnakangas
On 20/08/10 16:24, Tom Lane wrote: Heikki Linnakangas writes: Hmm, we have pg_usleep() calls in some fairly low-level functions, like mdunlink() and s_lock(). If someone has called SetSleepInterrupt(), we don't want those pg_usleep()s to return immediately. And pg_usleep() is used in some clien

Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!

2010-08-20 Thread Tom Lane
Heikki Linnakangas writes: > Hmm, we have pg_usleep() calls in some fairly low-level functions, like > mdunlink() and s_lock(). If someone has called SetSleepInterrupt(), we > don't want those pg_usleep()s to return immediately. And pg_usleep() is > used in some client code too. I think we need

Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!

2010-08-20 Thread Heikki Linnakangas
On 19/08/10 20:59, Tom Lane wrote: Offhand I'd suggest something like SetSleepInterrupt() -- called by signal handlers, writes pipe ClearSleepInterrupt() -- called by sleep-and-do-something loops, clears pipe pg_usleep() itself remains the same, but it is now guaranteed to return immediat

Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!

2010-08-20 Thread tomas
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On Thu, Aug 19, 2010 at 08:36:09PM +0300, Heikki Linnakangas wrote: [...] > Hmm, will need to think about a suitable API for that. The nice thing would > be that we could implement it using pselect() where available. (And > reliable - the Linux sel

Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!

2010-08-19 Thread Tom Lane
Heikki Linnakangas writes: > On 19/08/10 20:18, Tom Lane wrote: >> But I'm still not seeing how this self-pipe hack avoids a race >> condition. If the signal handler is sending a byte whenever it >> executes, then you could have bytes already stacked up in the pipe >> from previous interrupts tha

Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!

2010-08-19 Thread Heikki Linnakangas
On 19/08/10 20:18, Tom Lane wrote: Heikki Linnakangas writes: On 19/08/10 19:57, Tom Lane wrote: Hmm, but couldn't you still do that inside pg_usleep? Signal handlers that do that couldn't know if they were interrupting a sleep per se, so this would have to be a backend-wide convention. I

Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!

2010-08-19 Thread Tom Lane
Heikki Linnakangas writes: > On 19/08/10 19:57, Tom Lane wrote: >> Hmm, but couldn't you still do that inside pg_usleep? Signal handlers >> that do that couldn't know if they were interrupting a sleep per se, >> so this would have to be a backend-wide convention. > I don't understand, do what in

Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!

2010-08-19 Thread Heikki Linnakangas
On 19/08/10 19:57, Tom Lane wrote: Heikki Linnakangas writes: On 19/08/10 16:38, Tom Lane wrote: Considering that pg_usleep is implemented with select, I'm not following what you mean by "replace pg_usleep() with select()"? Instead of using pg_usleep(), call select() directly, waiting not o

Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!

2010-08-19 Thread Tom Lane
Heikki Linnakangas writes: > On 19/08/10 16:38, Tom Lane wrote: >> Considering that pg_usleep is implemented with select, I'm not following >> what you mean by "replace pg_usleep() with select()"? > Instead of using pg_usleep(), call select() directly, waiting not only > for the timeout, but als

Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!

2010-08-19 Thread Heikki Linnakangas
On 19/08/10 16:38, Tom Lane wrote: Heikki Linnakangas writes: BTW, on what platforms signals don't interrupt sleep? Although that issue has been discussed many times before, I couldn't find any reference to a real platform in the archives. I've got one in captivity (my old HPUX box). Happy t

Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!

2010-08-19 Thread Heikki Linnakangas
On 19/08/10 18:08, Alvaro Herrera wrote: Excerpts from Heikki Linnakangas's message of jue ago 19 02:02:34 -0400 2010: On 19/08/10 04:46, Robert Haas wrote: And so far we haven't seen a patch for that. Somebody write one. And then let's get it reviewed and committed RSN. Fujii is on vaca

Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!

2010-08-19 Thread Magnus Hagander
On Thu, Aug 19, 2010 at 17:08, Alvaro Herrera wrote: > Excerpts from Heikki Linnakangas's message of jue ago 19 02:02:34 -0400 2010: >> On 19/08/10 04:46, Robert Haas wrote: > >> >  And so far we haven't seen a patch for that. >> > Somebody write one.  And then let's get it reviewed and committed

Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!

2010-08-19 Thread Alvaro Herrera
Excerpts from Heikki Linnakangas's message of jue ago 19 02:02:34 -0400 2010: > On 19/08/10 04:46, Robert Haas wrote: > > And so far we haven't seen a patch for that. > > Somebody write one. And then let's get it reviewed and committed RSN. > > Fujii is on vacation, but I've started working on

Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!

2010-08-19 Thread Tom Lane
Heikki Linnakangas writes: > BTW, on what platforms signals don't interrupt sleep? Although that > issue has been discussed many times before, I couldn't find any > reference to a real platform in the archives. I've got one in captivity (my old HPUX box). Happy to test whatever you come up wit

Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!

2010-08-18 Thread Heikki Linnakangas
On 19/08/10 04:46, Robert Haas wrote: At any rate, we should definitely NOT wait another month to start thinking about Sync Rep again. Agreed. EnterpriseDB is interested in having that feature, so I'm on the hook to spend time on it regardless of commitfests. I haven't actually looked at an

Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!

2010-08-18 Thread Robert Haas
On Wed, Aug 18, 2010 at 7:46 PM, Greg Smith wrote: > Kevin Grittner wrote: >> >> I don't think I want to try to handle two in a row, and I think your style >> is better suited >> than mine to the final CF for a release, but I might be able to take on >> the 2010-11 CF if people want that > > Ha, y

Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!

2010-08-18 Thread Greg Smith
Kevin Grittner wrote: I don't think I want to try to handle two in a row, and I think your style is better suited than mine to the final CF for a release, but I might be able to take on the 2010-11 CF if people want that Ha, you just put yourself right back on the hook with that comment, and

Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!

2010-08-18 Thread Kevin Grittner
Robert Haas wrote: > I'd just like to take a minute to thank him publicly for his > efforts. We started this CommitFest with something like 60 > patches, which is definitely on the larger side for a CommitFest, > and Kevin did a great job staying on top of what was going on with > all of them a

[HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!

2010-08-18 Thread Robert Haas
Kevin didn't send out an official gavel-banging announcement of the end of CommitFest 2009-07 (possibly because I neglected until today to give him privileges to actually change it in the web application), but I'd just like to take a minute to thank him publicly for his efforts. We started this Com