Обсуждение: WaitLatchOrSocket seems to not count to 4 right...
Unless I'm misreading this code I think the nevents in WaitLatchOrSocket should really be 4 not 3. At least there are 4 calls to AddWaitEventToSet in it and I think it's possible to trigger all 4. I guess it's based on knowing that nobody would actually set WL_EXIT_ON_PM_DEATH and WL_POSTMASTER_DEATH on the same waitset? -- greg
On 2022/02/08 7:00, Greg Stark wrote:
> Unless I'm misreading this code I think the nevents in
> WaitLatchOrSocket should really be 4 not 3. At least there are 4 calls
> to AddWaitEventToSet in it and I think it's possible to trigger all 4.
Good catch! I think you're right.
As the quick test, I confirmed that the assertion failure happened when I passed four possible events to
WaitLatchOrSocket()in postgres_fdw.
TRAP: FailedAssertion("set->nevents < set->nevents_space", File: "latch.c", Line: 868, PID: 54424)
0 postgres 0x0000000107efa49f ExceptionalCondition + 223
1 postgres 0x0000000107cbca0c AddWaitEventToSet + 76
2 postgres 0x0000000107cbd86e WaitLatchOrSocket + 430
3 postgres_fdw.so 0x000000010848b1aa pgfdw_get_result + 218
4 postgres_fdw.so 0x000000010848accb do_sql_command + 75
5 postgres_fdw.so 0x000000010848c6b8 configure_remote_session + 40
6 postgres_fdw.so 0x000000010848c32d connect_pg_server + 1629
7 postgres_fdw.so 0x000000010848aa06 make_new_connection + 566
8 postgres_fdw.so 0x0000000108489a06 GetConnection + 550
9 postgres_fdw.so 0x0000000108497ba4 postgresBeginForeignScan + 260
10 postgres 0x0000000107a7d79f ExecInitForeignScan + 943
11 postgres 0x0000000107a5c8ab ExecInitNode + 683
12 postgres 0x0000000107a5028a InitPlan + 1386
13 postgres 0x0000000107a4fb66 standard_ExecutorStart + 806
14 postgres 0x0000000107a4f833 ExecutorStart + 83
15 postgres 0x0000000107d0277f PortalStart + 735
16 postgres 0x0000000107cfe150 exec_simple_query + 1168
17 postgres 0x0000000107cfd39e PostgresMain + 2110
18 postgres 0x0000000107c07e72 BackendRun + 50
19 postgres 0x0000000107c07438 BackendStartup + 552
20 postgres 0x0000000107c0621c ServerLoop + 716
21 postgres 0x0000000107c039f9 PostmasterMain + 6441
22 postgres 0x0000000107ae20d9 main + 809
23 libdyld.dylib 0x00007fff2045cf3d start + 1
24 ??? 0x0000000000000003 0x0 + 3
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
On Tue, Feb 8, 2022 at 1:48 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
> On 2022/02/08 7:00, Greg Stark wrote:
> > Unless I'm misreading this code I think the nevents in
> > WaitLatchOrSocket should really be 4 not 3. At least there are 4 calls
> > to AddWaitEventToSet in it and I think it's possible to trigger all 4.
>
> Good catch! I think you're right.
>
> As the quick test, I confirmed that the assertion failure happened when I passed four possible events to
WaitLatchOrSocket()in postgres_fdw.
>
> TRAP: FailedAssertion("set->nevents < set->nevents_space", File: "latch.c", Line: 868, PID: 54424)
Yeah, the assertion shows there's no problem, but we should change it
to 4 (and one day we should make it dynamic and change udata to hold
an index instead of a pointer...) or, since it's a bit silly to add
both of those events, maybe we should do:
- if ((wakeEvents & WL_POSTMASTER_DEATH) && IsUnderPostmaster)
- AddWaitEventToSet(set, WL_POSTMASTER_DEATH, PGINVALID_SOCKET,
- NULL, NULL);
-
if ((wakeEvents & WL_EXIT_ON_PM_DEATH) && IsUnderPostmaster)
AddWaitEventToSet(set, WL_EXIT_ON_PM_DEATH, PGINVALID_SOCKET,
NULL, NULL);
+ else if ((wakeEvents & WL_POSTMASTER_DEATH) && IsUnderPostmaster)
+ AddWaitEventToSet(set, WL_POSTMASTER_DEATH, PGINVALID_SOCKET,
+
On 2022/02/08 9:51, Thomas Munro wrote: > an index instead of a pointer...) or, since it's a bit silly to add > both of those events, maybe we should do: > > - if ((wakeEvents & WL_POSTMASTER_DEATH) && IsUnderPostmaster) > - AddWaitEventToSet(set, WL_POSTMASTER_DEATH, PGINVALID_SOCKET, > - NULL, NULL); > - > if ((wakeEvents & WL_EXIT_ON_PM_DEATH) && IsUnderPostmaster) > AddWaitEventToSet(set, WL_EXIT_ON_PM_DEATH, PGINVALID_SOCKET, > NULL, NULL); > + else if ((wakeEvents & WL_POSTMASTER_DEATH) && IsUnderPostmaster) > + AddWaitEventToSet(set, WL_POSTMASTER_DEATH, PGINVALID_SOCKET, > + +1 Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
On Tue, Feb 8, 2022 at 1:51 PM Thomas Munro <thomas.munro@gmail.com> wrote: > (and one day we should make it dynamic and change udata to hold > an index instead of a pointer...) Here's a patch like that. I'd originally sketched this out for another project, but I don't think I need it for that anymore. After this exchange I couldn't resist fleshing it out for a commitfest, just on useability grounds. Thoughts?
Вложения
Hi, On 2022-02-11 10:47:45 +1300, Thomas Munro wrote: > I'd originally sketched this out for another project, but I don't > think I need it for that anymore. After this exchange I couldn't > resist fleshing it out for a commitfest, just on useability grounds. > Thoughts? This currently doesn't apply: http://cfbot.cputube.org/patch_37_3533.log Marked as waiting-on-author for now. Are you aiming this for 15? Greetings, Andres Freund
This entry has been waiting on author input for a while (our current
threshold is roughly two weeks), so I've marked it Returned with
Feedback.
Once you think the patchset is ready for review again, you (or any
interested party) can resurrect the patch entry by visiting
https://commitfest.postgresql.org/38/3533/
and changing the status to "Needs Review", and then changing the
status again to "Move to next CF". (Don't forget the second step;
hopefully we will have streamlined this in the near future!)
Thanks,
--Jacob