Re: Improve the granularity of PQsocketPoll's timeout parameter?

Поиск
Список
Период
Сортировка
От Ranier Vilela
Тема Re: Improve the granularity of PQsocketPoll's timeout parameter?
Дата
Msg-id CAEudQAoS0XBqZND6v0HOOghVi=UvyQYhg-c7qtdeSmamJHz2Dg@mail.gmail.com
обсуждение исходный текст
Ответ на Improve the granularity of PQsocketPoll's timeout parameter?  (Tom Lane <tgl@sss.pgh.pa.us>)
Список 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?
Regarding your patch:

1. I think can remove *int64* in comments:
+ * The timeout is specified by end_time_us, which is the number of
+ * microseconds since the Unix epoch (that is, time_t times 1 million).
+ * Timeout is infinite if end_time is -1.  Timeout is immediate (no blocking)
+ * if end_time is 0 (or indeed, any time before now).

+ * The timeout is specified by end_time_us, which is the number of
+ * microseconds since the Unix epoch (that is, time_t times 1 million).

2. I think it's worth testing whether end_time_ns equals zero,
which can avoid a call to PQgetCurrentTimeNSec()

@@ -1103,14 +1113,16 @@ PQsocketPoll(int sock, int forRead, int forWrite, time_t end_time)
  input_fd.events |= POLLOUT;
 
  /* Compute appropriate timeout interval */
- if (end_time == ((time_t) -1))
+ if (end_time_ns == -1)
  timeout_ms = -1;
+ else if (end_time_ns == 0)
+ timeout_ms = 0;

3. I think it's worth testing whether end_time_ns equals zero,
which can avoid a call to PQgetCurrentTimeNSec() 

@@ -1138,17 +1150,29 @@ PQsocketPoll(int sock, int forRead, int forWrite, time_t end_time)
  FD_SET(sock, &except_mask);
 
  /* Compute appropriate timeout interval */
- if (end_time == ((time_t) -1))
+ if (end_time_ns == -1)
  ptr_timeout = NULL;
+ else if (end_time_ns == 0)
+ {
+ timeout.tv_sec = 0;
+ timeout.tv_usec = 0;
+
+ ptr_timeout = &timeout;
+ }

best regards,
Ranier Vilela

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Nathan Bossart
Дата:
Сообщение: Re: Remove dependence on integer wrapping
Следующее
От: Nathan Bossart
Дата:
Сообщение: Re: Track the amount of time waiting due to cost_delay