Thread: TODO-item: Add sleep() function, remove from regress.c
I propose the appended patch for the todo item: * Add sleep() function, remove from regress.c The patch also removes do_sleep() from the regression and substitues this with the call to the new function. I'd personally prefer to call the function pg_sleep(), but since it is called sleep() on the TODO list and in previous discussions, I kept the name. The internal function is called pg_sleep() however. There was a discussion about this item in August of last year http://archives.postgresql.org/pgsql-hackers/2005-08/msg00633.php A few people liked the idea, Tom Lane pointed out that it's bad to have a sleeping backend that can hold locks for a long time. Other developers answered that this should just be documented since the user can do that anyways very easily if he is allowed to create functions in one of the traditional scripting languages or with busy waiting also in plpgsql. Finally the item got added to the TODO list and nobody objected. In the mysqlcompat project there is a sleep function that does busy waiting, this function could profit from a sleep() function in the backend. To the docs I introduced a new paragraph such that it is easy to find this function and labeled the note about the locks with the "warning" tag. Tom, can you live with that? Other Comments? Joachim
Attachment
Joachim Wieland <joe@mcknight.de> writes:
> I'd personally prefer to call the function pg_sleep(), but since it is
> called sleep() on the TODO list and in previous discussions, I kept the
> name. The internal function is called pg_sleep() however.
pg_sleep seems like a better idea to me too.
Why is the function defined to take numeric rather than float8?
float8 is a whole lot easier to work with internally. (Performance
doesn't seem like an issue here, but length and readability of the code
are worth worrying about.) Further, you could avoid assuming that the
machine has working int64 arithmetic, which is an assumption I still
think we should avoid everywhere that it's not absolutely essential.
The proposed regression test seems unacceptably fragile, as well as
rather pointless.
regards, tom lane
BTW, a serious problem with just passing it off to pg_usleep like that
is that the sleep can't be aborted by a cancel request; doesn't seem
like a good idea to me. I'd suggest writing a loop that sleeps for at
most a second at a time, with a CHECK_FOR_INTERRUPTS() before each
sleep. This would also allow you to eliminate the arbitrary restriction
on length of sleep.
regards, tom lane
On Mon, Jan 09, 2006 at 08:50:36PM -0500, Tom Lane wrote: > pg_sleep seems like a better idea to me too. ok, renamed again. > Why is the function defined to take numeric rather than float8? > float8 is a whole lot easier to work with internally. ok, changed. > The proposed regression test seems unacceptably fragile, as well as > rather pointless. Why is it fragile? Which other regression test do you suggest? Or should it go without one? On Mon, Jan 09, 2006 at 08:54:49PM -0500, Tom Lane wrote: > BTW, a serious problem with just passing it off to pg_usleep like that > is that the sleep can't be aborted by a cancel request No, cancelling the sleep works (at least for Unix). Isn't cancelling implemented via a signal that interrupts select() ? Anyway, I've changed it, removing the ~2000s limit is a good point. I append a new version with the regression test ripped out. Joachim
Attachment
Joachim Wieland <joe@mcknight.de> writes:
> On Mon, Jan 09, 2006 at 08:50:36PM -0500, Tom Lane wrote:
>> The proposed regression test seems unacceptably fragile, as well as
>> rather pointless.
> Why is it fragile?
As your own comment pointed out, the test would "fail" on a heavily
loaded system, due to sleeping too long. I do not see the need for
such a test anyway --- the stats regression test will exercise the
code sufficiently.
>> BTW, a serious problem with just passing it off to pg_usleep like that
>> is that the sleep can't be aborted by a cancel request
> No, cancelling the sleep works (at least for Unix). Isn't cancelling
> implemented via a signal that interrupts select() ?
On some systems the signal will interrupt select. On some, not.
Note the coding in bgwriter.c's main loop for instance.
regards, tom lane
On Tue, Jan 10, 2006 at 11:31:13AM +0100, Joachim Wieland wrote:
> No, cancelling the sleep works (at least for Unix). Isn't cancelling
> implemented via a signal that interrupts select() ?
>
> Anyway, I've changed it, removing the ~2000s limit is a good point.
> + while (secs > 1.0)
> + {
> + pg_usleep(1000000);
> + CHECK_FOR_INTERRUPTS();
> + secs -= 1.0;
> + }
Won't this result in a call to pg_sleep with a long sleep time ending up
sleeping noticeably longer than requested?
--
Jim C. Nasby, Sr. Engineering Consultant jnasby@pervasive.com
Pervasive Software http://pervasive.com work: 512-231-6117
vcard: http://jim.nasby.net/pervasive.vcf cell: 512-569-9461
Jim C. Nasby wrote:
>On Tue, Jan 10, 2006 at 11:31:13AM +0100, Joachim Wieland wrote:
>
>
>>No, cancelling the sleep works (at least for Unix). Isn't cancelling
>>implemented via a signal that interrupts select() ?
>>
>>Anyway, I've changed it, removing the ~2000s limit is a good point.
>>+ while (secs > 1.0)
>>+ {
>>+ pg_usleep(1000000);
>>+ CHECK_FOR_INTERRUPTS();
>>+ secs -= 1.0;
>>+ }
>>
>>
>
>Won't this result in a call to pg_sleep with a long sleep time ending up
>sleeping noticeably longer than requested?
>
>
Looks like it to me.
Instead of using a counter it might be better to set a target end time
and check that against the value returned from time() or gettimeofday().
cheers
andrew
Andrew Dunstan <andrew@dunslane.net> writes:
> Jim C. Nasby wrote:
>> Won't this result in a call to pg_sleep with a long sleep time ending up
>> sleeping noticeably longer than requested?
> Looks like it to me.
Something on the order of 1% longer, hm? (1 extra clock tick per second,
probably.) Can't get excited about it --- *all* implementations of sleep
say that the time is minimum not exact.
regards, tom lane
Tom Lane said: > Andrew Dunstan <andrew@dunslane.net> writes: >> Jim C. Nasby wrote: >>> Won't this result in a call to pg_sleep with a long sleep time ending >>> up sleeping noticeably longer than requested? > >> Looks like it to me. > > Something on the order of 1% longer, hm? (1 extra clock tick per > second, probably.) Can't get excited about it --- *all* > implementations of sleep say that the time is minimum not exact. > Well yes, although it's cumulative. I guess I'm not excited for a different reason - I'm having trouble imagining much of a use case. cheers andrew
Joachim Wieland <joe@mcknight.de> writes:
> I append a new version with the regression test ripped out.
Applied with revisions. I concluded that the idea of computing the
end-time in advance had merit, so I changed the code to do it that way.
Aside from not allowing extra delay to accumulate in a multiple-second
wait, this guarantees that we cannot wait *less* than the requested
time, which is a hazard if the system has interruptible select().
Other minor comments: if you declare the function strict it is not
necessary to check for null inputs, and you were doing the float-to-int
conversion the hard way.
regards, tom lane