Re: Improve the granularity of PQsocketPoll's timeout parameter? - Mailing list pgsql-hackers

From Ranier Vilela
Subject Re: Improve the granularity of PQsocketPoll's timeout parameter?
Date
Msg-id CAEudQAo_rO5rGB=3kLaPPpBhgSsM1JOTAzhcHS66WiFyFma6bw@mail.gmail.com
Whole thread Raw
In response to Improve the granularity of PQsocketPoll's timeout parameter?  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers


Em seg., 10 de jun. de 2024 às 18:39, Tom Lane <tgl@sss.pgh.pa.us> escreveu:
In [1] Dominique Devienne complained that PQsocketPoll would be
far more useful to him if it had better-than-one-second timeout
resolution.  I initially pushed back on that on the grounds that
post-beta1 is a bit late to be redefining public APIs.  Which it is,
but if we don't fix it now then we'll be stuck supporting that API
indefinitely.  And it's not like one-second resolution is great
for our internal usage either --- for example, I see that psql
is now doing

                end_time = time(NULL) + 1;
                rc = PQsocketPoll(sock, forRead, !forRead, end_time);

which claims to be waiting one second, but actually it's waiting
somewhere between 0 and 1 second.  So I thought I'd look into
whether we can still change it without too much pain, and I think
we can.

The $64 question is how to represent the end_time if not as time_t.
The only alternative POSIX offers AFAIK is gettimeofday's "struct
timeval", which is painful to compute with and I don't think it's
native on Windows.  What I suggest is that we use int64 microseconds
since the epoch, which is the same idea as the backend's TimestampTz
except I think we'd better use the Unix epoch not 2000-01-01.
Then converting code is just a matter of changing variable types
and adding some zeroes to constants.

The next question is how to spell "int64" in libpq-fe.h.  As a
client-exposed header, the portability constraints on it are pretty
stringent, so even in 2024 I'm loath to make it depend on <stdint.h>;
and certainly depending on our internal int64 typedef won't do.
What I did in the attached is to write "long long int", which is
required to be at least 64 bits by C99.  Other opinions are possible
of course.

Lastly, we need a way to get current time in this form.  My first
draft of the attached patch had the callers calling gettimeofday
and doing arithmetic from that, but it seems a lot better to provide
a function that just parallels time(2).

BTW, I think this removes the need for libpq-fe.h to #include <time.h>,
but I didn't remove that because it seems likely that some callers are
indirectly relying on it to be present.  Removing it wouldn't gain
very much anyway.

Thoughts?
Hi Tom.

Why not use uint64?
I think it's available in (fe-misc.c)

IMO, gettimeofday It also seems to me that it is deprecated.

Can I suggest a version using *clock_gettime*, 
which I made based on versions available on the web?

/*
 * PQgetCurrentTimeUSec: get current time with nanosecond precision
 *
 * This provides a platform-independent way of producing a reference
 * value for PQsocketPoll's timeout parameter.
 */

uint64
PQgetCurrentTimeUSec(void)
{
#ifdef __MACH__
struct timespec ts;
clock_serv_t cclock;
mach_timespec_t mts;

host_get_clock_service(mach_host_self(), SYSTEM_CLOCK, &cclock);
clock_get_time(cclock, &mts);
mach_port_deallocate(mach_task_self(), cclock);
ts.tv_sec = mts.tv_sec;
ts.tv_nsec = mts.tv_nsec;
#eldef _WIN32_
struct timespec ts { long tv_sec; long tv_nsec; };
__int64 wintime;

GetSystemTimeAsFileTime((FILETIME*) &wintime);
wintime   -= 116444736000000000i64;             // 1jan1601 to 1jan1970
ts.tv_sec  = wintime / 10000000i64;             // seconds
ts.tv_nsec = wintime % 10000000i64 * 100;      // nano-seconds
#else
struct timespec ts;

clock_gettime(CLOCK_MONOTONIC, &ts);
#endif

return (ts.tv_sec * 1000000000L) + ts.tv_nsec;
}

best regards,
Ranier Vilela

pgsql-hackers by date:

Previous
From: Alexander Kukushkin
Date:
Subject: Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
Next
From: Ashutosh Sharma
Date:
Subject: Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions