Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3774.600.62\))
Subject: Re: Restart pg_usleep when interrupted
From: Sami Imseih <samimseih@gmail.com>
In-Reply-To: <ZqNd7WfTm3sUCpjy@ip-10-97-1-34.eu-west-3.compute.internal>
Date: Mon, 29 Jul 2024 18:15:49 -0500
Cc: Nathan Bossart <nathandbossart@gmail.com>,
 Tom Lane <tgl@sss.pgh.pa.us>,
 Robert Haas <robertmhaas@gmail.com>,
 "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <33287B8A-3CA0-494F-87D3-C9D85C6468FE@gmail.com>
References: <C18017A1-EDFD-4F2F-BCA1-0572D4CCC92B@gmail.com>
 <Zo_7blmO166ZDGbW@nathan> <2F781744-69B8-4BC2-A6EF-3998DCCDB634@gmail.com>
 <ZpA4_Pw14FCP6025@nathan> <01A15AEA-C35C-41DF-8E81-3B5A0B523939@gmail.com>
 <ZpGBjZZtbDGUtvTV@nathan> <521A6F3B-77E1-4F3F-B15E-BCF26D52C698@gmail.com>
 <ZpVaXQGEspp3gk8S@nathan>
 <ZpX/5/irF/gZqT7b@ip-10-97-1-34.eu-west-3.compute.internal>
 <CDF34F36-3A52-47C8-B1D3-5D32A34EC3A7@gmail.com>
 <ZqNd7WfTm3sUCpjy@ip-10-97-1-34.eu-west-3.compute.internal>
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/33287B8A-3CA0-494F-87D3-C9D85C6468FE%40gmail.com>
Precedence: bulk


> On Jul 26, 2024, at 3:27=E2=80=AFAM, Bertrand Drouvot =
<bertranddrouvot.pg@gmail.com> wrote:
>=20
> Hi,
>=20
> On Thu, Jul 25, 2024 at 05:27:15PM -0500, Sami Imseih wrote:
>> I am attaching v3 of the patch which addresses the comments made
>> earlier by Bertrand about the comment in the patch [1].
>=20
> Thanks!
>=20
> Looking at it:
>=20
> 1 =3D=3D=3D
>=20
> +       struct instr_time start_time;
>=20
> I think we can get rid of the "struct" keyword here.
>=20
> 2 =3D=3D=3D
>=20
> +           struct instr_time current_time;
> +           struct instr_time elapsed_time;
>=20
> Same as above.

Will fix those 2.

>=20
> 3 =3D=3D=3D
>=20
> I gave more thoughts and I think it can be simplified a bit to reduce =
the
> number of operations in the while loop.
>=20
> What about relying on a "absolute" time that way:
>=20
> 	instr_time absolute;
>    absolute.ticks =3D start_time.ticks + msec * 1000000;
>=20
> and then in the while loop:
>=20
>    while (nanosleep(&delay, &remain) =3D=3D -1 && errno =3D=3D EINTR)
>    {
>        instr_time current_time;
>        INSTR_TIME_SET_CURRENT(current_time);
>=20
>        if (current_time.ticks > absolute.ticks)
>        {
>            break;

While I agree this code is cleaner, myy hesitation there is we don=E2=80=99=
t=20
have any other place in which we access .ticks directly and the=20
common practice is to use the intsr_time.h APIs.


What do you think?


Regards,

Sami=20