On 07/10/11 20:50, Dmitry Kurochkin wrote:
Hi Amos.
On Fri, 07 Oct 2011 20:06:19 +1300, Amos Jeffries<[email protected]> wrote:
On 07/10/11 18:53, Dmitry Kurochkin wrote:
Unlinkd is used only by UFS storage. Bug before the change, Squid
always started unlinkd if it is built, even if it is not needed.
Moreover, unlinkd was started in non-daemon mode.
The patch adds unlinks() method for SwapDir to determine if it uses
unlinkd. Unlinkd is started iff:
* Squid is running in daemon mode
* a configured cache_dir uses unlinkd
After the change, unlinkdClose() may be called when unlinkd was never
started. The patch removes a warning which was printed in this case
on Windows.
---
src/SwapDir.h | 2 ++
src/fs/ufs/store_dir_ufs.cc | 6 ++++++
src/fs/ufs/ufscommon.h | 1 +
src/main.cc | 3 ++-
src/protos.h | 1 +
src/unlinkd.cc | 21 +++++++++++++++++++--
6 files changed, 31 insertions(+), 3 deletions(-)
There are a few important details overlooked in this patch.
The use of unlinkd is a property of cache_dir DiskIO strategy, not the
SwapDir type itself.
I wrongly assumed that all DiskIO strategies use unlinkd. Now I see
that is not correct (e.g. DiskThreads does not use unlinkd).
But making decision whether to run unlinkd solely based on the strategy,
is not correct. For example, Rock store may use IpcIo or Blocking
DiskIO strategy, and both may use unlinkd. But Rock does not use
unlinkFile() and does use unlinkd.
I think that unlinkd usage is a property of both DiskIO strategy and
SwapDir type.
Yes. It is up to the SwapDir to pass out the Strategies answer if it
matters (or not).
Definitely not the use of -N option on startup.
The disabling of unlinkd on -N will cause problems on all systems
needing ufs or diskd in that mode. Meaning all BSD default configs, and
those OS using upstart (Mac, Debian, and derivatives) which utilizes -N
mode to get direct control of a single worker process.
I did not know that -N was used for anything but testing and debugging.
Perhaps we should also use diskers in -N mode?
I think so. It will be on production machines, so the speed gain there
would be worth it.
(Though, neither Mac OS X nor Debian (by default) use upstart. Ubuntu
does.)
MacOSX uses some other weird thing launchd IIRC. It's identical to
upstart in behaviour and requirements. Debian is migrating towards
upstart with admin popularity.
But, I think the better way to cover that is to move the unlinks() T/F
decision down to the particular DiskIO strategy and have SwapDir check
relay the answer from there.
We should add mayUseUnlinkd() method to DiskIOStrategy. But only UFS
SwapDir would relay answer from it. Other store modules do not need
unlinkd even if the underlying IO strategy may use it. Do you agree?
I agree. But please add a comment in the rock test function to state why
its not calling its Strategy for details.
Something like: "rock storage does not have files to erase/unlink"
Amos
--
Please be using
Current Stable Squid 2.7.STABLE9 or 3.1.15
Beta testers wanted for 3.2.0.12
