On Fri, Oct 4, 2019 at 2:31 PM Amit Kapila <amit.kapil...@gmail.com> wrote:
>
> On Fri, Oct 4, 2019 at 10:28 AM Masahiko Sawada <sawada.m...@gmail.com> wrote:
>>
>> On Thu, Oct 3, 2019 at 9:06 PM Dilip Kumar <dilipbal...@gmail.com> wrote:
>> >
>> > On Wed, Oct 2, 2019 at 7:29 PM Masahiko Sawada <sawada.m...@gmail.com>
>> > wrote:
>> >
>> > + else
>> > + {
>> > + if (for_cleanup)
>> > + {
>> > + if (lps->nworkers_requested > 0)
>> > + appendStringInfo(&buf,
>> > + ngettext("launched %d parallel vacuum worker for index cleanup
>> > (planned: %d, requested %d)",
>> > + "launched %d parallel vacuum workers for index cleanup (planned:
>> > %d, requsted %d)",
>> > + lps->pcxt->nworkers_launched),
>> > + lps->pcxt->nworkers_launched,
>> > + lps->pcxt->nworkers,
>> > + lps->nworkers_requested);
>> > + else
>> > + appendStringInfo(&buf,
>> > + ngettext("launched %d parallel vacuum worker for index cleanup (planned:
>> > %d)",
>> > + "launched %d parallel vacuum workers for index cleanup (planned: %d)",
>> > + lps->pcxt->nworkers_launched),
>> > + lps->pcxt->nworkers_launched,
>> > + lps->pcxt->nworkers);
>> > + }
>> > + else
>> > + {
>> > + if (lps->nworkers_requested > 0)
>> > + appendStringInfo(&buf,
>> > + ngettext("launched %d parallel vacuum worker for index vacuuming
>> > (planned: %d, requested %d)",
>> > + "launched %d parallel vacuum workers for index vacuuming (planned:
>> > %d, requested %d)",
>> > + lps->pcxt->nworkers_launched),
>> > + lps->pcxt->nworkers_launched,
>> > + lps->pcxt->nworkers,
>> > + lps->nworkers_requested);
>> > + else
>> > + appendStringInfo(&buf,
>> > + ngettext("launched %d parallel vacuum worker for index vacuuming
>> > (planned: %d)",
>> > + "launched %d parallel vacuum workers for index vacuuming (planned:
>> > %d)",
>> > + lps->pcxt->nworkers_launched),
>> > + lps->pcxt->nworkers_launched,
>> > + lps->pcxt->nworkers);
>> > + }
>> >
>> > Multiple places I see a lot of duplicate code for for_cleanup is true
>> > or false. The only difference is in the error message whether we give
>> > index cleanup or index vacuuming otherwise complete code is the same
>> > for
>> > both the cases. Can't we create some string and based on the value of
>> > the for_cleanup and append it in the error message that way we can
>> > avoid duplicating this at many places?
>>
>> I think it's necessary for translation. IIUC if we construct the
>> message it cannot be translated.
>>
>
> Do we really need to log all those messages? The other places where we
> launch parallel workers doesn't seem to be using such messages. Why do you
> think it is important to log the messages here when other cases don't use it?
Well I would rather think that parallel create index doesn't log
enough messages. Parallel maintenance operation is invoked manually by
user. I can imagine that DBA wants to cancel and try the operation
again later if enough workers are not launched. But there is not a
convenient way to confirm how many parallel workers planned and
actually launched. We need to see ps command or pg_stat_activity.
That's why I think that log message would be helpful for users.
Regards,
--
Masahiko Sawada
