Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit50c67c2

Browse files
committed
Use ResourceOwner to track WaitEventSets.
A WaitEventSet holds file descriptors or event handles (on Windows).If FreeWaitEventSet is not called, those fds or handles are leaked.Use ResourceOwners to track WaitEventSets, to clean those upautomatically on error.This was a live bug in async Append nodes, if a FDW'sForeignAsyncRequest function failed. (In back branches, I will apply amore localized fix for that based on PG_TRY-PG_FINALLY.)The added test doesn't check for leaking resources, so it passed evenbefore this commit. But at least it covers the code path.In the passing, fix misleading comment on what the 'nevents' argumentto WaitEventSetWait means.Report by Alexander Lakhin, analysis and suggestion for the fix byTom Lane. Fixes bug #17828.Reviewed-by: Alexander Lakhin, Thomas MunroDiscussion:https://www.postgresql.org/message-id/472235.1678387869@sss.pgh.pa.us
1 parent414e755 commit50c67c2

File tree

9 files changed

+84
-13
lines changed

9 files changed

+84
-13
lines changed

‎contrib/postgres_fdw/expected/postgres_fdw.out

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10809,6 +10809,13 @@ SELECT * FROM result_tbl ORDER BY a;
1080910809
(2 rows)
1081010810

1081110811
DELETE FROM result_tbl;
10812+
-- Test error handling, if accessing one of the foreign partitions errors out
10813+
CREATE FOREIGN TABLE async_p_broken PARTITION OF async_pt FOR VALUES FROM (10000) TO (10001)
10814+
SERVER loopback OPTIONS (table_name 'non_existent_table');
10815+
SELECT * FROM async_pt;
10816+
ERROR: relation "public.non_existent_table" does not exist
10817+
CONTEXT: remote SQL command: SELECT a, b, c FROM public.non_existent_table
10818+
DROP FOREIGN TABLE async_p_broken;
1081210819
-- Check case where multiple partitions use the same connection
1081310820
CREATE TABLE base_tbl3 (a int, b int, c text);
1081410821
CREATE FOREIGN TABLE async_p3 PARTITION OF async_pt FOR VALUES FROM (3000) TO (4000)

‎contrib/postgres_fdw/sql/postgres_fdw.sql

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3607,6 +3607,12 @@ INSERT INTO result_tbl SELECT a, b, 'AAA' || c FROM async_pt WHERE b === 505;
36073607
SELECT*FROM result_tblORDER BY a;
36083608
DELETEFROM result_tbl;
36093609

3610+
-- Test error handling, if accessing one of the foreign partitions errors out
3611+
CREATE FOREIGN TABLE async_p_broken PARTITION OF async_pt FORVALUESFROM (10000) TO (10001)
3612+
SERVER loopback OPTIONS (table_name'non_existent_table');
3613+
SELECT*FROM async_pt;
3614+
DROP FOREIGN TABLE async_p_broken;
3615+
36103616
-- Check case where multiple partitions use the same connection
36113617
CREATETABLEbase_tbl3 (aint, bint, ctext);
36123618
CREATE FOREIGN TABLE async_p3 PARTITION OF async_pt FORVALUESFROM (3000) TO (4000)

‎src/backend/executor/nodeAppend.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1025,7 +1025,8 @@ ExecAppendAsyncEventWait(AppendState *node)
10251025
/* We should never be called when there are no valid async subplans. */
10261026
Assert(node->as_nasyncremain>0);
10271027

1028-
node->as_eventset=CreateWaitEventSet(CurrentMemoryContext,nevents);
1028+
Assert(node->as_eventset==NULL);
1029+
node->as_eventset=CreateWaitEventSet(CurrentResourceOwner,nevents);
10291030
AddWaitEventToSet(node->as_eventset,WL_EXIT_ON_PM_DEATH,PGINVALID_SOCKET,
10301031
NULL,NULL);
10311032

@@ -1050,7 +1051,7 @@ ExecAppendAsyncEventWait(AppendState *node)
10501051
return;
10511052
}
10521053

1053-
/*We wait onat most EVENT_BUFFER_SIZE events. */
1054+
/*Returnat most EVENT_BUFFER_SIZE events in one call. */
10541055
if (nevents>EVENT_BUFFER_SIZE)
10551056
nevents=EVENT_BUFFER_SIZE;
10561057

‎src/backend/libpq/pqcomm.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ pq_init(void)
207207
elog(FATAL,"fcntl(F_SETFD) failed on socket: %m");
208208
#endif
209209

210-
FeBeWaitSet=CreateWaitEventSet(TopMemoryContext,FeBeWaitSetNEvents);
210+
FeBeWaitSet=CreateWaitEventSet(NULL,FeBeWaitSetNEvents);
211211
socket_pos=AddWaitEventToSet(FeBeWaitSet,WL_SOCKET_WRITEABLE,
212212
MyProcPort->sock,NULL,NULL);
213213
latch_pos=AddWaitEventToSet(FeBeWaitSet,WL_LATCH_SET,PGINVALID_SOCKET,

‎src/backend/postmaster/postmaster.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1695,7 +1695,7 @@ ConfigurePostmasterWaitSet(bool accept_connections)
16951695
FreeWaitEventSet(pm_wait_set);
16961696
pm_wait_set=NULL;
16971697

1698-
pm_wait_set=CreateWaitEventSet(CurrentMemoryContext,
1698+
pm_wait_set=CreateWaitEventSet(NULL,
16991699
accept_connections ? (1+NumListenSockets) :1);
17001700
AddWaitEventToSet(pm_wait_set,WL_LATCH_SET,PGINVALID_SOCKET,MyLatch,
17011701
NULL);

‎src/backend/postmaster/syslogger.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ SysLoggerMain(int argc, char *argv[])
311311
* syslog pipe, which implies that all other backends have exited
312312
* (including the postmaster).
313313
*/
314-
wes=CreateWaitEventSet(CurrentMemoryContext,2);
314+
wes=CreateWaitEventSet(NULL,2);
315315
AddWaitEventToSet(wes,WL_LATCH_SET,PGINVALID_SOCKET,MyLatch,NULL);
316316
#ifndefWIN32
317317
AddWaitEventToSet(wes,WL_SOCKET_READABLE,syslogPipe[0],NULL,NULL);

‎src/backend/storage/ipc/latch.c

Lines changed: 61 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@
6262
#include"storage/pmsignal.h"
6363
#include"storage/shmem.h"
6464
#include"utils/memutils.h"
65+
#include"utils/resowner.h"
6566

6667
/*
6768
* Select the fd readiness primitive to use. Normally the "most modern"
@@ -101,6 +102,8 @@
101102
/* typedef in latch.h */
102103
structWaitEventSet
103104
{
105+
ResourceOwnerowner;
106+
104107
intnevents;/* number of registered events */
105108
intnevents_space;/* maximum number of events in this set */
106109

@@ -195,6 +198,31 @@ static void WaitEventAdjustWin32(WaitEventSet *set, WaitEvent *event);
195198
staticinlineintWaitEventSetWaitBlock(WaitEventSet*set,intcur_timeout,
196199
WaitEvent*occurred_events,intnevents);
197200

201+
/* ResourceOwner support to hold WaitEventSets */
202+
staticvoidResOwnerReleaseWaitEventSet(Datumres);
203+
204+
staticconstResourceOwnerDescwait_event_set_resowner_desc=
205+
{
206+
.name="WaitEventSet",
207+
.release_phase=RESOURCE_RELEASE_AFTER_LOCKS,
208+
.release_priority=RELEASE_PRIO_WAITEVENTSETS,
209+
.ReleaseResource=ResOwnerReleaseWaitEventSet,
210+
.DebugPrint=NULL
211+
};
212+
213+
/* Convenience wrappers over ResourceOwnerRemember/Forget */
214+
staticinlinevoid
215+
ResourceOwnerRememberWaitEventSet(ResourceOwnerowner,WaitEventSet*set)
216+
{
217+
ResourceOwnerRemember(owner,PointerGetDatum(set),&wait_event_set_resowner_desc);
218+
}
219+
staticinlinevoid
220+
ResourceOwnerForgetWaitEventSet(ResourceOwnerowner,WaitEventSet*set)
221+
{
222+
ResourceOwnerForget(owner,PointerGetDatum(set),&wait_event_set_resowner_desc);
223+
}
224+
225+
198226
/*
199227
* Initialize the process-local latch infrastructure.
200228
*
@@ -323,7 +351,7 @@ InitializeLatchWaitSet(void)
323351
Assert(LatchWaitSet==NULL);
324352

325353
/* Set up the WaitEventSet used by WaitLatch(). */
326-
LatchWaitSet=CreateWaitEventSet(TopMemoryContext,2);
354+
LatchWaitSet=CreateWaitEventSet(NULL,2);
327355
latch_pos=AddWaitEventToSet(LatchWaitSet,WL_LATCH_SET,PGINVALID_SOCKET,
328356
MyLatch,NULL);
329357
if (IsUnderPostmaster)
@@ -541,7 +569,7 @@ WaitLatchOrSocket(Latch *latch, int wakeEvents, pgsocket sock,
541569
intret=0;
542570
intrc;
543571
WaitEventevent;
544-
WaitEventSet*set=CreateWaitEventSet(CurrentMemoryContext,3);
572+
WaitEventSet*set=CreateWaitEventSet(CurrentResourceOwner,3);
545573

546574
if (wakeEvents&WL_TIMEOUT)
547575
Assert(timeout >=0);
@@ -716,9 +744,12 @@ ResetLatch(Latch *latch)
716744
*
717745
* These events can then be efficiently waited upon together, using
718746
* WaitEventSetWait().
747+
*
748+
* The WaitEventSet is tracked by the given 'resowner'. Use NULL for session
749+
* lifetime.
719750
*/
720751
WaitEventSet*
721-
CreateWaitEventSet(MemoryContextcontext,intnevents)
752+
CreateWaitEventSet(ResourceOwnerresowner,intnevents)
722753
{
723754
WaitEventSet*set;
724755
char*data;
@@ -744,7 +775,10 @@ CreateWaitEventSet(MemoryContext context, int nevents)
744775
sz+=MAXALIGN(sizeof(HANDLE)* (nevents+1));
745776
#endif
746777

747-
data= (char*)MemoryContextAllocZero(context,sz);
778+
if (resowner!=NULL)
779+
ResourceOwnerEnlarge(resowner);
780+
781+
data= (char*)MemoryContextAllocZero(TopMemoryContext,sz);
748782

749783
set= (WaitEventSet*)data;
750784
data+=MAXALIGN(sizeof(WaitEventSet));
@@ -770,6 +804,12 @@ CreateWaitEventSet(MemoryContext context, int nevents)
770804
set->nevents_space=nevents;
771805
set->exit_on_postmaster_death= false;
772806

807+
if (resowner!=NULL)
808+
{
809+
ResourceOwnerRememberWaitEventSet(resowner,set);
810+
set->owner=resowner;
811+
}
812+
773813
#if defined(WAIT_USE_EPOLL)
774814
if (!AcquireExternalFD())
775815
{
@@ -834,16 +874,20 @@ CreateWaitEventSet(MemoryContext context, int nevents)
834874
void
835875
FreeWaitEventSet(WaitEventSet*set)
836876
{
877+
if (set->owner)
878+
{
879+
ResourceOwnerForgetWaitEventSet(set->owner,set);
880+
set->owner=NULL;
881+
}
882+
837883
#if defined(WAIT_USE_EPOLL)
838884
close(set->epoll_fd);
839885
ReleaseExternalFD();
840886
#elif defined(WAIT_USE_KQUEUE)
841887
close(set->kqueue_fd);
842888
ReleaseExternalFD();
843889
#elif defined(WAIT_USE_WIN32)
844-
WaitEvent*cur_event;
845-
846-
for (cur_event=set->events;
890+
for (WaitEvent*cur_event=set->events;
847891
cur_event< (set->events+set->nevents);
848892
cur_event++)
849893
{
@@ -2300,3 +2344,13 @@ drain(void)
23002344
}
23012345

23022346
#endif
2347+
2348+
staticvoid
2349+
ResOwnerReleaseWaitEventSet(Datumres)
2350+
{
2351+
WaitEventSet*set= (WaitEventSet*)DatumGetPointer(res);
2352+
2353+
Assert(set->owner!=NULL);
2354+
set->owner=NULL;
2355+
FreeWaitEventSet(set);
2356+
}

‎src/include/storage/latch.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,8 @@
102102

103103
#include<signal.h>
104104

105+
#include"utils/resowner.h"
106+
105107
/*
106108
* Latch structure should be treated as opaque and only accessed through
107109
* the public functions. It is defined here to allow embedding Latches as
@@ -173,7 +175,7 @@ extern void SetLatch(Latch *latch);
173175
externvoidResetLatch(Latch*latch);
174176
externvoidShutdownLatchSupport(void);
175177

176-
externWaitEventSet*CreateWaitEventSet(MemoryContextcontext,intnevents);
178+
externWaitEventSet*CreateWaitEventSet(ResourceOwnerresowner,intnevents);
177179
externvoidFreeWaitEventSet(WaitEventSet*set);
178180
externvoidFreeWaitEventSetAfterFork(WaitEventSet*set);
179181
externintAddWaitEventToSet(WaitEventSet*set,uint32events,pgsocketfd,

‎src/include/utils/resowner.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ typedef uint32 ResourceReleasePriority;
7474
#defineRELEASE_PRIO_TUPDESC_REFS400
7575
#defineRELEASE_PRIO_SNAPSHOT_REFS500
7676
#defineRELEASE_PRIO_FILES600
77+
#defineRELEASE_PRIO_WAITEVENTSETS700
7778

7879
/* 0 is considered invalid */
7980
#defineRELEASE_PRIO_FIRST1

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp