public inbox for [email protected]  
help / color / mirror / Atom feed
Re: libpq v17 PQsocketPoll timeout is not granular enough
3+ messages / 3 participants
[nested] [flat]

* Re: libpq v17 PQsocketPoll timeout is not granular enough
@ 2024-06-10 16:18 Tom Lane <[email protected]>
  2024-06-10 18:43 ` Re: libpq v17 PQsocketPoll timeout is not granular enough Dominique Devienne <[email protected]>
  0 siblings, 1 reply; 3+ messages in thread

From: Tom Lane @ 2024-06-10 16:18 UTC (permalink / raw)
  To: Dominique Devienne <[email protected]>; +Cc: pgsql-general

Dominique Devienne <[email protected]> writes:
> PQsocketPoll() being based on time_t, it has only second resolution, AFAIK.
> Despite the [underlying implementation in fe-misc.c][2] supporting at
> least milliseconds.
> ...
> But I think it would a pity if that unreleased API couldn't be made to
> accomodate sub-second timeouts and more use-cases, like above.
> Especially given that libpq v17 isn't out yet. I may come late to the
> game, but hopefully it is not too late.

This is an interesting suggestion, but I think yes it's too late.
We're already past beta1 and this'd require some fairly fundamental
rethinking, since there's no easy substitute for type time_t that has
millisecond resolution.  (The callers do want to specify an end time
not a timeout interval, since some of them loop around PQsocketPoll
and don't want the end time to slip.)

I guess conceivably we could use gettimeofday() and struct timeval
instead of time() and time_t, but it'd touch a lot of places in
libpq and it'd make some of the calculations a lot more complex.

Maybe a better idea would be to convert to using our
src/include/portability/instr_time.h abstraction?  But that
would be problematic for outside callers.

In any case this doesn't seem like a sane thing to be redesigning
post-beta.  A few months ago maybe we'd have done it, but ...

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 3+ messages in thread

* Re: libpq v17 PQsocketPoll timeout is not granular enough
  2024-06-10 16:18 Re: libpq v17 PQsocketPoll timeout is not granular enough Tom Lane <[email protected]>
@ 2024-06-10 18:43 ` Dominique Devienne <[email protected]>
  2024-06-10 20:13   ` Re: libpq v17 PQsocketPoll timeout is not granular enough Adrian Klaver <[email protected]>
  0 siblings, 1 reply; 3+ messages in thread

From: Dominique Devienne @ 2024-06-10 18:43 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: pgsql-general

Bummer… I didn’t presume to suggest an api before, but simply adding an
extra int with the milliseconds offset from the time_t is simple, and
trivial to plug into the implementation I saw. Callers who don’t care can
simply pass zero. while I could pass a computed time_t and ms offset using
<chrono>. No need for fancy types imho. Aren’t betas precisely for the
purpose of exposing apis to those like myself to vet them? This is also
beta1, I,e, the first one. My €0.02

On Mon, Jun 10, 2024 at 6:18 PM Tom Lane <[email protected]> wrote:

> Dominique Devienne <[email protected]> writes:
> > PQsocketPoll() being based on time_t, it has only second resolution,
> AFAIK.
> > Despite the [underlying implementation in fe-misc.c][2] supporting at
> > least milliseconds.
> > ...
> > But I think it would a pity if that unreleased API couldn't be made to
> > accomodate sub-second timeouts and more use-cases, like above.
> > Especially given that libpq v17 isn't out yet. I may come late to the
> > game, but hopefully it is not too late.
>
> This is an interesting suggestion, but I think yes it's too late.
> We're already past beta1 and this'd require some fairly fundamental
> rethinking, since there's no easy substitute for type time_t that has
> millisecond resolution.  (The callers do want to specify an end time
> not a timeout interval, since some of them loop around PQsocketPoll
> and don't want the end time to slip.)
>
> I guess conceivably we could use gettimeofday() and struct timeval
> instead of time() and time_t, but it'd touch a lot of places in
> libpq and it'd make some of the calculations a lot more complex.
>
> Maybe a better idea would be to convert to using our
> src/include/portability/instr_time.h abstraction?  But that
> would be problematic for outside callers.
>
> In any case this doesn't seem like a sane thing to be redesigning
> post-beta.  A few months ago maybe we'd have done it, but ...
>
>                         regards, tom lane
>


^ permalink  raw  reply  [nested|flat] 3+ messages in thread

* Re: libpq v17 PQsocketPoll timeout is not granular enough
  2024-06-10 16:18 Re: libpq v17 PQsocketPoll timeout is not granular enough Tom Lane <[email protected]>
  2024-06-10 18:43 ` Re: libpq v17 PQsocketPoll timeout is not granular enough Dominique Devienne <[email protected]>
@ 2024-06-10 20:13   ` Adrian Klaver <[email protected]>
  0 siblings, 0 replies; 3+ messages in thread

From: Adrian Klaver @ 2024-06-10 20:13 UTC (permalink / raw)
  To: Dominique Devienne <[email protected]>; Tom Lane <[email protected]>; +Cc: pgsql-general

On 6/10/24 11:43, Dominique Devienne wrote:
> Bummer… I didn’t presume to suggest an api before, but simply adding an 
> extra int with the milliseconds offset from the time_t is simple, and 
> trivial to plug into the implementation I saw. Callers who don’t care 
> can simply pass zero. while I could pass a computed time_t and ms offset 
> using <chrono>. No need for fancy types imho. Aren’t betas precisely for 

https://www.postgresql.org/developer/beta/

"PostgreSQL beta and release candidate releases are pre-release testing 
versions before the community makes a new release generally available. 
They are feature-frozen (i.e. no new features are added), and we release 
these to the public for testing before our final release. PostgreSQL 
beta and release candidate release are not meant for use in production 
systems."


> the purpose of exposing apis to those like myself to vet them? This is 
> also beta1, I,e, the first one. My €0.02
> 

-- 
Adrian Klaver
[email protected]







^ permalink  raw  reply  [nested|flat] 3+ messages in thread


end of thread, other threads:[~2024-06-10 20:13 UTC | newest]

Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-06-10 16:18 Re: libpq v17 PQsocketPoll timeout is not granular enough Tom Lane <[email protected]>
2024-06-10 18:43 ` Dominique Devienne <[email protected]>
2024-06-10 20:13   ` Adrian Klaver <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox