Re: Restart pg_usleep when interrupted

2024-07-12 Thread Sami Imseih
> What does your testing show when you don't have > the extra check, i.e., > > struct timespec delay; > struct timespec remain; > > delay.tv_sec = microsec / 100L; > delay.tv_nsec = (microsec % 100L) * 1000; > > while (nanosleep(, ) == -1 && errno ==

Re: Restart pg_usleep when interrupted

2024-07-12 Thread Nathan Bossart
On Fri, Jul 12, 2024 at 12:14:56PM -0500, Sami Imseih wrote: > 1/ TimestampDifference has a dependency on gettimeofday, > while my proposal utilizes clock_gettime. There are old discussions > that did not reach a conclusion comparing both mechanisms. > My main conclusion from these hacker

Re: Restart pg_usleep when interrupted

2024-07-12 Thread Sami Imseih
> > I'm imagining something like this: > >struct timespec delay; >TimestampTz end_time; > >end_time = TimestampTzPlusMilliseconds(GetCurrentTimestamp(), msec); > >do >{ >longsecs; >int microsecs; > >

Re: Restart pg_usleep when interrupted

2024-07-12 Thread Alvaro Herrera
On 2024-Jul-11, Nathan Bossart wrote: > I'm imagining something like this: > > struct timespec delay; > TimestampTz end_time; > > end_time = TimestampTzPlusMilliseconds(GetCurrentTimestamp(), msec); > > do > { > longsecs; > int microsecs; >

Re: Restart pg_usleep when interrupted

2024-07-12 Thread Bertrand Drouvot
Hi, On Thu, Jul 11, 2024 at 10:15:41AM -0500, Sami Imseih wrote: > > > I did a few tests with the patch and did not see any "large" drifts like the > > ones observed above. > > Thanks for testing. > > > I think it could be "simplified" by making use of instr_time instead of > > timespec > >

Re: Restart pg_usleep when interrupted

2024-07-11 Thread Nathan Bossart
On Thu, Jul 11, 2024 at 01:10:25PM -0500, Sami Imseih wrote: >> I'm curious why we wouldn't just subtract "elapsed_time" from "delay" at >> the bottom of the while loop to avoid needing this extra check. > > Can you elaborate further? I am not sure how this will work since delay is a >

Re: Restart pg_usleep when interrupted

2024-07-11 Thread Sami Imseih
> > I'm curious why we wouldn't just subtract "elapsed_time" from "delay" at > the bottom of the while loop to avoid needing this extra check. Can you elaborate further? I am not sure how this will work since delay is a timespec and elapsed time is an instr_time. Also, in every iteration

Re: Restart pg_usleep when interrupted

2024-07-11 Thread Nathan Bossart
+ /* +* We allow nanosleep to handle interrupts and retry with the remaining time. +* However, since nanosleep is susceptible to time drift when interrupted +* frequently, we add a safeguard to break out of the nanosleep whenever the

Re: Restart pg_usleep when interrupted

2024-07-11 Thread Sami Imseih
> > Correct I attached a v2 of this patch that uses instr_time to check the > elapsed > time and break out of the loop. It needs some more benchmarking. My email client has an issue sending attachments it seems. Reattaching v2-0001-vaccum_delay-with-absolute-time-nanosleep.patch Description:

Re: Restart pg_usleep when interrupted

2024-07-11 Thread Sami Imseih
I did a few tests with the patch and did not see any "large" drifts like theones observed above.Thanks for testing.I think it could be "simplified" by making use of instr_time instead of timespecfor current and absolute. Then I think it would be enough to compare theirticks.Correct I attached a v2

Re: Restart pg_usleep when interrupted

2024-07-09 Thread Bertrand Drouvot
Hi, On Fri, Jul 05, 2024 at 11:49:45AM -0500, Sami Imseih wrote: > > > With 50 indexes and 10 parallel workers I can see things like: > > > > 2024-07-02 08:22:23.789 UTC [2189616] LOG: expected 1.00, actual > > 239.378368 > > 2024-07-02 08:22:24.575 UTC [2189616] LOG: expected 0.10,

Re: Restart pg_usleep when interrupted

2024-07-05 Thread Sami Imseih
> > A more portable approach which could be to continue using nanosleep and > add checks to ensure that nanosleep exists whenever > it goes past an absolute time. This was suggested by Bertrand in an offline > conversation. I am not yet fully convinced of this idea, but posting the patch > that

Re: Restart pg_usleep when interrupted

2024-07-05 Thread Sami Imseih
With 50 indexes and 10 parallel workers I can see things like:2024-07-02 08:22:23.789 UTC [2189616] LOG:  expected 1.00, actual 239.3783682024-07-02 08:22:24.575 UTC [2189616] LOG:  expected 0.10, actual 224.3317372024-07-02 08:22:25.363 UTC [2189616] LOG:  expected 1.30, actual

Re: Restart pg_usleep when interrupted

2024-07-02 Thread Bertrand Drouvot
Hi, On Fri, Jun 28, 2024 at 05:50:20PM -0500, Sami Imseih wrote: > > Thanks for the feedback! > > > On Jun 28, 2024, at 4:34 PM, Tom Lane wrote: > > > > Sami Imseih writes: > >> Reattaching the patch. > > > > I feel like this is fundamentally a wrong solution, for the reasons > > cited in

Re: Restart pg_usleep when interrupted

2024-07-01 Thread Sami Imseih
> >> Therefore, rather than "improving" pg_usleep (and uglifying its API), >> the right answer is to fix parallel vacuum leaders to not depend on >> pg_usleep in the first place. A better idea might be to use >> pg_sleep() or equivalent code. > > Yes, that is a good idea to explore and it will

Re: Restart pg_usleep when interrupted

2024-06-28 Thread Sami Imseih
Thanks for the feedback! > On Jun 28, 2024, at 4:34 PM, Tom Lane wrote: > > Sami Imseih writes: >> Reattaching the patch. > > I feel like this is fundamentally a wrong solution, for the reasons > cited in the comment for pg_usleep: long sleeps are a bad idea > because of the resulting

Re: Restart pg_usleep when interrupted

2024-06-28 Thread Thomas Munro
On Sat, Jun 29, 2024 at 9:34 AM Tom Lane wrote: > Therefore, rather than "improving" pg_usleep (and uglifying its API), > the right answer is to fix parallel vacuum leaders to not depend on > pg_usleep in the first place. A better idea might be to use > pg_sleep() or equivalent code. In case

Re: Restart pg_usleep when interrupted

2024-06-28 Thread Tom Lane
Sami Imseih writes: > Reattaching the patch. I feel like this is fundamentally a wrong solution, for the reasons cited in the comment for pg_usleep: long sleeps are a bad idea because of the resulting uncertainty about whether we'll respond to interrupts and such promptly. An example here is

Re: Restart pg_usleep when interrupted

2024-06-28 Thread Sami Imseih
> We want patches to be in the pgsql-hackers archives, not temporarily > accessible via some link. > > …Robert > Moving to another email going forward. Reattaching the patch. 0001-Handle-Sleep-interrupts.patch Description: Binary data Regards, Sami Imseih Amazon Web Services

Re: Restart pg_usleep when interrupted

2024-06-28 Thread Imseih (AWS), Sami
> I think you need to find a way to disable this "Attachment protected > by Amazon" thing: Yes, I am looking into that. I only noticed after sending the email. Sorry about that. Sami

Re: Restart pg_usleep when interrupted

2024-06-28 Thread Robert Haas
Hi, I think you need to find a way to disable this "Attachment protected by Amazon" thing: http://postgr.es/m/01000190606e3d2a-116ead16-84d2-4449-8d18-5053da66b1f4-000...@email.amazonses.com We want patches to be in the pgsql-hackers archives, not temporarily accessible via some link.

Restart pg_usleep when interrupted

2024-06-28 Thread Imseih (AWS), Sami
Attachment protected by Amazon: [0001-Handle-Sleep-interrupts.patch] https://us-east-1.secure-attach.amazon.com/fcdc82ce-7887-4aa1-af9e-c1161a6b1d6f/bc81fa24-41de-48f9-a767-a6d15801754b Amazon has replaced attachment with download link. Downloads will be available until July 28, 2024, 19:59