On Fri, Jan 14, 2022 at 7:30 PM Thomas Munro wrote:
> On Fri, Jan 14, 2022 at 4:35 PM Andres Freund wrote:
> > The more I think about it, the less I see why we *ever* need to re-arm the
> > latch in pq_check_connection() in this approach. pq_check_connection() is
> > only
> > used from from
On Fri, Jan 14, 2022 at 4:35 PM Andres Freund wrote:
> The more I think about it, the less I see why we *ever* need to re-arm the
> latch in pq_check_connection() in this approach. pq_check_connection() is only
> used from from ProcessInterrupts(), and there's plenty things inside
>
Hi,
On 2022-01-11 22:59:13 +1300, Thomas Munro wrote:
> I considered another idea we discussed: if we see a latch event, clear
> it and try again so that other events can be revealed (rince and
> repeat), but remember if that happens and set the latch at the end. I
> think that still requires
On Tue, Dec 14, 2021 at 11:50 PM Thomas Munro wrote:
> On Tue, Dec 14, 2021 at 11:18 AM Thomas Munro wrote:
> > Well, I was trying to avoid bikeshedding an API change just for a
> > hypothetical problem we could solve when the time comes (say, after
> > fixing the more egregious problems with
On Tue, Dec 14, 2021 at 11:18 AM Thomas Munro wrote:
> On Tue, Dec 14, 2021 at 7:53 AM Andres Freund wrote:
> > On 2021-12-13 17:51:00 +1300, Thomas Munro wrote:
> > > I tried that. It seems OK, and gets rid of the PG_FINALLY(), which is
> > > nice. Latches still have higher priority, and
On Tue, Dec 14, 2021 at 7:53 AM Andres Freund wrote:
> On 2021-12-13 17:51:00 +1300, Thomas Munro wrote:
> > I tried that. It seems OK, and gets rid of the PG_FINALLY(), which is
> > nice. Latches still have higher priority, and still have the fast
> > return if already set and you asked for
Hi,
On 2021-12-13 17:51:00 +1300, Thomas Munro wrote:
> On Sat, Dec 11, 2021 at 7:09 PM Thomas Munro wrote:
> > On Sat, Dec 11, 2021 at 6:11 PM Andres Freund wrote:
> > > Yuck. Is there really no better way to deal with this? What kind of
> > > errors is
> > > this trying to handle
On Mon, Dec 13, 2021 at 5:51 PM Thomas Munro wrote:
> [...] Everywhere else calls
> with nevents == 1, so that's hypothetical.
Erm, I forgot about ExecAppendAsyncEventWait(), so I'd have to update
the commit message on that point, but it's hard to worry too much
about that case -- it's creating
On Sat, Dec 11, 2021 at 7:09 PM Thomas Munro wrote:
> On Sat, Dec 11, 2021 at 6:11 PM Andres Freund wrote:
> > Yuck. Is there really no better way to deal with this? What kind of errors
> > is
> > this trying to handle transparently? Afaics this still changes when we'd
> > e.g. detect
On Sat, Dec 11, 2021 at 6:11 PM Andres Freund wrote:
> Yuck. Is there really no better way to deal with this? What kind of errors is
> this trying to handle transparently? Afaics this still changes when we'd
> e.g. detect postmaster death.
The problem is that WaitEventSetWait() only reports the
Hi,
On 2021-12-11 17:41:34 +1300, Thomas Munro wrote:
> --- a/src/backend/libpq/pqcomm.c
> +++ b/src/backend/libpq/pqcomm.c
> @@ -1959,22 +1959,36 @@ pq_settcpusertimeout(int timeout, Port *port)
> bool
> pq_check_connection(void)
> {
> -#if defined(POLLRDHUP)
> - /*
> - * POLLRDHUP
On Tue, Oct 12, 2021 at 3:10 AM Maksim Milyutin wrote:
> Good work. I have tested your patch on Linux and FreeBSD on three basic
> cases: client killing, cable breakdown (via manipulations with firewall)
> and silent closing client connection before completion of previously
> started query in
On 12.06.2021 07:24, Thomas Munro wrote:
On Fri, Apr 30, 2021 at 2:23 PM Thomas Munro wrote:
Here's something I wanted to park here to look into for the next
cycle: it turns out that kqueue's EV_EOF flag also has the right
semantics for this. That leads to the idea of exposing the event via
On Thu, Oct 7, 2021 at 8:43 PM Thomas Munro wrote:
> On Sat, Jun 12, 2021 at 8:31 PM Zhihong Yu wrote:
> > +#ifdef POLLRDHUP
> > + if ((cur_event->events & WL_SOCKET_CLOSED) &&
> > + (cur_pollfd->revents & (POLLRDHUP | errflags)))
> >
> > It seems the last condition
On Sat, Jun 12, 2021 at 8:31 PM Zhihong Yu wrote:
> +#ifdef POLLRDHUP
> + if ((cur_event->events & WL_SOCKET_CLOSED) &&
> + (cur_pollfd->revents & (POLLRDHUP | errflags)))
>
> It seems the last condition above should be written as:
>
> ((cur_pollfd->revents & POLLRDHUP) |
On Fri, Jun 11, 2021 at 9:24 PM Thomas Munro wrote:
> On Fri, Apr 30, 2021 at 2:23 PM Thomas Munro
> wrote:
> > Here's something I wanted to park here to look into for the next
> > cycle: it turns out that kqueue's EV_EOF flag also has the right
> > semantics for this. That leads to the idea
On Fri, Apr 30, 2021 at 2:23 PM Thomas Munro wrote:
> Here's something I wanted to park here to look into for the next
> cycle: it turns out that kqueue's EV_EOF flag also has the right
> semantics for this. That leads to the idea of exposing the event via
> the WaitEventSet API, and would the
On Sat, Apr 3, 2021 at 9:27 AM Thomas Munro wrote:
> Pushed! Thanks to all who contributed.
Here's something I wanted to park here to look into for the next
cycle: it turns out that kqueue's EV_EOF flag also has the right
semantics for this. That leads to the idea of exposing the event via
On Fri, Apr 2, 2021 at 6:18 PM tsunakawa.ta...@fujitsu.com
wrote:
> The patch looks committable to me.
I checked for performance impact compared to master with pgbench -S,
and didn't see any problem. I thought more about how to write a
decent race-free test but struggled with the lack of a good
From: Thomas Munro
> > Following PostmasterIsAlive(), maybe it's better to name it as
> pq_connection_is_alive() pq_client_is_alive(), or pq_frontend_is_alive() like
> the pqcomm.c's head comment uses the word frontend?
>
> I think it's OK, because it matches the name of the GUC. I'm more
On Fri, Apr 2, 2021 at 1:36 AM Bharath Rupireddy
wrote:
> Here's a minor comment: it would be good if we have an extra line
> after variable assignments, before and after function calls/if
> clauses, something like
Done in v11. Thanks.
On Thu, Apr 1, 2021 at 10:16 PM tsunakawa.ta...@fujitsu.com
wrote:
> From: Thomas Munro
> > I changed my mind. Let's commit the pleasingly simple Linux-only feature
> > for
> > now, and extend to it to send some kind of no-op message in a later release.
> > So this is the version I'd like to
On Thu, Apr 1, 2021 at 11:29 AM Thomas Munro wrote:
>
> On Tue, Mar 30, 2021 at 10:00 AM Thomas Munro wrote:
> > If we want to ship this in v14 we have to make a decision ASAP:
> >
> > 1. Ship the POLLHUP patch (like v9) that only works reliably on
> > Linux. Maybe disable the feature
From: Thomas Munro
> I changed my mind. Let's commit the pleasingly simple Linux-only feature for
> now, and extend to it to send some kind of no-op message in a later release.
> So this is the version I'd like to go with.
> Objections?
+1, as some of our users experienced the problem that the
On Tue, Mar 30, 2021 at 10:00 AM Thomas Munro wrote:
> If we want to ship this in v14 we have to make a decision ASAP:
>
> 1. Ship the POLLHUP patch (like v9) that only works reliably on
> Linux. Maybe disable the feature completely on other OSes?
> 2. Ship the patch that tries to read (like
On Tue, Mar 30, 2021 at 6:25 AM Maksim Milyutin wrote:
>
> Hi Thomas! Thanks for working on this patch.
>
> I have attached a new version with some typo corrections of doc entry,
> removing of redundant `include` entries and trailing whitespaces. Also I
> added in doc the case when single query
Hi Thomas! Thanks for working on this patch.
I have attached a new version with some typo corrections of doc entry,
removing of redundant `include` entries and trailing whitespaces. Also I
added in doc the case when single query transaction with disconnected
client might be eventually
Hi,
On 2021-03-24 16:08:13 +1300, Thomas Munro wrote:
> ... Andres just asked me the same question, when we were discussing
> the pq_peekmessage() patch (v7). I had remembered that POLLHUP didn't
> work for this type of thing, from some earlier attempt at something
> similar, and indeed on my
Going back a couple of years to something Konstantin said:
On Sat, Aug 3, 2019 at 4:40 AM Konstantin Knizhnik
wrote:
> But I wonder why we can not perform just pool with POLLOUT flag and zero
> timeout.
> If OS detected closed connection, it should return POLLHUP, should not it?
> I am not sure
Hi,
In the description:
Provide a new optional GUC that can be used to check whether the client
connection has gone away periodically while running very long queries.
I think moving 'periodically' to the vicinity of 'to check' would make the
sentence more readable.
+the operating
On Tue, Mar 23, 2021 at 11:47 PM Thomas Munro wrote:
> That leaves the thorny problem Tom mentioned at the top of this
> thread[1]: this socket-level approach can be fooled by an 'X' sitting
> in the socket buffer, if a client that did PQsendQuery() and then
> PQfinish(). Or perhaps even by SSL
On Mon, Mar 22, 2021 at 3:29 PM Thomas Munro wrote:
> 2. The tests need tightening up. The thing with the "sleep 3" will
> not survive contact with the build farm, and I'm not sure if the SSL
> test is as short as it could be.
I don't think the TAP test can be done in the way Sergey had it,
On Sat, Mar 6, 2021 at 5:50 PM Zhihong Yu wrote:
> + if (client_connection_check_interval > 0)
> + enable_timeout_after(CLIENT_CONNECTION_CHECK_TIMEOUT,
>
> + /* Start timeout for checking if the client has gone away if necessary. */
> + if (client_connection_check_interval)
For v4-0002-some-fixups.patch :
+ if (client_connection_check_interval > 0)
+ enable_timeout_after(CLIENT_CONNECTION_CHECK_TIMEOUT,
+ /* Start timeout for checking if the client has gone away if necessary.
*/
+ if (client_connection_check_interval)
It would be better if the
On Mon, Mar 1, 2021 at 6:18 PM Thomas Munro wrote:
> I've done a quick rebase of this the patch and added it to the
> commitfest. No other changes. Several things were mentioned earlier
> that still need to be tidied up.
Rebased again due to bitrot. This time I did some actual work:
1. I
On Sat, Aug 3, 2019 at 4:40 AM Konstantin Knizhnik
wrote:
> On 18.07.2019 6:19, Tatsuo Ishii wrote:
> > So the performance is about 5% down with the feature enabled in this
> > case. For me, 5% down is not subtle. Probably we should warn this in
> > the doc.
>
> I also see some performance
On 18.07.2019 6:19, Tatsuo Ishii wrote:
I noticed that there are some confusions in the doc and code regarding what the
new
configuration parameter means. According to the doc:
+Default value is zero - it disables connection
+checks, so the backend will detect client
On Thu, Jul 18, 2019 at 5:04 PM Tom Lane wrote:
> Tatsuo Ishii writes:
> >> Yeah, the timer logic is wrong. I didn't have time to look into it
> >> but with truss/strace for some reason I see 3 setitimer() syscalls for
> >> every query, but I think this doesn't even need to set the timer for
>
Tatsuo Ishii writes:
>> Yeah, the timer logic is wrong. I didn't have time to look into it
>> but with truss/strace for some reason I see 3 setitimer() syscalls for
>> every query, but I think this doesn't even need to set the timer for
>> every query.
> Hum. I see 2 settimer(), instead of 3.
> Yeah, the timer logic is wrong. I didn't have time to look into it
> but with truss/strace for some reason I see 3 setitimer() syscalls for
> every query, but I think this doesn't even need to set the timer for
> every query.
Hum. I see 2 settimer(), instead of 3.
Best regards,
--
Tatsuo
On Thu, Jul 18, 2019 at 3:19 PM Tatsuo Ishii wrote:
> So the performance is about 5% down with the feature enabled in this
> case. For me, 5% down is not subtle. Probably we should warn this in
> the doc.
Yeah, the timer logic is wrong. I didn't have time to look into it
but with truss/strace
> Yeah.
>
> +1 for this patch, with a few adjustments including making the test
> use pg_sleep() as mentioned. It does something useful, namely
> cancelling very long running queries sooner if the client has gone
> away instead of discovering that potentially much later when sending a
>
On Sat, Jul 6, 2019 at 12:27 AM Stas Kelvich wrote:
> Well, indeed in case of cable disconnect only way to detect it with
> proposed approach is to have tcp keepalive. However if disconnection
> happens due to client application shutdown then client OS should itself
> properly close than
> On 5 Jul 2019, at 11:46, Thomas Munro wrote:
>
> On Fri, Jul 5, 2019 at 6:28 PM Tatsuo Ishii wrote:
>>> The purpose of this patch is to stop the execution of continuous
>>> requests in case of a disconnection from the client.
>>
>> Pgpool-II already does this by sending a parameter status
On Fri, Jul 5, 2019 at 6:28 PM Tatsuo Ishii wrote:
> > The purpose of this patch is to stop the execution of continuous
> > requests in case of a disconnection from the client.
>
> Pgpool-II already does this by sending a parameter status message to
> the client. It is expected that clients are
On Fri, Jul 5, 2019 at 6:42 PM Tatsuo Ishii wrote:
> > This seems like a reasonable idea to me. There is no point in running
> > a monster 24 hour OLAP query if your client has gone away. It's using
> > MSG_PEEK which is POSIX, and I can't immediately think of any reason
> > why it's not safe
> This seems like a reasonable idea to me. There is no point in running
> a monster 24 hour OLAP query if your client has gone away. It's using
> MSG_PEEK which is POSIX, and I can't immediately think of any reason
> why it's not safe to try to peek at a byte in that socket at any time.
I am
> The purpose of this patch is to stop the execution of continuous
> requests in case of a disconnection from the client.
Pgpool-II already does this by sending a parameter status message to
the client. It is expected that clients are always prepared to receive
the parameter status message. This
On Fri, Jul 5, 2019 at 5:36 PM Thomas Munro wrote:
> On Sat, Feb 9, 2019 at 6:16 AM wrote:
> > The purpose of this patch is to stop the execution of continuous
> > requests in case of a disconnection from the client. In most cases, the
> > client must wait for a response from the server before
On Sat, Feb 9, 2019 at 6:16 AM wrote:
> The purpose of this patch is to stop the execution of continuous
> requests in case of a disconnection from the client. In most cases, the
> client must wait for a response from the server before sending new data
> - which means there should not remain
The purpose of this patch is to stop the execution of continuous
requests in case of a disconnection from the client. In most cases, the
client must wait for a response from the server before sending new data
- which means there should not remain unread data on the socket and we
will be able
Hi,
On 2019-01-13 18:05:39 -0500, Tom Lane wrote:
> s.cherkas...@postgrespro.ru writes:
> > This patch adds verification of the connection with the client during
> > the execution of the SQL query. The feature enables using the GUC
> > variable ‘client_connection_check_interval’. The default
s.cherkas...@postgrespro.ru writes:
> This patch adds verification of the connection with the client during
> the execution of the SQL query. The feature enables using the GUC
> variable ‘client_connection_check_interval’. The default check interval
> is 1 second. If you set the value of
This patch adds verification of the connection with the client during
the execution of the SQL query. The feature enables using the GUC
variable ‘client_connection_check_interval’. The default check interval
is 1 second. If you set the value of ‘client_connection_check_interval’
to 0, then the
54 matches
Mail list logo
