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

Commit7a8cb4a

Browse files
committed
Don't call palloc() while holding a spinlock, either.
Fix some more violations of the "only straight-line code inside aspinlock" rule. These are hazardous not only because they riskholding the lock for an excessively long time, but because it'spossible for palloc to throw elog(ERROR), leaving a stuck spinlockbehind.copy_replication_slot() had two separate places that did pallocswhile holding a spinlock. We can make the code simpler and saferby copying the whole ReplicationSlot struct into a local variablewhile holding the spinlock, and then referencing that copy.(While that's arguably more cycles than we really need to spendholding the lock, the struct isn't all that big, and this way seemsfar more maintainable than copying fields piecemeal. Anyway thisis surely much cheaper than a palloc.) That bug goes back to v12.InvalidateObsoleteReplicationSlots() not only did a palloc whileholding a spinlock, but for extra sloppiness then leaked the memory--- probably for the lifetime of the checkpointer process, thoughI didn't try to verify that. Fortunately that silliness is newin HEAD.pg_get_replication_slots() had a cosmetic violation of the rule,in that it only assumed it's safe to call namecpy() while holdinga spinlock. Still, that's a hazard waiting to bite somebody, andthere were some other cosmetic coding-rule violations in the samefunction, so clean it up. I back-patched this as far as v10; thecode exists before that but it looks different, and this didn'tseem important enough to adapt the patch further back.Discussion:https://postgr.es/m/20200602.161518.1399689010416646074.horikyota.ntt@gmail.com
1 parent8bc7449 commit7a8cb4a

File tree

2 files changed

+27
-41
lines changed

2 files changed

+27
-41
lines changed

‎src/backend/replication/slotfuncs.c

Lines changed: 25 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -228,87 +228,73 @@ pg_get_replication_slots(PG_FUNCTION_ARGS)
228228
for (slotno=0;slotno<max_replication_slots;slotno++)
229229
{
230230
ReplicationSlot*slot=&ReplicationSlotCtl->replication_slots[slotno];
231+
ReplicationSlotslot_contents;
231232
Datumvalues[PG_GET_REPLICATION_SLOTS_COLS];
232233
boolnulls[PG_GET_REPLICATION_SLOTS_COLS];
233-
234-
ReplicationSlotPersistencypersistency;
235-
TransactionIdxmin;
236-
TransactionIdcatalog_xmin;
237-
XLogRecPtrrestart_lsn;
238-
XLogRecPtrconfirmed_flush_lsn;
239-
pid_tactive_pid;
240-
Oiddatabase;
241-
NameDataslot_name;
242-
NameDataplugin;
243234
inti;
244235

245236
if (!slot->in_use)
246237
continue;
247238

239+
/* Copy slot contents while holding spinlock, then examine at leisure */
248240
SpinLockAcquire(&slot->mutex);
249-
250-
xmin=slot->data.xmin;
251-
catalog_xmin=slot->data.catalog_xmin;
252-
database=slot->data.database;
253-
restart_lsn=slot->data.restart_lsn;
254-
confirmed_flush_lsn=slot->data.confirmed_flush;
255-
namecpy(&slot_name,&slot->data.name);
256-
namecpy(&plugin,&slot->data.plugin);
257-
active_pid=slot->active_pid;
258-
persistency=slot->data.persistency;
259-
241+
slot_contents=*slot;
260242
SpinLockRelease(&slot->mutex);
261243

244+
memset(values,0,sizeof(values));
262245
memset(nulls,0,sizeof(nulls));
263246

264247
i=0;
265-
values[i++]=NameGetDatum(&slot_name);
248+
values[i++]=NameGetDatum(&slot_contents.data.name);
266249

267-
if (database==InvalidOid)
250+
if (slot_contents.data.database==InvalidOid)
268251
nulls[i++]= true;
269252
else
270-
values[i++]=NameGetDatum(&plugin);
253+
values[i++]=NameGetDatum(&slot_contents.data.plugin);
271254

272-
if (database==InvalidOid)
255+
if (slot_contents.data.database==InvalidOid)
273256
values[i++]=CStringGetTextDatum("physical");
274257
else
275258
values[i++]=CStringGetTextDatum("logical");
276259

277-
if (database==InvalidOid)
260+
if (slot_contents.data.database==InvalidOid)
278261
nulls[i++]= true;
279262
else
280-
values[i++]=database;
263+
values[i++]=ObjectIdGetDatum(slot_contents.data.database);
281264

282-
values[i++]=BoolGetDatum(persistency==RS_TEMPORARY);
283-
values[i++]=BoolGetDatum(active_pid!=0);
265+
values[i++]=BoolGetDatum(slot_contents.data.persistency==RS_TEMPORARY);
266+
values[i++]=BoolGetDatum(slot_contents.active_pid!=0);
284267

285-
if (active_pid!=0)
286-
values[i++]=Int32GetDatum(active_pid);
268+
if (slot_contents.active_pid!=0)
269+
values[i++]=Int32GetDatum(slot_contents.active_pid);
287270
else
288271
nulls[i++]= true;
289272

290-
if (xmin!=InvalidTransactionId)
291-
values[i++]=TransactionIdGetDatum(xmin);
273+
if (slot_contents.data.xmin!=InvalidTransactionId)
274+
values[i++]=TransactionIdGetDatum(slot_contents.data.xmin);
292275
else
293276
nulls[i++]= true;
294277

295-
if (catalog_xmin!=InvalidTransactionId)
296-
values[i++]=TransactionIdGetDatum(catalog_xmin);
278+
if (slot_contents.data.catalog_xmin!=InvalidTransactionId)
279+
values[i++]=TransactionIdGetDatum(slot_contents.data.catalog_xmin);
297280
else
298281
nulls[i++]= true;
299282

300-
if (restart_lsn!=InvalidXLogRecPtr)
301-
values[i++]=LSNGetDatum(restart_lsn);
283+
if (slot_contents.data.restart_lsn!=InvalidXLogRecPtr)
284+
values[i++]=LSNGetDatum(slot_contents.data.restart_lsn);
302285
else
303286
nulls[i++]= true;
304287

305-
if (confirmed_flush_lsn!=InvalidXLogRecPtr)
306-
values[i++]=LSNGetDatum(confirmed_flush_lsn);
288+
if (slot_contents.data.confirmed_flush!=InvalidXLogRecPtr)
289+
values[i++]=LSNGetDatum(slot_contents.data.confirmed_flush);
307290
else
308291
nulls[i++]= true;
309292

293+
Assert(i==PG_GET_REPLICATION_SLOTS_COLS);
294+
310295
tuplestore_putvalues(tupstore,tupdesc,values,nulls);
311296
}
297+
312298
LWLockRelease(ReplicationSlotControlLock);
313299

314300
tuplestore_donestoring(tupstore);

‎src/include/replication/slot.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -151,8 +151,8 @@ typedef struct ReplicationSlot
151151
XLogRecPtrcandidate_restart_lsn;
152152
}ReplicationSlot;
153153

154-
#defineSlotIsPhysical(slot) (slot->data.database == InvalidOid)
155-
#defineSlotIsLogical(slot) (slot->data.database != InvalidOid)
154+
#defineSlotIsPhysical(slot) ((slot)->data.database == InvalidOid)
155+
#defineSlotIsLogical(slot) ((slot)->data.database != InvalidOid)
156156

157157
/*
158158
* Shared memory control area for all of replication slots.

0 commit comments

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