Re: [HACKERS] Block level parallel vacuum

On Fri, Oct 4, 2019 at 10:28 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
wrote:

> On Thu, Oct 3, 2019 at 9:06 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> >
> > On Wed, Oct 2, 2019 at 7:29 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)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?

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com