Re: store_clean.c patch related to USE_TRUNCATE_NOT_UNLINK

In article <3746A857.1240BA00@hem.passagen.se> Henrik writes:

>Is USE_TRUNCATE_NOT_UNLINK really a good idea? On Linux at least it
>seems like it isn't (no noticeable performance gain, and a number of
>Squid problems related to it's use).

I've included my two messages I sent to squid-bugs last Friday at the
end to give some background information concerning Henrik's remark.

Whether USE_TRUNCATE_NOT_UNLINK is a good idea? I think it can be a good
way of keeping the contents of the directories constant by using a fixed
set of file entries which become truncated when unused.

I've been using Squid setups where the number of unlinks could reach
24/sec. Because expiring is not done on a per-directory basis this could
mean that 24 different directories need to be updated per second and
eventually written back to disk. Assuming a 30 seconds sync() time and
an 8KB blocksize this would mean 5760 KB being flushed to disk at once
in the worst case. By using truncate instead of unlink you can save this
type of directory update related disk I/O.

Whether it is noticeable? I have not done any measurements. But when
thinking about it, it should never make performance worse.

What my patches address are not strictly bugs but what I would call
'annoying behavior'. I'd like to keep the fileset and the directories as
small as possible without enormous amounts of unused and truncated
files. Therefore the 'suggestion' variable is reset if possible when it
has reached the optimal number of files (L1 * L2 * L2).

Arjan

-----------------------------------------------------------------------------

To: squid-bugs@ircache.net
Subject: store_clean.c patch related to USE_TRUNCATE_NOT_UNLINK

Hi,

It looks like storeDirClean never removes or only truncates misplaced
files when USE_TRUNCATE_NOT_UNLINK has been defined (the default). I
think files which are in the wrong directory should be unlinked,
independent of the USE_TRUNCATE_NOT_UNLINK setting, because it
unnecessarily uses inodes.

The patch below fixes this. The check for skipping a zero-sized file is
now only done when storeFilenoBelongsHere is true. Furthermore
safeunlink is always used to clean instead of truncate.

BTW, is there any reason why safeunlink is used and not the unlinkd?

Arjan

Index: store_clean.c
===================================================================
RCS file: /home/dibbs/CVS/dibbs/usrdibbs/http-proxy/dist/src/store_clean.c,v
retrieving revision 1.1.1.1
diff -u -u -r1.1.1.1 store_clean.c
--- store_clean.c 1999/05/05 15:39:35 1.1.1.1
+++ store_clean.c 1999/05/21 13:52:00
@@ -93,14 +93,17 @@
             continue;
         fn = storeDirProperFileno(D0, swapfileno);
         if (storeDirValidFileno(fn))
- if (storeDirMapBitTest(fn))
- if (storeFilenoBelongsHere(fn, D0, D1, D2))
+ if (storeFilenoBelongsHere(fn, D0, D1, D2)) {
+ if (storeDirMapBitTest(fn))
                     continue;
 #if USE_TRUNCATE_NOT_UNLINK
- if (!stat(de->d_name, &sb))
- if (sb.st_size == 0)
- continue;
+ else {
+ if (!stat(de->d_name, &sb))
+ if (sb.st_size == 0)
+ continue;
+ }
 #endif
+ }
         files[k++] = swapfileno;
     }
     closedir(dp);
@@ -113,11 +116,7 @@
     for (n = 0; n < k; n++) {
         debug(36, 3) ("storeDirClean: Cleaning file %08X\n", files[n]);
         snprintf(p2, MAXPATHLEN + 1, "%s/%08X", p1, files[n]);
-#if USE_TRUNCATE_NOT_UNLINK
- truncate(p2, 0);
-#else
         safeunlink(p2, 0);
-#endif
         Counter.swap_files_cleaned++;
     }
     debug(36, 3) ("Cleaned %d unused files from %s\n", k, p1);

-----------------------------------------------------------------------------

To: squid-bugs@ircache.net
Subject: file_map_allocate behavior when using USE_TRUNCATE_NOT_UNLINK

Hi,

I recently upgraded a machine to 2.2.STABLE2 from 1.1.20. It has 12GB
cache swap in one directory with the default 16/256 L1/L2 setup.

I noticed that after a while the cache directories got bigger than 1
disk block (8KB) because they had 512 files in them. What had happened
was that after L1 * L2 * L2 files had been written Squid started writing
new files in the first cache directory (/cache/00/00). It did this even
in the presence of _lots_ of unused truncated files which should have
been reused.

The problem is that the value of the 'suggestion' variable in
file_map_allocate increases until max_n_files has been reached; then it
will be reset to 0. max_n_files has no direct relation to L1 and L2,
therefore this problem can occur.

What the patch below does is adding a wanted_n_files field per filemap
with the value L1 * L2 * L2. As soon as the 'suggestion' variable
becomes greater than this value *and* n_files_in_map < wanted_n_files -
X the suggestion variable is reset to 0 too. This will cause the
existing unused and truncated files to be reused instead of creating new
files which unnecessarily increase the size of the directory blocks.

Arjan

Index: filemap.c
===================================================================
RCS file: /home/dibbs/CVS/dibbs/usrdibbs/http-proxy/dist/src/filemap.c,v
retrieving revision 1.1.1.1
diff -u -u -r1.1.1.1 filemap.c
--- filemap.c 1999/05/05 15:39:34 1.1.1.1
+++ filemap.c 1999/05/21 15:26:44
@@ -54,12 +54,14 @@
 #endif
 
 fileMap *
-file_map_create(int n)
+file_map_create(int n, int wanted)
 {
     fileMap *fm = xcalloc(1, sizeof(fileMap));
     fm->max_n_files = n;
+ fm->wanted_n_files = wanted;
     fm->nwords = n >> LONG_BIT_SHIFT;
     debug(8, 3) ("file_map_create: creating space for %d files\n", n);
+ debug(8, 3) ("file_map_create: trying to use the first %d files\n", wanted);
     debug(8, 5) ("--> %d words of %d bytes each\n",
         fm->nwords, sizeof(unsigned long));
     fm->file_map = xcalloc(fm->nwords, sizeof(unsigned long));
@@ -106,7 +108,10 @@
     int word;
     int bit;
     int count;
+ int wanted = fm->wanted_n_files;
     if (suggestion > fm->max_n_files)
+ suggestion = 0;
+ if ((suggestion > wanted) && (fm->n_files_in_map < wanted - (wanted >> 8)))
         suggestion = 0;
     if (!file_map_bit_test(fm, suggestion)) {
         return file_map_bit_set(fm, suggestion);
Index: store_dir.c
===================================================================
RCS file: /home/dibbs/CVS/dibbs/usrdibbs/http-proxy/dist/src/store_dir.c,v
retrieving revision 1.1.1.1
diff -u -u -r1.1.1.1 store_dir.c
--- store_dir.c 1999/05/05 15:39:35 1.1.1.1
+++ store_dir.c 1999/05/21 12:07:12
@@ -837,18 +837,24 @@
     SwapDir *SD;
     int n;
     int i;
+ int L1, L2, wanted;
     fileMap *fm;
     Config.Swap.maxSize = 0;
     for (i = 0; i < Config.cacheSwap.n_configured; i++) {
         SD = &Config.cacheSwap.swapDirs[i];;
         Config.Swap.maxSize += SD->max_size;
+ L1 = Config.cacheSwap.swapDirs[i].l1;
+ L2 = Config.cacheSwap.swapDirs[i].l2;
         n = 2 * SD->max_size / Config.Store.avgObjectSize;
+ wanted = L1 * L2 * L2;
+ if (wanted > n)
+ wanted = n;
         if (NULL == SD->map) {
             /* first time */
- SD->map = file_map_create(n);
+ SD->map = file_map_create(n, wanted);
         } else if (n > SD->map->max_n_files) {
             /* it grew, need to expand */
- fm = file_map_create(n);
+ fm = file_map_create(n, wanted);
             filemapCopy(SD->map, fm);
             filemapFreeMemory(SD->map);
             SD->map = fm;
Index: protos.h
===================================================================
RCS file: /home/dibbs/CVS/dibbs/usrdibbs/http-proxy/dist/src/protos.h,v
retrieving revision 1.1.1.2
diff -u -u -r1.1.1.2 protos.h
--- protos.h 1999/05/20 07:58:10 1.1.1.2
+++ protos.h 1999/05/21 12:01:37
@@ -233,7 +233,7 @@
 extern int fdNFree(void);
 extern void fdAdjustReserved(void);
 
-extern fileMap *file_map_create(int);
+extern fileMap *file_map_create(int, int);
 extern int file_map_allocate(fileMap *, int);
 extern int file_map_bit_set(fileMap *, int);
 extern int file_map_bit_test(fileMap *, int);
Index: structs.h
===================================================================
RCS file: /home/dibbs/CVS/dibbs/usrdibbs/http-proxy/dist/src/structs.h,v
retrieving revision 1.2
diff -u -u -r1.2 structs.h
--- structs.h 1999/05/05 16:35:45 1.2
+++ structs.h 1999/05/21 12:03:43
@@ -558,6 +558,7 @@
 
 struct _fileMap {
     int max_n_files;
+ int wanted_n_files;
     int n_files_in_map;
     int toggle;
     int nwords;
Received on Tue Jul 29 2003 - 13:15:58 MDT