public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v13 02/18] f! progress reporting
3+ messages / 3 participants
[nested] [flat]
* [PATCH v13 02/18] f! progress reporting
@ 2021-02-15 00:31 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 3+ messages in thread
From: Justin Pryzby @ 2021-02-15 00:31 UTC (permalink / raw)
---
src/backend/commands/indexcmds.c | 33 +++++++-------------------------
1 file changed, 7 insertions(+), 26 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 4ac1dacd7d..d6567ec231 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -1638,40 +1638,20 @@ reindex_invalid_child_indexes(Oid indexRelationId)
.options = REINDEXOPT_CONCURRENTLY
};
- 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)
+ foreach (lc, find_inheritance_children(indexRelationId, ShareLock))
{
Oid partoid = lfirst_oid(lc);
- /* XXX: need to retrofit progress reporting into it */
- // pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_DONE,
- // npart++);
-
- if (get_index_isvalid(partoid) ||
- !RELKIND_HAS_STORAGE(get_rel_relkind(partoid)))
- continue;
+ if (!get_index_isvalid(partoid) &&
+ RELKIND_HAS_STORAGE(get_rel_relkind(partoid)))
+ ReindexRelationConcurrently(partoid, ¶ms);
- /* Save partition OID */
- oldcontext = MemoryContextSwitchTo(ind_context);
- partitions = lappend_oid(partitions, partoid);
- MemoryContextSwitchTo(oldcontext);
+ pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_DONE,
+ npart++);
}
- /*
- * Process each partition listed in a separate transaction. Note that
- * this commits and then starts a new transaction immediately.
- * XXX: since this is done in 2*N transactions, it could just as well
- * call ReindexRelationConcurrently directly
- */
- ReindexMultipleInternal(partitions, ¶ms);
-
/*
* CIC needs to mark a partitioned index as VALID, which itself
* requires setting READY, which is unset for CIC (even though
@@ -1680,6 +1660,7 @@ reindex_invalid_child_indexes(Oid indexRelationId)
* partitions.
* See also: validatePartitionedIndex().
*/
+ CommandCounterIncrement();
index_set_state_flags(indexRelationId, INDEX_CREATE_SET_READY);
CommandCounterIncrement();
index_set_state_flags(indexRelationId, INDEX_CREATE_SET_VALID);
--
2.17.0
--QTprm0S8XgL7H0Dt
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v13-0003-WIP-Add-SKIPVALID-flag-for-more-integration.patch"
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Support escape sequence for cluster_name in postgres_fdw.application_name
@ 2022-01-27 10:26 Fujii Masao <[email protected]>
2022-01-28 05:07 ` RE: Support escape sequence for cluster_name in postgres_fdw.application_name [email protected] <[email protected]>
0 siblings, 1 reply; 3+ messages in thread
From: Fujii Masao @ 2022-01-27 10:26 UTC (permalink / raw)
To: Kyotaro Horiguchi <[email protected]>; +Cc: pgsql-hackers
On 2022/01/27 17:10, Kyotaro Horiguchi wrote:
> At Tue, 25 Jan 2022 16:02:39 +0900, Fujii Masao <[email protected]> wrote in
>> Hi,
>>
>> Commit 6e0cb3dec1 allowed postgres_fdw.application_name to include
>> escape sequences %a (application name), %d (database name), %u (user
>> name) and %p (pid). In addition to them, I'd like to support the
>> escape sequence (e.g., %C) for cluster name there. This escape
>> sequence is helpful to investigate where each remote transactions came
>> from. Thought?
>>
>> Patch attached.
>
> I don't object to adding more meaningful replacements, but more escape
> sequence makes me anxious about the increased easiness of exceeding
> the size limit of application_name.
If this is really an issue, it might be time to reconsider the size limit of application_name. If it's considered too short, the patch that enlarges it should be proposed separately.
> Considering that it is used to
> identify fdw-initinator server, we might need to add padding (or
> rather truncating) option in the escape sequence syntax, then warn
> about truncated application_names for safety.
I failed to understand this. Could you tell me why we might need to add padding option here?
> Is the reason for 'C' in upper-case to avoid possible conflict with
> 'c' of log_line_prefix?
Yes.
> I'm not sure that preventive measure is worth
> doing. Looking the escape-sequence spec alone, it seems to me rather
> strange that an upper-case letter is used in spite of its lower-case
> is not used yet.
I have no strong opinion about using %C. If there is better character for the escape sequence, I'm happy to use it. So what character is more proper? %c?
> Otherwise all looks fine to me except the lack of documentation.
The patch updated postgres-fdw.sgml, but you imply there are other documents that the patch should update? Could you tell me where the patch should update?
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
^ permalink raw reply [nested|flat] 3+ messages in thread
* RE: Support escape sequence for cluster_name in postgres_fdw.application_name
2022-01-27 10:26 Re: Support escape sequence for cluster_name in postgres_fdw.application_name Fujii Masao <[email protected]>
@ 2022-01-28 05:07 ` [email protected] <[email protected]>
0 siblings, 0 replies; 3+ messages in thread
From: [email protected] @ 2022-01-28 05:07 UTC (permalink / raw)
To: 'Fujii Masao' <[email protected]>; Kyotaro Horiguchi <[email protected]>; +Cc: pgsql-hackers
Hi,
Thank you for developing this feature.
I think adding escape sequence for cluster_name is useful too.
> Is the reason for 'C' in upper-case to avoid possible conflict with
> 'c' of log_line_prefix? I'm not sure that preventive measure is worth
> doing. Looking the escape-sequence spec alone, it seems to me rather
> strange that an upper-case letter is used in spite of its lower-case
> is not used yet.
I think %c of log_line_prefix (Session ID) is also useful for postgres_fdw.application_name.
Therefore, how about adding both %c (Session ID) and %C (cluster_name)?
Regards,
Ryohei Takahashi
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2022-01-28 05:07 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-02-15 00:31 [PATCH v13 02/18] f! progress reporting Justin Pryzby <[email protected]>
2022-01-27 10:26 Re: Support escape sequence for cluster_name in postgres_fdw.application_name Fujii Masao <[email protected]>
2022-01-28 05:07 ` RE: Support escape sequence for cluster_name in postgres_fdw.application_name [email protected] <[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