Re: [PATCH] Bug 3150: do not start useless unlinkd.

On Fri, 07 Oct 2011 15:54:34 -0600, Alex Rousskov <rousskov_at_measurement-factory.com> wrote:
> On 10/07/2011 03:20 PM, Dmitry Kurochkin wrote:
> > +bool
> > +CossSwapDir::unlinkdUseful() const
> > +{
> > + // UFS storage does not have files to erase/unlink
> > + return false;
> > +}
> > +
>
> This is about COSS, not UFS.

oops

> And since you would be changing this, I
> would also change the message to something more precise like
>
> // no entry-specific files to remove/unlink
>
> which can be used for Rock and COSS.
>

done

>
> > if (!configured_once) {
> > #if USE_UNLINKD
> > - unlinkdInit();
> > + if (unlinkdNeeded())
> > + unlinkdInit();
> > #endif
>
> Can the DiskIO strategy change during reconfigure? We may not support
> that today, but it feels wrong to exclude such possibility. If you
> agree, it would be best to move unlinkdInit() outside !configured_once
> protection and initialize it right after unlinkdNeeded() changed from
> false to true. No need to shut down unlinkd processes, I guess.
>

Good point! Though, moving outside of !configured_once is not enough,
since mainInitialize() runs only on startup. We need to call
unlinkdInit() in mainReconfigureFinish().

> During earlier reviews, I said that we should not start unlinkd on
> reconfigure because Squid may fork a large process. I was wrong -- we do
> not use fork() to start unlinkd. Your earlier code would have probably
> handled this better. Sorry.
>

We do fork() to start unlinkd, see ipcCreate(). IIRC we were discussing
starting unlinkd on the first unlink call, not during reconfigure.

Regards,
  Dmitry

>
> Thank you,
>
> Alex.