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

Commitcc811f9

Browse files
committed
Adjust signature of cluster_rel() and its subroutines
cluster_rel() receives the OID of the relation to process, which itopens and locks; but then its subroutine copy_table_data() also receivesthe relation OID and opens it by itself. This is a bit wasteful. It'sbetter to have cluster_rel() receive the relation already open, and passit down to its subroutines as necessary; then cluster_rel closes the relbefore returning. This simplifies things.But a better motivation to make this change is that a future command todo logical-decoding-based "concurrent VACUUM FULL" will need to releaseall locks on the relation (and possibly on the clustering index) at somepoint. Since it makes little sense to keep the relation referencewithout the lock, the cluster_rel() function will also close it (andthe index). With this arrangement, neither the function nor itssubroutines need open extra references, which, again, makes things simpler.Author: Antonin Houska <ah@cybertec.at>Discussion:https://postgr.es/m/82651.1720540558@antos
1 parent2310064 commitcc811f9

File tree

4 files changed

+75
-73
lines changed

4 files changed

+75
-73
lines changed

‎src/backend/commands/cluster.c

Lines changed: 69 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,8 @@ typedef struct
6969

7070

7171
staticvoidcluster_multiple_rels(List*rtcs,ClusterParams*params);
72-
staticvoidrebuild_relation(RelationOldHeap,OidindexOid,boolverbose);
73-
staticvoidcopy_table_data(OidOIDNewHeap,OidOIDOldHeap,OidOIDOldIndex,
72+
staticvoidrebuild_relation(RelationOldHeap,Relationindex,boolverbose);
73+
staticvoidcopy_table_data(RelationNewHeap,RelationOldHeap,RelationOldIndex,
7474
boolverbose,bool*pSwapToastByContent,
7575
TransactionId*pFreezeXid,MultiXactId*pCutoffMulti);
7676
staticList*get_tables_to_cluster(MemoryContextcluster_context);
@@ -191,13 +191,11 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
191191
stmt->indexname,stmt->relation->relname)));
192192
}
193193

194+
/* For non-partitioned tables, do what we came here to do. */
194195
if (rel->rd_rel->relkind!=RELKIND_PARTITIONED_TABLE)
195196
{
196-
/* close relation, keep lock till commit */
197-
table_close(rel,NoLock);
198-
199-
/* Do the job. */
200-
cluster_rel(tableOid,indexOid,&params);
197+
cluster_rel(rel,indexOid,&params);
198+
/* cluster_rel closes the relation, but keeps lock */
201199

202200
return;
203201
}
@@ -274,15 +272,19 @@ cluster_multiple_rels(List *rtcs, ClusterParams *params)
274272
foreach(lc,rtcs)
275273
{
276274
RelToCluster*rtc= (RelToCluster*)lfirst(lc);
275+
Relationrel;
277276

278277
/* Start a new transaction for each relation. */
279278
StartTransactionCommand();
280279

281280
/* functions in indexes may want a snapshot set */
282281
PushActiveSnapshot(GetTransactionSnapshot());
283282

284-
/* Do the job. */
285-
cluster_rel(rtc->tableOid,rtc->indexOid,params);
283+
rel=table_open(rtc->tableOid,AccessExclusiveLock);
284+
285+
/* Process this table */
286+
cluster_rel(rel,rtc->indexOid,params);
287+
/* cluster_rel closes the relation, but keeps lock */
286288

287289
PopActiveSnapshot();
288290
CommitTransactionCommand();
@@ -295,8 +297,7 @@ cluster_multiple_rels(List *rtcs, ClusterParams *params)
295297
* This clusters the table by creating a new, clustered table and
296298
* swapping the relfilenumbers of the new table and the old table, so
297299
* the OID of the original table is preserved. Thus we do not lose
298-
* GRANT, inheritance nor references to this table (this was a bug
299-
* in releases through 7.3).
300+
* GRANT, inheritance nor references to this table.
300301
*
301302
* Indexes are rebuilt too, via REINDEX. Since we are effectively bulk-loading
302303
* the new table, it's better to create the indexes afterwards than to fill
@@ -307,14 +308,17 @@ cluster_multiple_rels(List *rtcs, ClusterParams *params)
307308
* and error messages should refer to the operation as VACUUM not CLUSTER.
308309
*/
309310
void
310-
cluster_rel(OidtableOid,OidindexOid,ClusterParams*params)
311+
cluster_rel(RelationOldHeap,OidindexOid,ClusterParams*params)
311312
{
312-
RelationOldHeap;
313+
OidtableOid=RelationGetRelid(OldHeap);
313314
Oidsave_userid;
314315
intsave_sec_context;
315316
intsave_nestlevel;
316317
boolverbose= ((params->options&CLUOPT_VERBOSE)!=0);
317318
boolrecheck= ((params->options&CLUOPT_RECHECK)!=0);
319+
Relationindex;
320+
321+
Assert(CheckRelationLockedByMe(OldHeap,AccessExclusiveLock, false));
318322

319323
/* Check for user-requested abort. */
320324
CHECK_FOR_INTERRUPTS();
@@ -327,21 +331,6 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params)
327331
pgstat_progress_update_param(PROGRESS_CLUSTER_COMMAND,
328332
PROGRESS_CLUSTER_COMMAND_VACUUM_FULL);
329333

330-
/*
331-
* We grab exclusive access to the target rel and index for the duration
332-
* of the transaction. (This is redundant for the single-transaction
333-
* case, since cluster() already did it.) The index lock is taken inside
334-
* check_index_is_clusterable.
335-
*/
336-
OldHeap=try_relation_open(tableOid,AccessExclusiveLock);
337-
338-
/* If the table has gone away, we can skip processing it */
339-
if (!OldHeap)
340-
{
341-
pgstat_progress_end_command();
342-
return;
343-
}
344-
345334
/*
346335
* Switch to the table owner's userid, so that any index functions are run
347336
* as that user. Also lock down security-restricted operations and
@@ -444,7 +433,14 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params)
444433

445434
/* Check heap and index are valid to cluster on */
446435
if (OidIsValid(indexOid))
436+
{
437+
/* verify the index is good and lock it */
447438
check_index_is_clusterable(OldHeap,indexOid,AccessExclusiveLock);
439+
/* also open it */
440+
index=index_open(indexOid,NoLock);
441+
}
442+
else
443+
index=NULL;
448444

449445
/*
450446
* Quietly ignore the request if this is a materialized view which has not
@@ -473,9 +469,8 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params)
473469
TransferPredicateLocksToHeapRelation(OldHeap);
474470

475471
/* rebuild_relation does all the dirty work */
476-
rebuild_relation(OldHeap,indexOid,verbose);
477-
478-
/* NB: rebuild_relation does table_close() on OldHeap */
472+
rebuild_relation(OldHeap,index,verbose);
473+
/* rebuild_relation closes OldHeap, and index if valid */
479474

480475
out:
481476
/* Roll back any GUC changes executed by index functions */
@@ -623,45 +618,68 @@ mark_index_clustered(Relation rel, Oid indexOid, bool is_internal)
623618
/*
624619
* rebuild_relation: rebuild an existing relation in index or physical order
625620
*
626-
* OldHeap: table to rebuild --- must be opened and exclusive-locked!
627-
*indexOid: index to cluster by, orInvalidOid to rewrite in physical order.
621+
* OldHeap: table to rebuild.
622+
*index: index to cluster by, orNULL to rewrite in physical order.
628623
*
629-
* NB: this routine closes OldHeap at the right time; caller should not.
624+
* On entry, heap and index (if one is given) must be open, and
625+
* AccessExclusiveLock held on them.
626+
* On exit, they are closed, but locks on them are not released.
630627
*/
631628
staticvoid
632-
rebuild_relation(RelationOldHeap,OidindexOid,boolverbose)
629+
rebuild_relation(RelationOldHeap,Relationindex,boolverbose)
633630
{
634631
OidtableOid=RelationGetRelid(OldHeap);
635632
OidaccessMethod=OldHeap->rd_rel->relam;
636633
OidtableSpace=OldHeap->rd_rel->reltablespace;
637634
OidOIDNewHeap;
635+
RelationNewHeap;
638636
charrelpersistence;
639637
boolis_system_catalog;
640638
boolswap_toast_by_content;
641639
TransactionIdfrozenXid;
642640
MultiXactIdcutoffMulti;
643641

644-
if (OidIsValid(indexOid))
642+
Assert(CheckRelationLockedByMe(OldHeap,AccessExclusiveLock, false)&&
643+
(index==NULL||CheckRelationLockedByMe(index,AccessExclusiveLock, false)));
644+
645+
if (index)
645646
/* Mark the correct index as clustered */
646-
mark_index_clustered(OldHeap,indexOid, true);
647+
mark_index_clustered(OldHeap,RelationGetRelid(index), true);
647648

648649
/* Remember info about rel before closing OldHeap */
649650
relpersistence=OldHeap->rd_rel->relpersistence;
650651
is_system_catalog=IsSystemRelation(OldHeap);
651652

652-
/* Close relcache entry, but keep lock until transaction commit */
653-
table_close(OldHeap,NoLock);
654-
655-
/* Create the transient table that will receive the re-ordered data */
653+
/*
654+
* Create the transient table that will receive the re-ordered data.
655+
*
656+
* OldHeap is already locked, so no need to lock it again. make_new_heap
657+
* obtains AccessExclusiveLock on the new heap and its toast table.
658+
*/
656659
OIDNewHeap=make_new_heap(tableOid,tableSpace,
657660
accessMethod,
658661
relpersistence,
659-
AccessExclusiveLock);
662+
NoLock);
663+
Assert(CheckRelationOidLockedByMe(OIDNewHeap,AccessExclusiveLock, false));
664+
NewHeap=table_open(OIDNewHeap,NoLock);
660665

661666
/* Copy the heap data into the new table in the desired order */
662-
copy_table_data(OIDNewHeap,tableOid,indexOid,verbose,
667+
copy_table_data(NewHeap,OldHeap,index,verbose,
663668
&swap_toast_by_content,&frozenXid,&cutoffMulti);
664669

670+
671+
/* Close relcache entries, but keep lock until transaction commit */
672+
table_close(OldHeap,NoLock);
673+
if (index)
674+
index_close(index,NoLock);
675+
676+
/*
677+
* Close the new relation so it can be dropped as soon as the storage is
678+
* swapped. The relation is not visible to others, so no need to unlock it
679+
* explicitly.
680+
*/
681+
table_close(NewHeap,NoLock);
682+
665683
/*
666684
* Swap the physical files of the target and transient tables, then
667685
* rebuild the target's indexes and throw away the transient table.
@@ -810,13 +828,10 @@ make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, Oid NewAccessMethod,
810828
* *pCutoffMulti receives the MultiXactId used as a cutoff point.
811829
*/
812830
staticvoid
813-
copy_table_data(OidOIDNewHeap,OidOIDOldHeap,OidOIDOldIndex,boolverbose,
831+
copy_table_data(RelationNewHeap,RelationOldHeap,RelationOldIndex,boolverbose,
814832
bool*pSwapToastByContent,TransactionId*pFreezeXid,
815833
MultiXactId*pCutoffMulti)
816834
{
817-
RelationNewHeap,
818-
OldHeap,
819-
OldIndex;
820835
RelationrelRelation;
821836
HeapTuplereltup;
822837
Form_pg_classrelform;
@@ -835,16 +850,6 @@ copy_table_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex, bool verbose,
835850

836851
pg_rusage_init(&ru0);
837852

838-
/*
839-
* Open the relations we need.
840-
*/
841-
NewHeap=table_open(OIDNewHeap,AccessExclusiveLock);
842-
OldHeap=table_open(OIDOldHeap,AccessExclusiveLock);
843-
if (OidIsValid(OIDOldIndex))
844-
OldIndex=index_open(OIDOldIndex,AccessExclusiveLock);
845-
else
846-
OldIndex=NULL;
847-
848853
/* Store a copy of the namespace name for logging purposes */
849854
nspname=get_namespace_name(RelationGetNamespace(OldHeap));
850855

@@ -945,7 +950,8 @@ copy_table_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex, bool verbose,
945950
* provided, else plain seqscan.
946951
*/
947952
if (OldIndex!=NULL&&OldIndex->rd_rel->relam==BTREE_AM_OID)
948-
use_sort=plan_cluster_use_sort(OIDOldHeap,OIDOldIndex);
953+
use_sort=plan_cluster_use_sort(RelationGetRelid(OldHeap),
954+
RelationGetRelid(OldIndex));
949955
else
950956
use_sort= false;
951957

@@ -1000,24 +1006,21 @@ copy_table_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex, bool verbose,
10001006
tups_recently_dead,
10011007
pg_rusage_show(&ru0))));
10021008

1003-
if (OldIndex!=NULL)
1004-
index_close(OldIndex,NoLock);
1005-
table_close(OldHeap,NoLock);
1006-
table_close(NewHeap,NoLock);
1007-
10081009
/* Update pg_class to reflect the correct values of pages and tuples. */
10091010
relRelation=table_open(RelationRelationId,RowExclusiveLock);
10101011

1011-
reltup=SearchSysCacheCopy1(RELOID,ObjectIdGetDatum(OIDNewHeap));
1012+
reltup=SearchSysCacheCopy1(RELOID,
1013+
ObjectIdGetDatum(RelationGetRelid(NewHeap)));
10121014
if (!HeapTupleIsValid(reltup))
1013-
elog(ERROR,"cache lookup failed for relation %u",OIDNewHeap);
1015+
elog(ERROR,"cache lookup failed for relation %u",
1016+
RelationGetRelid(NewHeap));
10141017
relform= (Form_pg_class)GETSTRUCT(reltup);
10151018

10161019
relform->relpages=num_pages;
10171020
relform->reltuples=num_tuples;
10181021

10191022
/* Don't update the stats for pg_class. See swap_relation_files. */
1020-
if (OIDOldHeap!=RelationRelationId)
1023+
if (RelationGetRelid(OldHeap)!=RelationRelationId)
10211024
CatalogTupleUpdate(relRelation,&reltup->t_self,reltup);
10221025
else
10231026
CacheInvalidateRelcacheByTuple(reltup);

‎src/backend/commands/matview.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@ RefreshMatViewByOid(Oid matviewOid, bool is_create, bool skipData,
319319
OIDNewHeap=make_new_heap(matviewOid,tableSpace,
320320
matviewRel->rd_rel->relam,
321321
relpersistence,ExclusiveLock);
322-
LockRelationOid(OIDNewHeap,AccessExclusiveLock);
322+
Assert(CheckRelationOidLockedByMe(OIDNewHeap,AccessExclusiveLock, false));
323323

324324
/* Generate the data, if wanted. */
325325
if (!skipData)

‎src/backend/commands/vacuum.c

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2218,15 +2218,14 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
22182218
{
22192219
ClusterParamscluster_params= {0};
22202220

2221-
/* close relation before vacuuming, but hold lock until commit */
2222-
relation_close(rel,NoLock);
2223-
rel=NULL;
2224-
22252221
if ((params->options&VACOPT_VERBOSE)!=0)
22262222
cluster_params.options |=CLUOPT_VERBOSE;
22272223

22282224
/* VACUUM FULL is now a variant of CLUSTER; see cluster.c */
2229-
cluster_rel(relid,InvalidOid,&cluster_params);
2225+
cluster_rel(rel,InvalidOid,&cluster_params);
2226+
/* cluster_rel closes the relation, but keeps lock */
2227+
2228+
rel=NULL;
22302229
}
22312230
else
22322231
table_relation_vacuum(rel,params,bstrategy);

‎src/include/commands/cluster.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ typedef struct ClusterParams
3232
}ClusterParams;
3333

3434
externvoidcluster(ParseState*pstate,ClusterStmt*stmt,boolisTopLevel);
35-
externvoidcluster_rel(OidtableOid,OidindexOid,ClusterParams*params);
35+
externvoidcluster_rel(RelationOldHeap,OidindexOid,ClusterParams*params);
3636
externvoidcheck_index_is_clusterable(RelationOldHeap,OidindexOid,
3737
LOCKMODElockmode);
3838
externvoidmark_index_clustered(Relationrel,OidindexOid,boolis_internal);

0 commit comments

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