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

Commitf9dd1c3

Browse files
committed
Mostly cosmetical improvements.
 * (Arguably) improved comments around locking during circular buffer maintenance; also, don't lock procarray during global_snapshot_xmin bump. * s/snaphot/snapshot, other typos. * Don't track_global_snapshots by default -- while handy for testing, it doesn't look generally good.
1 parentb469747 commitf9dd1c3

File tree

7 files changed

+53
-56
lines changed

7 files changed

+53
-56
lines changed

‎src/backend/access/transam/global_snapshot.c

Lines changed: 33 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ GlobalSnapshotShmemInit()
172172
/*
173173
* GlobalSnapshotStartup
174174
*
175-
* Set gsXidMapetnries to oldestActiveXID during startup.
175+
* Set gsXidMapentries to oldestActiveXID during startup.
176176
*/
177177
void
178178
GlobalSnapshotStartup(TransactionIdoldestActiveXID)
@@ -200,24 +200,33 @@ GlobalSnapshotStartup(TransactionId oldestActiveXID)
200200
* global transaction. Otherwise old versions of tuples that were needed for
201201
* this transaction can be recycled by other processes (vacuum, HOT, etc).
202202
*
203-
* Called upon each snapshot creation after ProcArrayLock is released. Such
204-
*usage creates a race condition. It is possible that backend who got
205-
*glabal_csn called GlobalSnapshotMapXmin() only after other backends managed
206-
*to get snapshot and complete GlobalSnapshotMapXmin() call. To address that
207-
*race we do two thigs:
203+
*Locking here is not trivial.Called upon each snapshot creation after
204+
*ProcArrayLock is released. Such usage creates several race conditions. It
205+
*is possible that backend who got global_csn called GlobalSnapshotMapXmin()
206+
*only after other backends managed to get snapshot and complete
207+
*GlobalSnapshotMapXmin() call, or even committed. This is safe because
208208
*
209-
** snapshot_global_csn is always rounded up to next second. So that is
210-
* okay if call to GlobalSnapshotMapXmin() with later global_csn will
211-
* succeed first -- it anyway will be taken into account for a next
209+
* * We already hold our xmin in MyPgXact, so our snapshot will not be
210+
* harmed even though ProcArrayLock is released.
211+
*
212+
** snapshot_global_csn is always pessmistically rounded up to the next
212213
* second.
213214
*
215+
* * For performance reasons, xmin value for particular second is filled
216+
* only once. Because of that instead of writing to buffer just our
217+
* xmin (which is enough for our snapshot), we bump oldestXmin there --
218+
* it mitigates the possibility of damaging someone else's snapshot by
219+
* writing to the buffer too advanced value in case of slowness of
220+
* another backend who generated csn earlier, but didn't manage to
221+
* insert it before us.
222+
*
214223
** if GlobalSnapshotMapXmin() founds a gap in several seconds between
215224
* current call and latest completed call then it should fill that gap
216-
* with latest known values instead of new one. Otherwise it is possible
217-
* (however highly unlikely) that this gap also happend between taking
218-
* snapshot and call to GlobalSnapshotMapXmin() for some backend. And we
219-
* are at risk to fill circullar buffer with oldestXmin's that are
220-
* bigger then they actually were.
225+
* with latest known values instead of new one. Otherwise it is
226+
*possible(however highly unlikely) that this gap also happend
227+
*between takingsnapshot and call to GlobalSnapshotMapXmin() for some
228+
*backend. And weare at risk to fill circullar buffer with
229+
*oldestXmin's that arebigger then they actually were.
221230
*/
222231
void
223232
GlobalSnapshotMapXmin(GlobalCSNsnapshot_global_csn)
@@ -233,10 +242,7 @@ GlobalSnapshotMapXmin(GlobalCSN snapshot_global_csn)
233242
Assert(gsXidMap!=NULL);
234243

235244
/*
236-
* We don't have guarantee that process who called us first for this
237-
* csn_seconds is actually one who took snapshot firt in this second.
238-
* So just round up global_csn to the next second -- snapshots for next
239-
* second would have oldestXmin greater or equal then ours anyway.
245+
* Round up global_csn to the next second -- pessimistically and safely.
240246
*/
241247
csn_seconds= (snapshot_global_csn /NSECS_PER_SEC+1);
242248

@@ -290,16 +296,15 @@ GlobalSnapshotMapXmin(GlobalCSN snapshot_global_csn)
290296

291297
/* Fill new entry with current_oldest_xmin */
292298
gsXidMap->xmin_by_second[offset]=current_oldest_xmin;
293-
offset= (offset+gsXidMap->size-1) %gsXidMap->size;
294299

295300
/*
296301
* If we have gap then fill it with previous_oldest_xmin for reasons
297302
* outlined in comment above this function.
298303
*/
299304
for (i=1;i<gap;i++)
300305
{
301-
gsXidMap->xmin_by_second[offset]=previous_oldest_xmin;
302306
offset= (offset+gsXidMap->size-1) %gsXidMap->size;
307+
gsXidMap->xmin_by_second[offset]=previous_oldest_xmin;
303308
}
304309

305310
oldest_deferred_xmin=
@@ -309,8 +314,11 @@ GlobalSnapshotMapXmin(GlobalCSN snapshot_global_csn)
309314

310315
/*
311316
* Advance procArray->global_snapshot_xmin after we released
312-
* GlobalSnapshotXidMapLock.
317+
* GlobalSnapshotXidMapLock. Since we gather not xmin but oldestXmin, it
318+
* never goes backwards regardless of how slow we can do that.
313319
*/
320+
Assert(TransactionIdFollowsOrEquals(oldest_deferred_xmin,
321+
ProcArrayGetGlobalSnapshotXmin()));
314322
ProcArraySetGlobalSnapshotXmin(oldest_deferred_xmin);
315323
}
316324

@@ -554,7 +562,7 @@ XidInvisibleInGlobalSnapshot(TransactionId xid, Snapshot snapshot)
554562
* Functions to handle distributed commit on transaction coordinator:
555563
* GlobalSnapshotPrepareCurrent() / GlobalSnapshotAssignCsnCurrent().
556564
* Correspoding functions for remote nodes are defined in twophase.c:
557-
*pg_global_snaphot_prepare/pg_global_snaphot_assign.
565+
*pg_global_snapshot_prepare/pg_global_snapshot_assign.
558566
*****************************************************************************/
559567

560568

@@ -594,7 +602,7 @@ GlobalSnapshotPrepareCurrent()
594602
*
595603
* Asign GlobalCSN for currently active transaction. GlobalCSN is supposedly
596604
* maximal among of values returned by GlobalSnapshotPrepareCurrent and
597-
*pg_global_snaphot_prepare.
605+
*pg_global_snapshot_prepare.
598606
*/
599607
void
600608
GlobalSnapshotAssignCsnCurrent(GlobalCSNglobal_csn)
@@ -609,7 +617,7 @@ GlobalSnapshotAssignCsnCurrent(GlobalCSN global_csn)
609617
if (!GlobalCSNIsNormal(global_csn))
610618
ereport(ERROR,
611619
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
612-
errmsg("pg_global_snaphot_assign expects normal global_csn")));
620+
errmsg("pg_global_snapshot_assign expects normal global_csn")));
613621

614622
/* Skip emtpty transactions */
615623
if (!TransactionIdIsValid(GetCurrentTransactionIdIfAny()))
@@ -633,7 +641,7 @@ GlobalSnapshotAssignCsnCurrent(GlobalCSN global_csn)
633641
* proc->assignedGlobalCsn to GlobalCSNLog.
634642
*
635643
* Same rules applies to global transaction, except that global_csn is already
636-
* assigned by GlobalSnapshotAssignCsnCurrent/pg_global_snaphot_assign and
644+
* assigned by GlobalSnapshotAssignCsnCurrent/pg_global_snapshot_assign and
637645
* GlobalSnapshotPrecommit is basically no-op.
638646
*
639647
* GlobalSnapshotAbort is slightly different comparing to commit because abort

‎src/backend/access/transam/twophase.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2601,7 +2601,7 @@ GlobalSnapshotPrepareTwophase(const char *gid)
26012601
* TODO: Rewrite this as PREPARE TRANSACTION 'gid' RETURNING SNAPSHOT
26022602
*/
26032603
Datum
2604-
pg_global_snaphot_prepare(PG_FUNCTION_ARGS)
2604+
pg_global_snapshot_prepare(PG_FUNCTION_ARGS)
26052605
{
26062606
constchar*gid=text_to_cstring(PG_GETARG_TEXT_PP(0));
26072607
GlobalCSNglobal_csn;
@@ -2617,7 +2617,7 @@ pg_global_snaphot_prepare(PG_FUNCTION_ARGS)
26172617
*
26182618
* Asign GlobalCSN for currently active transaction. GlobalCSN is supposedly
26192619
* maximal among of values returned by GlobalSnapshotPrepareCurrent and
2620-
*pg_global_snaphot_prepare.
2620+
*pg_global_snapshot_prepare.
26212621
*
26222622
* This function is a counterpart of GlobalSnapshotAssignCsnCurrent() for
26232623
* twophase transactions.
@@ -2638,7 +2638,7 @@ GlobalSnapshotAssignCsnTwoPhase(const char *gid, GlobalCSN global_csn)
26382638
if (!GlobalCSNIsNormal(global_csn))
26392639
ereport(ERROR,
26402640
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
2641-
errmsg("pg_global_snaphot_assign expects normal global_csn")));
2641+
errmsg("pg_global_snapshot_assign expects normal global_csn")));
26422642

26432643
/*
26442644
* Validate the GID, and lock the GXACT to ensure that two backends do not
@@ -2662,7 +2662,7 @@ GlobalSnapshotAssignCsnTwoPhase(const char *gid, GlobalCSN global_csn)
26622662
* TODO: Rewrite this as COMMIT PREPARED 'gid' SNAPSHOT 'global_csn'
26632663
*/
26642664
Datum
2665-
pg_global_snaphot_assign(PG_FUNCTION_ARGS)
2665+
pg_global_snapshot_assign(PG_FUNCTION_ARGS)
26662666
{
26672667
constchar*gid=text_to_cstring(PG_GETARG_TEXT_PP(0));
26682668
GlobalCSNglobal_csn=PG_GETARG_INT64(1);

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

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3083,28 +3083,17 @@ ProcArrayGetReplicationSlotXmin(TransactionId *xmin,
30833083
void
30843084
ProcArraySetGlobalSnapshotXmin(TransactionIdxmin)
30853085
{
3086-
LWLockAcquire(ProcArrayLock,LW_SHARED);
3087-
3088-
if (TransactionIdFollows(xmin,procArray->global_snapshot_xmin))
3089-
procArray->global_snapshot_xmin=xmin;
3090-
3091-
LWLockRelease(ProcArrayLock);
3086+
/* We rely on atomic fetch/store of xid */
3087+
procArray->global_snapshot_xmin=xmin;
30923088
}
30933089

30943090
/*
30953091
* ProcArrayGetGlobalSnapshotXmin
30963092
*/
30973093
TransactionId
3098-
ProcArrayGetGlobalSnapshotXmin(boollocked)
3094+
ProcArrayGetGlobalSnapshotXmin()
30993095
{
3100-
volatileTransactionIdxmin;
3101-
3102-
if (!locked)
3103-
LWLockAcquire(ProcArrayLock,LW_SHARED);
3104-
xmin=procArray->global_snapshot_xmin;
3105-
if (!locked)
3106-
LWLockRelease(ProcArrayLock);
3107-
returnxmin;
3096+
returnprocArray->global_snapshot_xmin;
31083097
}
31093098

31103099
#defineXidCacheRemove(i) \

‎src/backend/utils/misc/guc.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1018,7 +1018,7 @@ static struct config_bool ConfigureNamesBool[] =
10181018
gettext_noop("Used to achieve REPEATEBLE READ isolation level for postgres_fdw transactions.")
10191019
},
10201020
&track_global_snapshots,
1021-
true,/* XXX: set true to simplify tesing. XXX2: Seems that RESOURCES_MEM isn't the best catagory */
1021+
false,/* XXX: Seems that RESOURCES_MEM isn't the best catagory */
10221022
NULL,NULL,NULL
10231023
},
10241024
{

‎src/backend/utils/time/snapmgr.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2227,7 +2227,7 @@ ExportGlobalSnapshot()
22272227

22282228
/* SQL accessor to ExportGlobalSnapshot() */
22292229
Datum
2230-
pg_global_snaphot_export(PG_FUNCTION_ARGS)
2230+
pg_global_snapshot_export(PG_FUNCTION_ARGS)
22312231
{
22322232
GlobalCSNglobal_csn=ExportGlobalSnapshot();
22332233
PG_RETURN_UINT64(global_csn);
@@ -2289,7 +2289,7 @@ ImportGlobalSnapshot(GlobalCSN snap_global_csn)
22892289

22902290
/* SQL accessor to ImportGlobalSnapshot() */
22912291
Datum
2292-
pg_global_snaphot_import(PG_FUNCTION_ARGS)
2292+
pg_global_snapshot_import(PG_FUNCTION_ARGS)
22932293
{
22942294
GlobalCSNglobal_csn=PG_GETARG_UINT64(0);
22952295
ImportGlobalSnapshot(global_csn);

‎src/include/catalog/pg_proc.dat

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10208,16 +10208,16 @@
1020810208

1020910209
# global transaction handling
1021010210
{ oid => '3430', descr => 'export global transaction snapshot',
10211-
proname => 'pg_global_snaphot_export', provolatile => 'v', proparallel => 'u',
10212-
prorettype => 'int8', proargtypes => '', prosrc => 'pg_global_snaphot_export' },
10211+
proname => 'pg_global_snapshot_export', provolatile => 'v', proparallel => 'u',
10212+
prorettype => 'int8', proargtypes => '', prosrc => 'pg_global_snapshot_export' },
1021310213
{ oid => '3431', descr => 'import global transaction snapshot',
10214-
proname => 'pg_global_snaphot_import', provolatile => 'v', proparallel => 'u',
10215-
prorettype => 'void', proargtypes => 'int8', prosrc => 'pg_global_snaphot_import' },
10214+
proname => 'pg_global_snapshot_import', provolatile => 'v', proparallel => 'u',
10215+
prorettype => 'void', proargtypes => 'int8', prosrc => 'pg_global_snapshot_import' },
1021610216
{ oid => '3432', descr => 'prepare distributed transaction for commit, get global_csn',
10217-
proname => 'pg_global_snaphot_prepare', provolatile => 'v', proparallel => 'u',
10218-
prorettype => 'int8', proargtypes => 'text', prosrc => 'pg_global_snaphot_prepare' },
10217+
proname => 'pg_global_snapshot_prepare', provolatile => 'v', proparallel => 'u',
10218+
prorettype => 'int8', proargtypes => 'text', prosrc => 'pg_global_snapshot_prepare' },
1021910219
{ oid => '3433', descr => 'assign global_csn to distributed transaction',
10220-
proname => 'pg_global_snaphot_assign', provolatile => 'v', proparallel => 'u',
10221-
prorettype => 'void', proargtypes => 'text int8', prosrc => 'pg_global_snaphot_assign' },
10220+
proname => 'pg_global_snapshot_assign', provolatile => 'v', proparallel => 'u',
10221+
prorettype => 'void', proargtypes => 'text int8', prosrc => 'pg_global_snapshot_assign' },
1022210222

1022310223
]

‎src/include/storage/procarray.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,6 @@ extern void ProcArrayGetReplicationSlotXmin(TransactionId *xmin,
130130

131131
externvoidProcArraySetGlobalSnapshotXmin(TransactionIdxmin);
132132

133-
externTransactionIdProcArrayGetGlobalSnapshotXmin(boollocked);
133+
externTransactionIdProcArrayGetGlobalSnapshotXmin();
134134

135135
#endif/* PROCARRAY_H */

0 commit comments

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