public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v11 2/9] Add SKIPVALID flag for more integration 3+ messages / 3 participants [nested] [flat]
* [PATCH v11 2/9] Add SKIPVALID flag for more integration @ 2020-10-30 21:23 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Justin Pryzby @ 2020-10-30 21:23 UTC (permalink / raw) --- src/backend/commands/indexcmds.c | 54 +++++++++++--------------------- src/include/nodes/parsenodes.h | 7 +++-- 2 files changed, 22 insertions(+), 39 deletions(-) diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 76219381c1..ffc166206b 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -1617,53 +1617,30 @@ DefineIndex(Oid relationId, return address; } -/* Reindex invalid child indexes created earlier */ +/* + * Reindex invalid child indexes created earlier thereby validating + * the parent index. + */ static void reindex_invalid_child_indexes(Oid indexRelationId) { - ListCell *lc; - int npart = 0; - - MemoryContext ind_context = AllocSetContextCreate(PortalContext, "CREATE INDEX", - ALLOCSET_DEFAULT_SIZES); - MemoryContext oldcontext; - List *childs = find_inheritance_children(indexRelationId, ShareLock); - List *partitions = NIL; - - PreventInTransactionBlock(true, "REINDEX INDEX"); - - foreach (lc, childs) - { - Oid partoid = lfirst_oid(lc); - - pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_DONE, - npart++); - - if (get_index_isvalid(partoid) || - !RELKIND_HAS_STORAGE(get_rel_relkind(partoid))) - continue; - - /* Save partition OID */ - oldcontext = MemoryContextSwitchTo(ind_context); - partitions = lappend_oid(partitions, partoid); - MemoryContextSwitchTo(oldcontext); - } - - /* - * Process each partition listed in a separate transaction. Note that - * this commits and then starts a new transaction immediately. - */ - ReindexMultipleInternal(partitions, REINDEXOPT_CONCURRENTLY); - /* * CIC needs to mark a partitioned index as VALID, which itself * requires setting READY, which is unset for CIC (even though * it's meaningless for an index without storage). * This must be done only while holding a lock which precludes adding * partitions. - * See also: validatePartitionedIndex(). */ + CommandCounterIncrement(); index_set_state_flags(indexRelationId, INDEX_CREATE_SET_READY); + + /* + * Process each partition listed in a separate transaction. Note that + * this commits and then starts a new transaction immediately. + */ + ReindexPartitions(indexRelationId, + REINDEXOPT_CONCURRENTLY | REINDEXOPT_SKIPVALID, true); + CommandCounterIncrement(); index_set_state_flags(indexRelationId, INDEX_CREATE_SET_VALID); } @@ -2947,6 +2924,11 @@ ReindexPartitions(Oid relid, int options, bool isTopLevel) if (!RELKIND_HAS_STORAGE(partkind)) continue; + /* Skip invalid indexes, if requested */ + if ((options & REINDEXOPT_SKIPVALID) != 0 && + get_index_isvalid(partoid)) + continue; + Assert(partkind == RELKIND_INDEX || partkind == RELKIND_RELATION); diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index d1f9ef29ca..1373b57ae7 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -3347,10 +3347,11 @@ typedef struct ConstraintsSetStmt */ /* Reindex options */ -#define REINDEXOPT_VERBOSE (1 << 0) /* print progress info */ +#define REINDEXOPT_VERBOSE (1 << 0) /* print progress info */ #define REINDEXOPT_REPORT_PROGRESS (1 << 1) /* report pgstat progress */ -#define REINDEXOPT_MISSING_OK (1 << 2) /* skip missing relations */ -#define REINDEXOPT_CONCURRENTLY (1 << 3) /* concurrent mode */ +#define REINDEXOPT_MISSING_OK (1 << 2) /* skip missing relations */ +#define REINDEXOPT_CONCURRENTLY (1 << 3) /* concurrent mode */ +#define REINDEXOPT_SKIPVALID (1 << 4) /* skip valid indexes */ typedef enum ReindexObjectType { -- 2.17.0 --cWoXeonUoKmBZSoM Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v11-0003-ReindexPartitions-to-set-indisvalid.patch" ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Issue in postgres_fdw causing unnecessary wait for cancel request reply @ 2023-04-13 18:19 Fujii Masao <[email protected]> 0 siblings, 1 reply; 3+ messages in thread From: Fujii Masao @ 2023-04-13 18:19 UTC (permalink / raw) To: Etsuro Fujita <[email protected]>; +Cc: [email protected] On 2023/04/13 15:13, Etsuro Fujita wrote: > I am not 100% sure that it is a good idea to use the same error > message "could not send cancel request" for the PQgetCancel() and > PQcancel() cases, because they are different functions. How about > "could not create PGcancel structure” or something like that, for the The primary message basically should avoid reference to implementation details such as specific structure names like PGcancel, shouldn't it, as per the error message style guide? > former case, so we can distinguish the former error from the latter? Although the primary message is the same, the supplemental message provides additional context that can help distinguish which function is reporting the message. Therefore, I'm fine with the current primary message in the 0001 patch. However, I'm open to any better message ideas. Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Issue in postgres_fdw causing unnecessary wait for cancel request reply @ 2023-04-14 09:59 Etsuro Fujita <[email protected]> parent: Fujii Masao <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Etsuro Fujita @ 2023-04-14 09:59 UTC (permalink / raw) To: Fujii Masao <[email protected]>; +Cc: [email protected] On Fri, Apr 14, 2023 at 3:19 AM Fujii Masao <[email protected]> wrote: > On 2023/04/13 15:13, Etsuro Fujita wrote: > > I am not 100% sure that it is a good idea to use the same error > > message "could not send cancel request" for the PQgetCancel() and > > PQcancel() cases, because they are different functions. How about > > "could not create PGcancel structure” or something like that, for the > > The primary message basically should avoid reference to implementation details such as specific structure names like PGcancel, shouldn't it, as per the error message style guide? I do not think that PGcancel is that specific, as it is described in the user-facing documentation [1]. (In addition, the error message I proposed was created by copying the existing error message "could not create OpenSSL BIO structure" in contrib/sslinfo.c.) > > former case, so we can distinguish the former error from the latter? > > Although the primary message is the same, the supplemental message provides additional context that can help distinguish which function is reporting the message. If the user is familiar with the PQgetCancel/PQcancel internals, this is true, but if not, I do not think this is always true. Consider this error message, for example: 2023-04-14 17:48:55.862 JST [24344] WARNING: could not send cancel request: invalid integer value "99999999999" for connection option "keepalives" It would be hard for users without the knowledge about those internals to distinguish that from this message. For average users, I think it would be good to use a more distinguishable error message. Best regards, Etsuro Fujita [1] https://www.postgresql.org/docs/current/libpq-cancel.html ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2023-04-14 09:59 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-10-30 21:23 [PATCH v11 2/9] Add SKIPVALID flag for more integration Justin Pryzby <[email protected]> 2023-04-13 18:19 Re: Issue in postgres_fdw causing unnecessary wait for cancel request reply Fujii Masao <[email protected]> 2023-04-14 09:59 ` Re: Issue in postgres_fdw causing unnecessary wait for cancel request reply Etsuro Fujita <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox