date:20111007

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

2011-10-07 Thread Amos Jeffries


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


If unlindNeeded() intention was to see that unlinkd was always enabled 
for mmapped and IPC DiskIO methods unless daemon mode was used, then it 
should be inverted to identify the true case and placed after the for loop.


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.


Amos
--
Please be using
  Current Stable Squid 2.7.STABLE9 or 3.1.15
  Beta testers wanted for 3.2.0.12

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

2011-10-07 Thread Dmitry Kurochkin

Hi Amos.

On Fri, 07 Oct 2011 20:06:19 +1300, Amos Jeffries squ...@treenet.co.nz 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.

 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?

(Though, neither Mac OS X nor Debian (by default) use upstart.  Ubuntu
does.)

 If unlindNeeded() intention was to see that unlinkd was always enabled 
 for mmapped and IPC DiskIO methods unless daemon mode was used, then it 
 should be inverted to identify the true case and placed after the for loop.
 

I did not understand this, but I hope my answer clarifies.

 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?

Regards,
  Dmitry

 Amos
 -- 
 Please be using
Current Stable Squid 2.7.STABLE9 or 3.1.15
Beta testers wanted for 3.2.0.12

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

2011-10-07 Thread Amos Jeffries


On 07/10/11 20:50, Dmitry Kurochkin wrote:

Hi Amos.

On Fri, 07 Oct 2011 20:06:19 +1300, Amos Jeffriessqu...@treenet.co.nz  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

[PATCH] Session helper: upgrade DB and fix active mode

2011-10-07 Thread Andrew Beverley

Further to discussions, please find attached a patch for the session
helper to:

- Remove support for Berkeley DB 1.85
- Add support for the current Berkeley DB (db.h)
- Add support for a DB environment (fixes synchronisation between
multiple processes)
- Fix the active mode bug previously submitted (but not accepted)

I admit that I am rushing to submit this as I go away for the weekend,
so please let me know if it's not up to scratch! Works For Me (TM)
though.

Thanks,

Andy


This patch makes the following changes to the session helper:

- Removes support for Berkeley DB 1.85
- Adds support for the current Berkeley DB (db.h)
- Adds support for a DB environment (if a directory is specified as
  the path then an environment is created). This gives better
  synchronisation to multiple processes
- Fixes a bug with active mode where LOGIN/LOGOUT did not write to the DB

=== modified file 'helpers/external_acl/session/ext_session_acl.8'
--- helpers/external_acl/session/ext_session_acl.8	2011-09-21 00:19:19 +
+++ helpers/external_acl/session/ext_session_acl.8	2011-10-07 18:10:31 +
@@ -52,9 +52,15 @@
 .if !'po4a'hide' .TP
 .if !'po4a'hide' .B \-b path
 .B Path
-to persistent database. If not specified the session details
-will be kept in memory only and all sessions will reset each time
-Squid restarts its helpers (Squid restart or rotation of logs).
+to persistent database. If a file is specified then that single file is
+used as the database. If a path is specified, a Berkeley DB database
+environment is created within the directory. The advantage of the latter
+is better database support between multiple instances of the session
+helper. Using multiple instances of the session helper with a single
+database file will cause synchronisation problems between processes.
+If this option is not specified the session details will be kept in
+memory only and all sessions will reset each time Squid restarts its
+helpers (Squid restart or rotation of logs).
 .
 .if !'po4a'hide' .TP
 .if !'po4a'hide' .B \-a

=== modified file 'helpers/external_acl/session/ext_session_acl.cc'
--- helpers/external_acl/session/ext_session_acl.cc	2011-09-21 00:19:19 +
+++ helpers/external_acl/session/ext_session_acl.cc	2011-10-07 18:10:27 +
@@ -23,19 +23,22 @@
 #endif
 #include helpers/defines.h
 
-#include sys/types.h
-#include sys/stat.h
+#if HAVE_DB_H
+#include db.h
+#endif
 #include fcntl.h
+#if HAVE_GETOPT_H
+#include getopt.h
+#endif
 #include stdio.h
 #include stdlib.h
+#include string.h
+#include sys/types.h
+#include sys/stat.h
+#include time.h
 #if HAVE_UNISTD_H
 #include unistd.h
 #endif
-#include string.h
-#include time.h
-#if HAVE_GETOPT_H
-#include getopt.h
-#endif
 
 /* At this point all Bit Types are already defined, so we must
protect from multiple type definition on platform where
@@ -45,48 +48,79 @@
 #define__BIT_TYPES_DEFINED__
 #endif
 
-#if HAVE_DB_185_H
-#include db_185.h
-#elif HAVE_DB_H
-#include db.h
-#endif
-
 static int session_ttl = 3600;
 static int fixed_timeout = 0;
 char *db_path = NULL;
 const char *program_name;
 
 DB *db = NULL;
+DB_ENV *db_env = NULL;
 
 static void init_db(void)
 {
-db = dbopen(db_path, O_CREAT | O_RDWR, 0666, DB_BTREE, NULL);
-if (!db) {
-fprintf(stderr, FATAL: %s: Failed to open session db '%s'\n, program_name, db_path);
-exit(1);
+struct stat st_buf;
+
+if (db_path) {
+if (!stat(db_path, st_buf)) {
+if (S_ISDIR (st_buf.st_mode)) {
+/* If directory then open database environment. This prevents sync problems
+between different processes. Otherwise fallback to single file */
+db_env_create(db_env, 0);
+if (db_env-open(db_env, db_path, DB_CREATE | DB_INIT_MPOOL | DB_INIT_LOCK , 0666)) {
+fprintf(stderr, FATAL: %s: Failed to open database environment in '%s'\n, program_name, db_path);
+db_env-close(db_env, 0);
+exit(1);
+}
+db_create(db, db_env, 0);
+}
+}
+}
+
+if (db_env) {
+if (db-open(db, NULL, program_name, NULL, DB_BTREE, DB_CREATE, 0666)) {
+fprintf(stderr, FATAL: %s: Failed to open db file '%s' in dir '%s'\n,
+program_name, program_name, db_path);
+db_env-close(db_env, 0);
+exit(1);
+}
+} else {
+db_create(db, NULL, 0);
+if (db-open(db, NULL, db_path, NULL, DB_BTREE, DB_CREATE, 0666)) {
+fprintf(stderr, FATAL: %s: Failed to open session db '%s'\n, program_name, db_path);
+exit(1);
+}
 }
 }
 
 static void shutdown_db(void)
 {
-db-close(db);
+db-close(db, 0);
+if (db_env) {
+db_env-close(db_env, 0);
+}
 }
 
 int session_is_active = 0;
 
 static int session_active(const char *details, size_t len)
 {
-DBT key, data;
-key.data = (void *)details;
-key.size =

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

2011-10-07 Thread Alex Rousskov

On 10/07/2011 03:56 AM, Amos Jeffries wrote:
 On 07/10/11 20:50, Dmitry Kurochkin wrote:
 On Fri, 07 Oct 2011 20:06:19 +1300, Amos Jeffries 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).


Agreed. Let's assume that frequent unlinking is the only valid reason to
use unlinkd and that frequent unlinking can only be caused by store
entry removal. All SwapDirs must implement the unlink(entry) method. A
SwapDir implementation has two correct choices:

1) Handle entry removal without removing any files (Rock, COSS).
2) Remove some file using the configured DiskIO module (ufs).

For SwapDirs using option #1, the implementation of unlinks() method is
clear: It should return false unconditionally.

For SwapDirs using option #2, the answer depends on the DiskIO module in
use. SwapDir code should ask the module if it can benefit from a running
unlinkd and relay that answer to the caller.


 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.

-N is used for many things (unfortunately). Let's focus on unlinkd for
now. We can revisit SMP and -N relationship separately. Since -N is
often used as the preferred startup method in many production
environments that use unlinkd, we should not disable unlinkd just
because -N is on.


 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

Same for COSS.

I would rename things a little so that the flow is consistent with the
above corrections. To put it all together:

::unlinkdNeeded() calls sd-unlinkdUseful() which may call
io-unlinkdUseful().


Thank you,

Alex.

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

2011-10-07 Thread Dmitry Kurochkin


Unlinkd may be used only by UFS storage.  But before the change, Squid
always started unlinkd if it is built, even if it is not needed.

Whether a SwapDir may use unlinkd depends on the SwapDir
implementation and DiskIO strategy it uses.  The patch adds
unlinkdUseful() method to SwapDir and DiskIOStrategy to decide if
unlinkd should be started.

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/DiskIO/AIO/AIODiskIOStrategy.cc |6 ++
 src/DiskIO/AIO/AIODiskIOStrategy.h  |2 ++
 src/DiskIO/Blocking/BlockingIOStrategy.cc   |6 ++
 src/DiskIO/Blocking/BlockingIOStrategy.h|1 +
 src/DiskIO/DiskDaemon/DiskdIOStrategy.cc|6 ++
 src/DiskIO/DiskDaemon/DiskdIOStrategy.h |1 +
 src/DiskIO/DiskIOStrategy.h |5 +
 src/DiskIO/DiskThreads/DiskThreadsIOStrategy.cc |6 ++
 src/DiskIO/DiskThreads/DiskThreadsIOStrategy.h  |1 +
 src/DiskIO/IpcIo/IpcIoIOStrategy.cc |6 ++
 src/DiskIO/IpcIo/IpcIoIOStrategy.h  |1 +
 src/DiskIO/Mmapped/MmappedIOStrategy.cc |6 ++
 src/DiskIO/Mmapped/MmappedIOStrategy.h  |1 +
 src/SwapDir.h   |2 ++
 src/fs/coss/CossSwapDir.h   |1 +
 src/fs/coss/store_io_coss.cc|7 +++
 src/fs/rock/RockSwapDir.cc  |7 +++
 src/fs/rock/RockSwapDir.h   |1 +
 src/fs/ufs/store_dir_ufs.cc |7 +++
 src/fs/ufs/ufscommon.h  |1 +
 src/main.cc |3 ++-
 src/protos.h|1 +
 src/tests/TestSwapDir.cc|6 ++
 src/tests/TestSwapDir.h |1 +
 src/unlinkd.cc  |   17 +++--
 25 files changed, 99 insertions(+), 3 deletions(-)

diff --git src/DiskIO/AIO/AIODiskIOStrategy.cc src/DiskIO/AIO/AIODiskIOStrategy.cc
index 96e11f4..7a94637 100644
--- src/DiskIO/AIO/AIODiskIOStrategy.cc
+++ src/DiskIO/AIO/AIODiskIOStrategy.cc
@@ -83,40 +83,46 @@ AIODiskIOStrategy::newFile (char const *path)
 }
 
 return new AIODiskFile (path, this);
 }
 
 void
 AIODiskIOStrategy::sync()
 {
 assert(aq.aq_state == AQ_STATE_SETUP);
 
 /*
  * Keep calling callback to complete ops until the queue is empty
  * We can't quit when callback returns 0 - some calls may not
  * return any completed pending events, but they're still pending!
  */
 
 while (aq.aq_numpending)
 callback();
 }
 
+bool
+AIODiskIOStrategy::unlinkdUseful() const
+{
+return false;
+}
+
 void
 AIODiskIOStrategy::unlinkFile (char const *)
 {}
 
 /*
  * Note: we grab the state and free the state before calling the callback
  * because this allows us to cut down the amount of time it'll take
  * to find a free slot (since if we call the callback first, we're going
  * to probably be allocated the slot _after_ this one..)
  *
  * I'll make it much more optimal later.
  */
 int
 AIODiskIOStrategy::callback()
 {
 return 0;
 int i;
 int completed = 0;
 int retval, reterr;
 FREE *freefunc;
diff --git src/DiskIO/AIO/AIODiskIOStrategy.h src/DiskIO/AIO/AIODiskIOStrategy.h
index 9d461d0..c3c7d34 100644
--- src/DiskIO/AIO/AIODiskIOStrategy.h
+++ src/DiskIO/AIO/AIODiskIOStrategy.h
@@ -35,40 +35,42 @@
 
 #if USE_DISKIO_AIO
 
 #include DiskIO/DiskIOStrategy.h
 #include async_io.h
 
 class AIODiskIOStrategy : public DiskIOStrategy
 {
 
 public:
 AIODiskIOStrategy();
 virtual ~AIODiskIOStrategy();
 
 virtual bool shedLoad();
 /* What is the current load? 999 = 99.9% */
 virtual int load();
 /* Return a handle for performing IO operations */
 virtual RefCountDiskFile newFile (char const *path);
 /* flush all IO operations  */
 virtual void sync();
+/** whether the IO Strategy can use unlinkd */
+virtual bool unlinkdUseful() const;
 /* unlink a file by path */
 virtual void unlinkFile (char const *);
 
 /* perform any pending callbacks */
 virtual int callback();
 
 /* Init per-instance logic */
 virtual void init();
 
 /* cachemgr output on the IO instance stats */
 virtual void statfs(StoreEntry  sentry)const;
 /* module specific options */
 virtual ConfigOption *getOptionTree() const;
 /* a file descriptor */
 int fd;
 /* queue of requests */
 async_queue_t aq;
 
 int findSlot();
 };
diff --git src/DiskIO/Blocking/BlockingIOStrategy.cc src/DiskIO/Blocking/BlockingIOStrategy.cc
index 9a61973..8610037 100644
--- src/DiskIO/Blocking/BlockingIOStrategy.cc
+++ src/DiskIO/Blocking/BlockingIOStrategy.cc
@@ -40,29 +40,35 @@
 
 bool
 BlockingIOStrategy::shedLoad()
 {
 return false;
 }
 
 int

Jenkins build is back to normal : website-builds #727

2011-10-07 Thread noc

See http://build.squid-cache.org/job/website-builds/727/

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

2011-10-07 Thread Dmitry Kurochkin

On Fri, 07 Oct 2011 15:54:34 -0600, Alex Rousskov 
rouss...@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.
Bug 3150: do not start useless unlinkd.

Unlinkd may be used only by UFS storage.  But before the change, Squid
always started unlinkd if it is built, even if it is not needed.

Whether a SwapDir may use unlinkd depends on the SwapDir
implementation and DiskIO strategy it uses.  The patch adds
unlinkdUseful() method to SwapDir and DiskIOStrategy to decide if
unlinkd should be started.

After the change, unlinkd may be started during reconfiguration and
unlinkdInit() may be called multiple times.

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.

diff --git src/DiskIO/AIO/AIODiskIOStrategy.cc src/DiskIO/AIO/AIODiskIOStrategy.cc
index 96e11f4..7a94637 100644
--- src/DiskIO/AIO/AIODiskIOStrategy.cc
+++ src/DiskIO/AIO/AIODiskIOStrategy.cc
@@ -83,40 +83,46 @@ AIODiskIOStrategy::newFile (char const *path)
 }
 
 return new AIODiskFile (path, this);
 }
 
 void
 AIODiskIOStrategy::sync()
 {
 assert(aq.aq_state == AQ_STATE_SETUP);
 
 /*
  * Keep calling callback to complete ops until the queue is empty
  * We can't quit when callback returns 0 - some calls may not
  * return any completed pending events, but they're still pending!
  */
 
 while (aq.aq_numpending)
 callback();
 }
 
+bool
+AIODiskIOStrategy::unlinkdUseful() const
+{
+return false;
+}
+
 void
 AIODiskIOStrategy::unlinkFile (char const *)
 {}
 
 /*
  * Note: we grab the state and free the state before calling the callback
  * because this allows us to cut down the amount of time it'll take
  * to find a free slot (since if we call the callback first, we're going
  * to probably be allocated the slot _after_ this one..)
  *
  * I'll make it much more optimal later.
  */
 int
 AIODiskIOStrategy::callback()
 {
 return 0;
 int i;
 int completed = 0;
 int retval, reterr;
 FREE *freefunc;
diff --git src/DiskIO/AIO/AIODiskIOStrategy.h src/DiskIO/AIO/AIODiskIOStrategy.h
index 9d461d0..c3c7d34 100644
--- src/DiskIO/AIO/AIODiskIOStrategy.h
+++ src/DiskIO/AIO/AIODiskIOStrategy.h
@@ -35,40 +35,42 @@
 
 #if USE_DISKIO_AIO
 
 #include DiskIO/DiskIOStrategy.h
 #include async_io.h
 
 class AIODiskIOStrategy : public DiskIOStrategy
 {
 
 public:
 AIODiskIOStrategy();
 virtual ~AIODiskIOStrategy();
 
 virtual bool shedLoad();
 /* What is the current load? 999 = 99.9% */
 virtual int load();
 /* Return a handle for performing IO operations */
 virtual RefCountDiskFile newFile (char const *path);
 /* flush all IO operations  */
 virtual void sync();
+/** whether the IO Strategy can use unlinkd */
+virtual bool unlinkdUseful() const;
 /* unlink a file by path */
 virtual void unlinkFile (char const *);
 
 /* perform any pending callbacks */
 virtual int callback();
 
 /* Init per-instance logic */
 virtual void init();
 
 /* cachemgr output on the IO instance stats */
 virtual void statfs(StoreEntry  sentry)const;
 /* module specific options */
 virtual ConfigOption *getOptionTree() const;
 /* a file descriptor */
 int fd;
 /* queue of requests */
 async_queue_t aq;
 
 int findSlot();
 };
diff --git src/DiskIO/Blocking/BlockingIOStrategy.cc src/DiskIO/Blocking/BlockingIOStrategy.cc
index 9a61973..8610037 100644

Re: [PATCH] Session helper: upgrade DB and fix active mode

2011-10-07 Thread Amos Jeffries


On 08/10/11 07:18, Andrew Beverley wrote:

Further to discussions, please find attached a patch for the session
helper to:

- Remove support for Berkeley DB 1.85
- Add support for the current Berkeley DB (db.h)
- Add support for a DB environment (fixes synchronisation between
multiple processes)
- Fix the active mode bug previously submitted (but not accepted)

I admit that I am rushing to submit this as I go away for the weekend,
so please let me know if it's not up to scratch! Works For Me (TM)
though.

Thanks,

Andy




Looks okay. Have a good weekend.

Amos
--
Please be using
  Current Stable Squid 2.7.STABLE9 or 3.1.15
  Beta testers wanted for 3.2.0.12

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

2011-10-07 Thread Amos Jeffries


On 08/10/11 12:53, Dmitry Kurochkin wrote:

On Fri, 07 Oct 2011 15:54:34 -0600, Alex 
Rousskovrouss...@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


Yes it can. When people switch between ufs/aufs/diskd.

Lets try to avoid loosing that ability.


Amos
--
Please be using
  Current Stable Squid 2.7.STABLE9 or 3.1.15
  Beta testers wanted for 3.2.0.12

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

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

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

[PATCH] Session helper: upgrade DB and fix active mode

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

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

Jenkins build is back to normal : website-builds #727

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

Re: [PATCH] Session helper: upgrade DB and fix active mode

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

10 matches

Site Navigation

Mail list logo

Footer information