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

Commit27f5c71

Browse files
committed
Fix CREATE INDEX progress reporting for multi-level partitioning.
The "partitions_total" and "partitions_done" fields were updatedas though the current level of partitioning was the only one.In multi-level cases, not only could partitions_total changeover the course of the command, but partitions_done could gobackwards or exceed the currently-reported partitions_total.Fix by setting partitions_total to the total number of directand indirect children once at command start, and then justincrementing partitions_done at appropriate points. Inventa new progress monitoring function "pgstat_progress_incr_param"to simplify doing the latter. We can avoid adding cost for theformer when doing CREATE INDEX, because ProcessUtility alreadyenumerates the children and it's pretty easy to pass the countdown to DefineIndex. In principle the same could be done inALTER TABLE, but that's structurally difficult; for now, justeat the cost of an extra find_all_inheritors scan in that case.Ilya Gladyshev and Justin PryzbyDiscussion:https://postgr.es/m/a15f904a70924ffa4ca25c3c744cff31e0e6e143.camel@gmail.com
1 parent81a6d57 commit27f5c71

File tree

8 files changed

+106
-15
lines changed

8 files changed

+106
-15
lines changed

‎doc/src/sgml/monitoring.sgml

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6817,7 +6817,7 @@ FROM pg_stat_get_backend_idset() AS backendid;
68176817
<structfield>pid</structfield> <type>integer</type>
68186818
</para>
68196819
<para>
6820-
Process ID of backend.
6820+
Process ID ofthebackend creating indexes.
68216821
</para></entry>
68226822
</row>
68236823

@@ -6863,7 +6863,7 @@ FROM pg_stat_get_backend_idset() AS backendid;
68636863
<structfield>command</structfield> <type>text</type>
68646864
</para>
68656865
<para>
6866-
The commandthat is running: <literal>CREATE INDEX</literal>,
6866+
Specific commandtype: <literal>CREATE INDEX</literal>,
68676867
<literal>CREATE INDEX CONCURRENTLY</literal>,
68686868
<literal>REINDEX</literal>, or <literal>REINDEX CONCURRENTLY</literal>.
68696869
</para></entry>
@@ -6946,9 +6946,10 @@ FROM pg_stat_get_backend_idset() AS backendid;
69466946
<structfield>partitions_total</structfield> <type>bigint</type>
69476947
</para>
69486948
<para>
6949-
When creating an index on a partitioned table, this column is set to
6950-
the total number of partitions on which the index is to be created.
6951-
This field is <literal>0</literal> during a <literal>REINDEX</literal>.
6949+
Total number of partitions on which the index is to be created
6950+
or attached, including both direct and indirect partitions.
6951+
<literal>0</literal> during a <literal>REINDEX</literal>, or when
6952+
the index is not partitioned.
69526953
</para></entry>
69536954
</row>
69546955

@@ -6957,9 +6958,10 @@ FROM pg_stat_get_backend_idset() AS backendid;
69576958
<structfield>partitions_done</structfield> <type>bigint</type>
69586959
</para>
69596960
<para>
6960-
When creating an index on a partitioned table, this column is set to
6961-
the number of partitions on which the index has been created.
6962-
This field is <literal>0</literal> during a <literal>REINDEX</literal>.
6961+
Number of partitions on which the index has already been created
6962+
or attached, including both direct and indirect partitions.
6963+
<literal>0</literal> during a <literal>REINDEX</literal>, or when
6964+
the index is not partitioned.
69636965
</para></entry>
69646966
</row>
69656967
</tbody>

‎src/backend/bootstrap/bootparse.y

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,7 @@ Boot_DeclareIndexStmt:
306306
$4,
307307
InvalidOid,
308308
InvalidOid,
309+
-1,
309310
false,
310311
false,
311312
false,
@@ -358,6 +359,7 @@ Boot_DeclareUniqueIndexStmt:
358359
$5,
359360
InvalidOid,
360361
InvalidOid,
362+
-1,
361363
false,
362364
false,
363365
false,

‎src/backend/commands/indexcmds.c

Lines changed: 59 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -513,6 +513,8 @@ WaitForOlderSnapshots(TransactionId limitXmin, bool progress)
513513
*of a partitioned index.
514514
* 'parentConstraintId': the OID of the parent constraint; InvalidOid if not
515515
*the child of a constraint (only used when recursing)
516+
* 'total_parts': total number of direct and indirect partitions of relation;
517+
*pass -1 if not known or rel is not partitioned.
516518
* 'is_alter_table': this is due to an ALTER rather than a CREATE operation.
517519
* 'check_rights': check for CREATE rights in namespace and tablespace. (This
518520
*should be true except when ALTER is deleting/recreating an index.)
@@ -530,6 +532,7 @@ DefineIndex(Oid relationId,
530532
OidindexRelationId,
531533
OidparentIndexId,
532534
OidparentConstraintId,
535+
inttotal_parts,
533536
boolis_alter_table,
534537
boolcheck_rights,
535538
boolcheck_not_in_use,
@@ -1225,8 +1228,37 @@ DefineIndex(Oid relationId,
12251228
RelationparentIndex;
12261229
TupleDescparentDesc;
12271230

1228-
pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_TOTAL,
1229-
nparts);
1231+
/*
1232+
* Report the total number of partitions at the start of the
1233+
* command; don't update it when being called recursively.
1234+
*/
1235+
if (!OidIsValid(parentIndexId))
1236+
{
1237+
/*
1238+
* When called by ProcessUtilitySlow, the number of partitions
1239+
* is passed in as an optimization; but other callers pass -1
1240+
* since they don't have the value handy. This should count
1241+
* partitions the same way, ie one less than the number of
1242+
* relations find_all_inheritors reports.
1243+
*
1244+
* We assume we needn't ask find_all_inheritors to take locks,
1245+
* because that should have happened already for all callers.
1246+
* Even if it did not, this is safe as long as we don't try to
1247+
* touch the partitions here; the worst consequence would be a
1248+
* bogus progress-reporting total.
1249+
*/
1250+
if (total_parts<0)
1251+
{
1252+
List*children=find_all_inheritors(relationId,
1253+
NoLock,NULL);
1254+
1255+
total_parts=list_length(children)-1;
1256+
list_free(children);
1257+
}
1258+
1259+
pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_TOTAL,
1260+
total_parts);
1261+
}
12301262

12311263
/* Make a local copy of partdesc->oids[], just for safety */
12321264
memcpy(part_oids,partdesc->oids,sizeof(Oid)*nparts);
@@ -1353,6 +1385,18 @@ DefineIndex(Oid relationId,
13531385
invalidate_parent= true;
13541386

13551387
found= true;
1388+
1389+
/*
1390+
* Report this partition as processed. Note that if
1391+
* the partition has children itself, we'd ideally
1392+
* count the children and update the progress report
1393+
* for all of them; but that seems unduly expensive.
1394+
* Instead, the progress report will act like all such
1395+
* indirect children were processed in zero time at
1396+
* the end of the command.
1397+
*/
1398+
pgstat_progress_incr_param(PROGRESS_CREATEIDX_PARTITIONS_DONE,1);
1399+
13561400
/* keep lock till commit */
13571401
index_close(cldidx,NoLock);
13581402
break;
@@ -1432,14 +1476,13 @@ DefineIndex(Oid relationId,
14321476
InvalidOid,/* no predefined OID */
14331477
indexRelationId,/* this is our child */
14341478
createdConstraintId,
1479+
-1,
14351480
is_alter_table,check_rights,check_not_in_use,
14361481
skip_build,quiet);
14371482
SetUserIdAndSecContext(child_save_userid,
14381483
child_save_sec_context);
14391484
}
14401485

1441-
pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_DONE,
1442-
i+1);
14431486
free_attrmap(attmap);
14441487
}
14451488

@@ -1479,6 +1522,12 @@ DefineIndex(Oid relationId,
14791522
table_close(rel,NoLock);
14801523
if (!OidIsValid(parentIndexId))
14811524
pgstat_progress_end_command();
1525+
else
1526+
{
1527+
/* Update progress for an intermediate partitioned index itself */
1528+
pgstat_progress_incr_param(PROGRESS_CREATEIDX_PARTITIONS_DONE,1);
1529+
}
1530+
14821531
returnaddress;
14831532
}
14841533

@@ -1490,9 +1539,14 @@ DefineIndex(Oid relationId,
14901539
/* Close the heap and we're done, in the non-concurrent case */
14911540
table_close(rel,NoLock);
14921541

1493-
/* If this is the top-level index, we're done. */
1542+
/*
1543+
* If this is the top-level index, the command is done overall;
1544+
* otherwise, increment progress to report one child index is done.
1545+
*/
14941546
if (!OidIsValid(parentIndexId))
14951547
pgstat_progress_end_command();
1548+
else
1549+
pgstat_progress_incr_param(PROGRESS_CREATEIDX_PARTITIONS_DONE,1);
14961550

14971551
returnaddress;
14981552
}

‎src/backend/commands/tablecmds.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1216,6 +1216,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
12161216
InvalidOid,
12171217
RelationGetRelid(idxRel),
12181218
constraintOid,
1219+
-1,
12191220
false, false, false, false, false);
12201221

12211222
index_close(idxRel, AccessShareLock);
@@ -8640,6 +8641,7 @@ ATExecAddIndex(AlteredTableInfo *tab, Relation rel,
86408641
InvalidOid,/* no predefined OID */
86418642
InvalidOid,/* no parent index */
86428643
InvalidOid,/* no parent constraint */
8644+
-1,/* total_parts unknown */
86438645
true, /* is_alter_table */
86448646
check_rights,
86458647
false,/* check_not_in_use - we did it already */
@@ -18106,6 +18108,7 @@ AttachPartitionEnsureIndexes(Relation rel, Relation attachrel)
1810618108
DefineIndex(RelationGetRelid(attachrel), stmt, InvalidOid,
1810718109
RelationGetRelid(idxRel),
1810818110
conOid,
18111+
-1,
1810918112
true, false, false, false, false);
1811018113
}
1811118114

‎src/backend/tcop/utility.c

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1464,6 +1464,7 @@ ProcessUtilitySlow(ParseState *pstate,
14641464
IndexStmt*stmt= (IndexStmt*)parsetree;
14651465
Oidrelid;
14661466
LOCKMODElockmode;
1467+
intnparts=-1;
14671468
boolis_alter_table;
14681469

14691470
if (stmt->concurrent)
@@ -1494,7 +1495,9 @@ ProcessUtilitySlow(ParseState *pstate,
14941495
*
14951496
* We also take the opportunity to verify that all
14961497
* partitions are something we can put an index on, to
1497-
* avoid building some indexes only to fail later.
1498+
* avoid building some indexes only to fail later. While
1499+
* at it, also count the partitions, so that DefineIndex
1500+
* needn't do a duplicative find_all_inheritors search.
14981501
*/
14991502
if (stmt->relation->inh&&
15001503
get_rel_relkind(relid)==RELKIND_PARTITIONED_TABLE)
@@ -1505,7 +1508,8 @@ ProcessUtilitySlow(ParseState *pstate,
15051508
inheritors=find_all_inheritors(relid,lockmode,NULL);
15061509
foreach(lc,inheritors)
15071510
{
1508-
charrelkind=get_rel_relkind(lfirst_oid(lc));
1511+
Oidpartrelid=lfirst_oid(lc);
1512+
charrelkind=get_rel_relkind(partrelid);
15091513

15101514
if (relkind!=RELKIND_RELATION&&
15111515
relkind!=RELKIND_MATVIEW&&
@@ -1523,6 +1527,8 @@ ProcessUtilitySlow(ParseState *pstate,
15231527
errdetail("Table \"%s\" contains partitions that are foreign tables.",
15241528
stmt->relation->relname)));
15251529
}
1530+
/* count direct and indirect children, but not rel */
1531+
nparts=list_length(inheritors)-1;
15261532
list_free(inheritors);
15271533
}
15281534

@@ -1548,6 +1554,7 @@ ProcessUtilitySlow(ParseState *pstate,
15481554
InvalidOid,/* no predefined OID */
15491555
InvalidOid,/* no parent index */
15501556
InvalidOid,/* no parent constraint */
1557+
nparts,/* # of partitions, or -1 */
15511558
is_alter_table,
15521559
true,/* check_rights */
15531560
true,/* check_not_in_use */

‎src/backend/utils/activity/backend_progress.c

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,27 @@ pgstat_progress_update_param(int index, int64 val)
5858
PGSTAT_END_WRITE_ACTIVITY(beentry);
5959
}
6060

61+
/*-----------
62+
* pgstat_progress_incr_param() -
63+
*
64+
* Increment index'th member in st_progress_param[] of own backend entry.
65+
*-----------
66+
*/
67+
void
68+
pgstat_progress_incr_param(intindex,int64incr)
69+
{
70+
volatilePgBackendStatus*beentry=MyBEEntry;
71+
72+
Assert(index >=0&&index<PGSTAT_NUM_PROGRESS_PARAM);
73+
74+
if (!beentry|| !pgstat_track_activities)
75+
return;
76+
77+
PGSTAT_BEGIN_WRITE_ACTIVITY(beentry);
78+
beentry->st_progress_param[index]+=incr;
79+
PGSTAT_END_WRITE_ACTIVITY(beentry);
80+
}
81+
6182
/*-----------
6283
* pgstat_progress_update_multi_param() -
6384
*

‎src/include/commands/defrem.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ extern ObjectAddress DefineIndex(Oid relationId,
2929
OidindexRelationId,
3030
OidparentIndexId,
3131
OidparentConstraintId,
32+
inttotal_parts,
3233
boolis_alter_table,
3334
boolcheck_rights,
3435
boolcheck_not_in_use,

‎src/include/utils/backend_progress.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ typedef enum ProgressCommandType
3636
externvoidpgstat_progress_start_command(ProgressCommandTypecmdtype,
3737
Oidrelid);
3838
externvoidpgstat_progress_update_param(intindex,int64val);
39+
externvoidpgstat_progress_incr_param(intindex,int64incr);
3940
externvoidpgstat_progress_update_multi_param(intnparam,constint*index,
4041
constint64*val);
4142
externvoidpgstat_progress_end_command(void);

0 commit comments

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