Jelte Fennema-Nio writes:
> On Fri, 22 Nov 2024 at 01:37, Tom Lane wrote:
>> How would we do that? libpqsrv_cancel is not chartered to wait around
>> for the results of the cancel, and I'm not even sure that it could
>> know what to check for.
> Ah yeah, you're right. I got confused by the two
On Fri, 22 Nov 2024 at 01:37, Tom Lane wrote:
> How would we do that? libpqsrv_cancel is not chartered to wait around
> for the results of the cancel, and I'm not even sure that it could
> know what to check for.
Ah yeah, you're right. I got confused by the two timeouts (the one to
wait for the
Jelte Fennema-Nio writes:
> On Thu, 21 Nov 2024 at 02:31, Tom Lane wrote:
>> This seems to fix the problem here. Thoughts?
> Overall, a good approach to fix issue number 1. I think it would be
> best if this was integrated into libpqsrv_cancel instead though. That
> way the dblink would benefit
On Thu, 21 Nov 2024 at 02:31, Tom Lane wrote:
> Anyway, given that info, Jelte's unapplied 0002 patch earlier in the
> thread is not the answer, because this is about dropping a query
> cancel not about losing a timeout interrupt.
Agreed that 0002 does not fix the issue re-reported by Andres (let
Alexander Lakhin writes:
> 17.11.2024 05:33, Tom Lane wrote:
>> Yeah. This has been happening off-and-on in the buildfarm ever
>> since we added that test. I'm not sure if it's just "the test
>> is unstable" or if it's telling us there's a problem with the
>> cancel logic. Scraping the last 3 m
Hi,
On 2024-10-01 15:00:00 +0300, Alexander Lakhin wrote:
> Hello Tom and Jelte,
>
> 31.08.2024 07:04, Alexander Lakhin wrote:
> > I've tested your fix with the modification I proposed upthread:
> > idle_session_timeout_enabled = false;
> > }
> > +if (rand() % 10 == 0) pg_us
Hello Tom and Andres,
17.11.2024 05:33, Tom Lane wrote:
Andres Freund writes:
Another failure in CI, that cleared up after a retry:
+WARNING: could not get result of cancel request due to timeout
Yeah. This has been happening off-and-on in the buildfarm ever
since we added that test. I'm
Andres Freund writes:
> On 2024-10-01 15:00:00 +0300, Alexander Lakhin wrote:
>> One month later, treehopper has found a way to break that test: [1].
>> The failure log contains:
>> 2024-09-30 19:35:01.485 UTC [3201033:12] pg_regress/query_cancel WARNING:
>> could not get result of cancel request
Hello Tom and Jelte,
31.08.2024 07:04, Alexander Lakhin wrote:
I've tested your fix with the modification I proposed upthread:
idle_session_timeout_enabled = false;
}
+if (rand() % 10 == 0) pg_usleep(1);
/*
* (5) disable async signal conditions again.
On Sat, 31 Aug 2024 at 06:04, Alexander Lakhin wrote:
> At the same time, mylodon confirmed my other finding at [1] and failed [2]
> with:
> -ERROR: canceling statement due to statement timeout
> +ERROR: canceling statement due to user request
>
> [1]
> https://www.postgresql.org/message-id/4d
Hello Tom,
30.08.2024 23:55, Tom Lane wrote:
Pushed with that addition and some comment-tweaking. We'll see
whether that actually makes things more stable, but I don't think
it could make it worse.
Thank you for fixing that issue!
I've tested your fix with the modification I proposed upthrea
On Fri, 30 Aug 2024 at 22:12, Tom Lane wrote:
>
> Jelte Fennema-Nio writes:
> > On Fri, Aug 30, 2024, 21:21 Tom Lane wrote:
> >> While we're piling on, has anyone noticed that *non* Windows buildfarm
> >> animals are also failing this test pretty frequently?
>
> > Yes. Fixes are here (see the ~1
I wrote:
> Hmm. I'm not convinced that 0001 is an actual *fix*, but it should
> at least reduce the frequency of occurrence a lot, which'd help.
After enabling log_statement = all to verify what commands are being
sent to the remote, I realized that there's a third thing this patch
can do to stab
Jelte Fennema-Nio writes:
> On Fri, Aug 30, 2024, 21:21 Tom Lane wrote:
>> While we're piling on, has anyone noticed that *non* Windows buildfarm
>> animals are also failing this test pretty frequently?
> Yes. Fixes are here (see the ~10 emails above in the thread for details):
> https://www.pos
On Fri, Aug 30, 2024, 21:21 Tom Lane wrote:
>
> While we're piling on, has anyone noticed that *non* Windows buildfarm
> animals are also failing this test pretty frequently?
>
Any thoughts?
>
Yes. Fixes are here (see the ~10 emails above in the thread for details):
https://www.postgresql.org/
Alexander Lakhin writes:
> Let me show you another related anomaly, which drongo kindly discovered
> recently: [1]. That test failed with:
> SELECT dblink_cancel_query('dtest1');
> - dblink_cancel_query
> --
> - OK
> + dblink_cancel_query
> +--
> + c
Hello Alvaro,
Let me show you another related anomaly, which drongo kindly discovered
recently: [1]. That test failed with:
SELECT dblink_cancel_query('dtest1');
- dblink_cancel_query
--
- OK
+ dblink_cancel_query
+--
+ cancel request timed out
(1 r
On 2024-Jul-16, Alvaro Herrera wrote:
> On 2024-Jul-16, Alvaro Herrera wrote:
>
> > Maybe we can disable this test specifically on Cygwin. We could do that
> > by creating a postgres_fdw_cancel.sql file, with the current output for
> > all platforms, and a "SELECT version() ~ 'cygwin' AS skip_te
On Thu, Jul 18, 2024 at 7:00 AM Alexander Lakhin wrote:
> As far as I can see (having analyzed a number of runs), the hanging occurs
> when some itimer-related activity happens before "peek_socket" in this
> event sequence:
> [main] postgres {pid} select_stuff::wait: res after verify 0
> [main] po
On Wed, Jul 17, 2024 at 3:08 AM Alvaro Herrera wrote:
> Ugh. I tried to follow what's going on in that cygwin code, but I gave
> up pretty quickly. It depends on a mutex, but I didn't see the mutex
> being defined or initialized anywhere.
https://github.com/cygwin/cygwin/blob/cygwin-3.5.3/winsu
On 2024-Jul-16, Alvaro Herrera wrote:
> Maybe we can disable this test specifically on Cygwin. We could do that
> by creating a postgres_fdw_cancel.sql file, with the current output for
> all platforms, and a "SELECT version() ~ 'cygwin' AS skip_test" query,
> as we do for encoding tests and such
On 2024-Jul-16, Alexander Lakhin wrote:
> I've managed to reproduce this issue in my Cygwin environment by running
> the postgres_fdw test in a loop (10 iterations are enough to get the
> described effect). And what I'm seeing is that a query-cancelling backend
> is stuck inside pgfdw_xact_callbac
Hello,
25.06.2024 11:24, Jelte Fennema-Nio wrote:
My expectation is that that should remove all failure cases. If it
doesn't, I think our best bet is removing the test again.
It looks like that test eventually showed what could be called a virtue.
Please take a look at a recent BF failure [1]:
On Mon, 1 Jul 2024 at 00:38, Jelte Fennema-Nio wrote:
> Ugh yes, I think this was a copy paste error. See attached patch 0003
> to fix this (rest of the patches are untouched from previous
> revision).
Alvaro committed 0003, which caused cfbot to think a rebase is
necessary. Attached should solve
On Sun, 30 Jun 2024 at 21:00, Noah Misch wrote:
> Shouldn't that be s/conn->status/cancelConn->status/?
Ugh yes, I think this was a copy paste error. See attached patch 0003
to fix this (rest of the patches are untouched from previous
revision).
v3-0001-Make-postgres_fdw-cancel-test-not-flaky-a
On Tue, Mar 12, 2024 at 05:50:48PM +0100, Alvaro Herrera wrote:
> On 2024-Mar-12, Jelte Fennema-Nio wrote:
> > On Tue, 12 Mar 2024 at 10:19, Alvaro Herrera
> > wrote:
> > > Here's a last one for the cfbot.
> >
> > Thanks for committing the first 3 patches btw.
>
> Thanks, I included it.
PGcanc
On Tue, 25 Jun 2024 at 07:00, Alexander Lakhin wrote:
> I'd just like to add that that one original query assumes several "remote"
> queries (see the attached excerpt from postmaster.log with verbose logging
> enabled).
Nice catch! All those EXPLAIN queries are definitely not intentional,
and lik
24.06.2024 01:59, Jelte Fennema-Nio wrote:
On Sat, 22 Jun 2024 at 17:00, Alexander Lakhin wrote:
@@ -2775,6 +2775,7 @@
SET LOCAL statement_timeout = '10ms';
select count(*) from ft1 CROSS JOIN ft2 CROSS JOIN ft4 CROSS JOIN ft5; --
this takes very long
ERROR: canceling statement due t
On Sat, 22 Jun 2024 at 17:00, Alexander Lakhin wrote:
> @@ -2775,6 +2775,7 @@
> SET LOCAL statement_timeout = '10ms';
> select count(*) from ft1 CROSS JOIN ft2 CROSS JOIN ft4 CROSS JOIN ft5; --
> this takes very long
> ERROR: canceling statement due to statement timeout
> +WARNING: could
Hello hackers,
30.03.2024 01:17, Noah Misch wrote:
On Fri, Mar 29, 2024 at 09:17:55AM +0100, Jelte Fennema-Nio wrote:
On Thu, 28 Mar 2024 at 19:03, Tom Lane wrote:
Could we make this test bulletproof by using an injection point?
If not, I remain of the opinion that we're better off without it
[ from a week ago ]
Alvaro Herrera writes:
> Hm, indri failed:
> ccache gcc -Wall -Wmissing-prototypes -Wpointer-arith
> -Wdeclaration-after-statement -Werror=vla -Werror=unguarded-availability-new
> -Wendif-labels -Wmissing-format-attribute -Wcast-function-type
> -Wformat-security -fno-strict
On Thu, 4 Apr 2024 at 10:45, Alvaro Herrera wrote:
> Yeah, this seems to work and I no longer get that complaint from
> headerscheck.
patch LGTM
On 2024-Apr-03, Tom Lane wrote:
> On my machine, headerscheck does not like this:
>
> $ src/tools/pginclude/headerscheck --cplusplus
> In file included from /tmp/headerscheck.4gTaW5/test.cpp:3:
> ./src/include/libpq/libpq-be-fe-helpers.h: In function 'char*
> libpqsrv_cancel(PGconn*, TimestampTz
Alvaro Herrera writes:
> Great, thanks for looking. Pushed now, I'll be closing the commitfest
> entry shortly.
On my machine, headerscheck does not like this:
$ src/tools/pginclude/headerscheck --cplusplus
In file included from /tmp/headerscheck.4gTaW5/test.cpp:3:
./src/include/libpq/libpq-be-
On Fri, Mar 29, 2024 at 09:17:55AM +0100, Jelte Fennema-Nio wrote:
> On Thu, 28 Mar 2024 at 19:03, Tom Lane wrote:
> > Could we make this test bulletproof by using an injection point?
> > If not, I remain of the opinion that we're better off without it.
>
> Possibly, and if so, I agree that would
On Thu, 28 Mar 2024 at 19:03, Tom Lane wrote:
>
> Alvaro Herrera writes:
> > It doesn't fail when it's too fast -- it's just that it doesn't cover
> > the case we want to cover.
>
> That's hardly better, because then you think you have test
> coverage but maybe you don't.
Honestly, that seems qu
Alvaro Herrera writes:
> On 2024-Mar-28, Tom Lane wrote:
>> If the test fails both when the machine is too slow and when it's
>> too fast, then there's zero hope of making it stable and we should
>> just remove it.
> It doesn't fail when it's too fast -- it's just that it doesn't cover
> the case
On 2024-Mar-28, Tom Lane wrote:
> Jelte Fennema-Nio writes:
>
> > I think we don't really want to make the timeout too short. Otherwise
> > the query might get cancelled before we push any query down to the
> > FDW. I guess that means that for some slow machines even 10ms is not
> > enough to ma
Jelte Fennema-Nio writes:
> On Thu, 28 Mar 2024 at 17:43, Alvaro Herrera wrote:
>> Hah, you're right, I can reproduce with a smaller timeout, and using SET
>> LOCAL works as a fix. If we're doing that, why not reduce the timeout
>> to 1ms? We don't need to wait extra 9ms ...
> I think we don't
On Thu, 28 Mar 2024 at 17:43, Alvaro Herrera wrote:
> Hah, you're right, I can reproduce with a smaller timeout, and using SET
> LOCAL works as a fix. If we're doing that, why not reduce the timeout
> to 1ms? We don't need to wait extra 9ms ...
I think we don't really want to make the timeout t
On 2024-Mar-28, Jelte Fennema-Nio wrote:
> Ugh that's annoying, the RESET is timing out too I guess.
Hah, you're right, I can reproduce with a smaller timeout, and using SET
LOCAL works as a fix. If we're doing that, why not reduce the timeout
to 1ms? We don't need to wait extra 9ms ...
--
Ál
On Thu, 28 Mar 2024 at 17:34, Alvaro Herrera wrote:
>
> Eh, kestrel has also failed[1], apparently every query after the large
> JOIN that this commit added as test fails with a statement timeout error.
>
> [1]
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=kestrel&dt=2024-03-28%2016%3
Eh, kestrel has also failed[1], apparently every query after the large
JOIN that this commit added as test fails with a statement timeout error.
[1]
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=kestrel&dt=2024-03-28%2016%3A01%3A14
--
Álvaro Herrera 48°01'N 7°57'E — ht
On 2024-Mar-28, Alvaro Herrera wrote:
> Undefined symbols for architecture arm64:
> "_libintl_gettext", referenced from:
> _libpqsrv_cancel in dblink.o
> _libpqsrv_cancel in dblink.o
> ld: symbol(s) not found for architecture arm64
> clang: error: linker command failed with exit code
Hm, indri failed:
ccache gcc -Wall -Wmissing-prototypes -Wpointer-arith
-Wdeclaration-after-statement -Werror=vla -Werror=unguarded-availability-new
-Wendif-labels -Wmissing-format-attribute -Wcast-function-type
-Wformat-security -fno-strict-aliasing -fwrapv
-Wno-unused-command-line-argument -
On 2024-Mar-28, Jelte Fennema-Nio wrote:
> Your changes look good, apart from the prverror stuff indeed. If you
> remove the prverror stuff again I think this is ready to commit.
Great, thanks for looking. Pushed now, I'll be closing the commitfest
entry shortly.
--
Álvaro Herrera
On Wed, 27 Mar 2024 at 19:27, Alvaro Herrera wrote:
> I ended up reducing the two PG_TRY blocks to a single one. I see no
> reason to split them up, and this way it looks more legible.
I definitely agree this looks better. Not sure why I hadn't done that,
maybe it wasn't possible in one of the e
On Wed, 27 Mar 2024 at 19:46, Alvaro Herrera wrote:
>
> On 2024-Mar-27, Alvaro Herrera wrote:
>
> > I changed it so that the error messages are returned as translated
> > phrases, and was bothered by the fact that if errors happen repeatedly,
> > the memory for them might be leaked. Maybe this is
On 2024-Mar-27, Alvaro Herrera wrote:
> I changed it so that the error messages are returned as translated
> phrases, and was bothered by the fact that if errors happen repeatedly,
> the memory for them might be leaked. Maybe this is fine depending on
> the caller's memory context, but since it's
On 2024-Mar-22, Jelte Fennema-Nio wrote:
> On Thu, 21 Mar 2024 at 03:54, Noah Misch wrote:
> >
> > On Mon, Mar 18, 2024 at 07:40:10PM +0100, Alvaro Herrera wrote:
> > > I enabled the test again and also pushed the changes to dblink,
> > > isolationtester and fe_utils (which AFAICS is used by pg_d
On Thu, 21 Mar 2024 at 03:54, Noah Misch wrote:
>
> On Mon, Mar 18, 2024 at 07:40:10PM +0100, Alvaro Herrera wrote:
> > I enabled the test again and also pushed the changes to dblink,
> > isolationtester and fe_utils (which AFAICS is used by pg_dump,
>
> I recommend adding a libpqsrv_cancel() func
On Mon, Mar 18, 2024 at 07:40:10PM +0100, Alvaro Herrera wrote:
> I enabled the test again and also pushed the changes to dblink,
> isolationtester and fe_utils (which AFAICS is used by pg_dump,
I recommend adding a libpqsrv_cancel() function to libpq-be-fe-helpers.h, to
use from dblink and postgr
I enabled the test again and also pushed the changes to dblink,
isolationtester and fe_utils (which AFAICS is used by pg_dump,
pg_amcheck, reindexdb and vacuumdb). I chickened out of committing the
postgres_fdw changes though, so here they are again. Not sure I'll find
courage to get these done b
On Thu, 14 Mar 2024 at 11:33, Alvaro Herrera wrote:
> Hmm, isn't this basically saying that we're giving up on reliably
> canceling queries altogether? I mean, maybe we'd like to instead fix
> the bug about canceling queries in extended query protocol ...
> Isn't that something you're worried abo
On 2024-Mar-14, Jelte Fennema-Nio wrote:
> On Wed, 13 Mar 2024 at 20:08, Jacob Champion
> wrote:
> > I hit this on my machine. With the attached diff I can reproduce
> > constantly (including with the most recent test patch); I think the
> > cancel must be arriving between the bind/execute steps?
On Wed, 13 Mar 2024 at 20:08, Jacob Champion
wrote:
> I hit this on my machine. With the attached diff I can reproduce
> constantly (including with the most recent test patch); I think the
> cancel must be arriving between the bind/execute steps?
Nice find! Your explanation makes total sense. Att
On Wed, Mar 13, 2024 at 12:01 PM Alvaro Herrera wrote:
> On 2024-Mar-13, Jelte Fennema-Nio wrote:
> > Sadly I'm having a hard time reliably reproducing this race condition
> > locally. So it's hard to be sure what is happening here. Attached is a
> > patch with a wild guess as to what the issue mi
On 2024-Mar-13, Jelte Fennema-Nio wrote:
> I agree it's probably a timing issue. The cancel being received after
> the query is done seems very unlikely, since the query takes 180
> seconds (assuming PG_TEST_TIMEOUT_DEFAULT is not lowered for these
> animals). I think it's more likely that the can
On Wed, 13 Mar 2024 at 04:53, Tom Lane wrote:
> I suspect it's basically just a
> timing dependency. Have you thought about the fact that a cancel
> request is a no-op if it arrives after the query's done?
I agree it's probably a timing issue. The cancel being received after
the query is done se
Jelte Fennema-Nio writes:
> On Tue, 12 Mar 2024 at 19:28, Alvaro Herrera wrote:
>>> Hmm, buildfarm member kestrel (which uses
>>> -fsanitize=undefined,alignment) failed:
>> Hm, I tried using the same compile flags, couldn't reproduce.
> Okay, it passed now it seems so I guess this test is flaky
On Tue, 12 Mar 2024 at 19:28, Alvaro Herrera wrote:
>
> On 2024-Mar-12, Alvaro Herrera wrote:
>
> > Hmm, buildfarm member kestrel (which uses
> > -fsanitize=undefined,alignment) failed:
> >
> > # Running: libpq_pipeline -r 700 cancel port=49975 host=/tmp/dFh46H7YGc
> > dbname='postgres'
> > test c
On 2024-Mar-12, Alvaro Herrera wrote:
> Hmm, buildfarm member kestrel (which uses
> -fsanitize=undefined,alignment) failed:
>
> # Running: libpq_pipeline -r 700 cancel port=49975 host=/tmp/dFh46H7YGc
> dbname='postgres'
> test cancellations...
> libpq_pipeline:260: query did not fail when it was
Hmm, buildfarm member kestrel (which uses
-fsanitize=undefined,alignment) failed:
# Running: libpq_pipeline -r 700 cancel port=49975 host=/tmp/dFh46H7YGc
dbname='postgres'
test cancellations...
libpq_pipeline:260: query did not fail when it was expected
https://buildfarm.postgresql.org/cgi-bin/s
On 2024-Mar-12, Jelte Fennema-Nio wrote:
> On Tue, 12 Mar 2024 at 10:19, Alvaro Herrera wrote:
> > Here's a last one for the cfbot.
>
> Thanks for committing the first 3 patches btw. Attached a tiny change
> to 0001, which adds "(backing struct for PGcancelConn)" to the comment
> on pg_cancel_co
On Tue, 12 Mar 2024 at 15:04, Jelte Fennema-Nio wrote:
> Attached a tiny change to 0001
One more tiny comment change, stating that pg_cancel is used by the
deprecated PQcancel function.
v36-0001-libpq-Add-encrypted-and-non-blocking-query-cance.patch
Description: Binary data
v36-0002-Start-usi
On Tue, 12 Mar 2024 at 10:19, Alvaro Herrera wrote:
> Here's a last one for the cfbot.
Thanks for committing the first 3 patches btw. Attached a tiny change
to 0001, which adds "(backing struct for PGcancelConn)" to the comment
on pg_cancel_conn.
From d340fde6883a249fd7c1a90033675a3b5edb603e Mon
On Tue, 12 Mar 2024 at 10:53, Jelte Fennema-Nio wrote:
> I'd rather fail as hard as possible when someone is using the API
> wrongly.
To be clear, this is my way of looking at it. If you feel strongly
about that we should not change conn.status, I'm fine with making that
change to the patchset.
On Tue, 12 Mar 2024 at 10:19, Alvaro Herrera wrote:
> If we do this and we see conn.status is not ALLOCATED, meaning a cancel
> is already ongoing, shouldn't we leave conn.status alone instead of
> changing to CONNECTION_BAD? I mean, we shouldn't be juggling the elbow
> of whoever's doing that, s
Here's a last one for the cfbot.
I have a question about this one
int
PQcancelStart(PGcancelConn *cancelConn)
{
[...]
if (cancelConn->conn.status != CONNECTION_ALLOCATED)
{
libpq_append_conn_error(&cancelConn->conn,
"cancel request is already be
Attached is a new patchset with various changes. I created a dedicated
0002 patch to add tests for the already existing cancellation
functions, because that seemed useful for another thread where changes
to the cancellation protocol are being proposed[1].
[1]:
https://www.postgresql.org/message-i
On Wed, 6 Mar 2024 at 19:22, Alvaro Herrera wrote:
> Docs: one bogus "that that".
will fix
> Did we consider having PQcancelConn() instead be called
> PQcancelCreate()?
Fine by me
> Also, the comment still says
> "Asynchronously cancel a query on the given connection. This requires
> polling t
Docs: one bogus "that that".
Did we consider having PQcancelConn() instead be called
PQcancelCreate()? I think this better conveys that what we're doing is
create an object that can be used to do something, and that nothing else
is done with it by default. Also, the comment still says
"Asynchron
On Wed, 6 Mar 2024 at 15:03, Denis Laxalde wrote:
>
> In patch 0004, I noticed a couple of typos in the documentation; please
> find attached a fixup patch correcting these.
Thanks, applied.
> while not actually listing the "statuses". Should we list them?
I listed the relevant statuses over no
In patch 0004, I noticed a couple of typos in the documentation; please
find attached a fixup patch correcting these.
Still in the documentation, same patch, the last paragraph documenting
PQcancelPoll() ends as:
+ indicate the current stage of the connection procedure and might
be use
On Wed, 14 Feb 2024 at 18:41, Alvaro Herrera wrote:
> Hmm, I think the changes to libpq_pipeline in 0005 should be in 0004.
Yeah, you're correct. Fixed that now.
v32-0005-Start-using-new-libpq-cancel-APIs.patch
Description: Binary data
v32-0004-libpq-Add-encrypted-and-non-blocking-versions-of
On 2024-Feb-14, Jelte Fennema-Nio wrote:
> Attached is a new version of the final patches, with much improved
> docs (imho) and the new function names: PQcancelStart and
> PQcancelBlocking.
Hmm, I think the changes to libpq_pipeline in 0005 should be in 0004.
--
Álvaro Herrera PostgreSQ
On Sun, 4 Feb 2024 at 16:39, Alvaro Herrera wrote:
> Maybe this is okay? I'll have a look at the whole final situation more
> carefully later; or if somebody else wants to share an opinion, please
> do so.
Attached is a new version of the final patches, with much improved
docs (imho) and the new
On 2024-Feb-02, Jelte Fennema-Nio wrote:
> The only reasonable thing I can think of to make that situation better
> is to move that part of the function outside of PQcancelPoll and
> create a dedicated PQcancelStart function for it. It introduces an
> extra function, but it does seem more in line
On Fri, 2 Feb 2024 at 16:06, Alvaro Herrera wrote:
> Now, looking at this list, I think it's surprising that the nonblocking
> request for a cancellation is called PQcancelPoll. PQcancelSend() is at
> odds with the asynchronous query API, which uses the verb "send" for the
> asynchronous variants
Hello,
The patched docs claim that PQrequestCancel is insecure, but neither the
code nor docs explain why. The docs for PQcancel on the other hand do
mention that encryption is not used; does that apply to PQrequestCancel
as well and is that the reason? If so, I think we should copy the
warning
onn);
+extern PGconn *pqMakeEmptyPGconn(void);
+extern bool pqCopyPGconn(PGconn *srcConn, PGconn *dstConn);
+extern void pqClosePGconn(PGconn *conn);
extern pgthreadlock_t pg_g_threadlock;
--
2.34.1
From f14412006e804ededda2063b08b37aaa8dbba355 Mon Sep 17 00:00:00 2001
From: Jelte Fennema-Ni
On 2024-Jan-29, Jelte Fennema-Nio wrote:
> On Mon, 29 Jan 2024 at 12:44, Alvaro Herrera wrote:
> > Thanks! I committed 0001 now. I also renamed the new
> > pq_parse_int_param to pqParseIntParam, for consistency with other
> > routines there. Please rebase the other patches.
>
> Awesome! Rebas
On Mon, 29 Jan 2024 at 12:44, Alvaro Herrera wrote:
> Thanks! I committed 0001 now. I also renamed the new
> pq_parse_int_param to pqParseIntParam, for consistency with other
> routines there. Please rebase the other patches.
Awesome! Rebased, and renamed pq_release_conn_hosts to
pqReleaseConn
On 2024-Jan-28, Jelte Fennema-Nio wrote:
> On Sun, 28 Jan 2024 at 10:51, Jelte Fennema-Nio wrote:
> > Both of those are fixed now.
>
> Okay, there turned out to also be an issue on Windows with
> setKeepalivesWin32 not being available in fe-cancel.c. That's fixed
> now too (as well as some minor
On Sun, 28 Jan 2024 at 10:51, Jelte Fennema-Nio wrote:
> Both of those are fixed now.
Okay, there turned out to also be an issue on Windows with
setKeepalivesWin32 not being available in fe-cancel.c. That's fixed
now too (as well as some minor formatting issues).
From 4efbb0c75341f4612f0c5b8d5d3f
On Sun, 28 Jan 2024 at 04:15, vignesh C wrote:
> CFBot shows that the patch has few compilation errors as in [1]:
> [17:07:07.621] /usr/bin/ld:
> ../../../src/fe_utils/libpgfeutils.a(cancel.o): in function
> `handle_sigint':
> [17:07:07.621] cancel.c:(.text+0x50): undefined reference to `PQcancel'
On Fri, 26 Jan 2024 at 22:22, Jelte Fennema-Nio wrote:
>
> On Fri, 26 Jan 2024 at 13:11, Alvaro Herrera wrote:
> > I wonder, would it make sense to put all these new functions in a
> > separate file fe-cancel.c?
>
>
> Okay I tried doing that. I think the end result is indeed quite nice,
> having
On Fri, 26 Jan 2024 at 18:19, Alvaro Herrera wrote:
> Yeah, I see that point of view as well. I like the end result; the
> additional protos in libpq-int.h don't bother me. Does anybody else
> wants to share their opinion on it? If none, then I'd consider going
> ahead with this version.
To be
On 2024-Jan-26, Jelte Fennema-Nio wrote:
> Okay I tried doing that. I think the end result is indeed quite nice,
> having all the cancellation related functions together in a file. But
> it did require making a bunch of static functions in fe-connect
> extern, and adding them to libpq-int.h. On on
On Fri, 26 Jan 2024 at 13:11, Alvaro Herrera wrote:
> I wonder, would it make sense to put all these new functions in a
> separate file fe-cancel.c?
Okay I tried doing that. I think the end result is indeed quite nice,
having all the cancellation related functions together in a file. But
it did
Pushed 0001.
I wonder, would it make sense to put all these new functions in a
separate file fe-cancel.c?
--
Álvaro HerreraBreisgau, Deutschland — https://www.EnterpriseDB.com/
"World domination is proceeding according to plan"(Andrew Morton)
On Fri, 26 Jan 2024 at 02:59, vignesh C wrote:
> Please post an updated version for the same.
Done.
From 5a94d610a4fe138365e2e88c5cec72eba53ed036 Mon Sep 17 00:00:00 2001
From: Jelte Fennema-Nio
Date: Thu, 14 Dec 2023 13:39:04 +0100
Subject: [PATCH v25 2/3] Add non-blocking version of PQcan
On Wed, 20 Dec 2023 at 19:17, Jelte Fennema-Nio wrote:
>
> On Thu, 14 Dec 2023 at 13:57, Jelte Fennema-Nio wrote:
> > I changed all the places that were not adhering to those spellings.
>
> It seems I forgot a /g on my sed command to do this so it turned out I
> missed one that caused the test to
On Thu, 14 Dec 2023 at 13:57, Jelte Fennema-Nio wrote:
> I changed all the places that were not adhering to those spellings.
It seems I forgot a /g on my sed command to do this so it turned out I
missed one that caused the test to fail to compile... Attached is a
fixed version.
I also updated th
On Mon, 13 Nov 2023 at 03:39, Thomas Munro wrote:
> We follow the common en-US usage: "canceled", "canceling" but
> "cancellation". Blame Webstah et al.
I changed all the places that were not adhering to those spellings.
There were also a few of such places in parts of the codebase that
these ch
Trivial observation: these patches obviously introduce many instances
of words derived from "cancel", but they don't all conform to
established project decisions (cf 21f1e15a) about how to spell them.
We follow the common en-US usage: "canceled", "canceling" but
"cancellation". Blame Webstah et al
Rebased again to resolve some conflicts
v22-0003-Add-non-blocking-version-of-PQcancel.patch
Description: Binary data
v22-0002-Return-2-from-pqReadData-on-EOF.patch
Description: Binary data
v22-0004-Start-using-new-libpq-cancel-APIs.patch
Description: Binary data
I noticed that cfbot was unable to run tests due to some rebase
conflict. It seems the pgindent changes from patch 1 have now been
made.
So adding the rebased patches without patch 1 now to unblock cfbot.
v21-0002-Return-2-from-pqReadData-on-EOF.patch
Description: Binary data
v21-0003-Add-non-b
Okay, I rebased again. Indeed 983ec23007b gave the most problems.
On Fri, 7 Apr 2023 at 10:02, Denis Laxalde wrote:
>
> The patch set does not apply any more.
>
> I tried to rebase locally; even leaving out 1 ("libpq: Run pgindent
> after a9e9a9f32b3"), patch 4 ("Start using new libpq cancel APIs
The patch set does not apply any more.
I tried to rebase locally; even leaving out 1 ("libpq: Run pgindent
after a9e9a9f32b3"), patch 4 ("Start using new libpq cancel APIs") is
harder to resolve following 983ec23007b (I suppose).
Appart from that, the implementation in v19 sounds good to me,
1 - 100 of 140 matches
Mail list logo
