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

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. 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.

> 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.

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.

Thank you,

Alex.
Received on Fri Oct 07 2011 - 21:54:48 MDT