Tradindexed cache entries

[Russ Allbery]

Julien ÉLIE <julien at trigofacile.com> writes:

> I think I have just found out what is going on.  tdx_cache_delete() does
> not decrement the known number of elements in the hash table, even
> though they are removed from it.  So at some point, we end up having 0
> element in the hash, though the count is non zero.  Therefore, no
> "oldest cache entry" is found.

Oh, good catch!  This bug has been latent for a long time since previously
_reopen was only called rarely by nnrpd, probably never enough to have a
noticable impact from the leak.

> Index: storage/tradindexed/tdx-cache.c
> ===================================================================
> --- storage/tradindexed/tdx-cache.c     (révision 8880)
> +++ storage/tradindexed/tdx-cache.c     (copie de travail)
> @@ -197,9 +197,12 @@
> void
> tdx_cache_delete(struct cache *cache, HASH hash)
> {
> -    if (!hash_delete(cache->hashtable, &hash))
> +    if (!hash_delete(cache->hashtable, &hash)) {
>         warn("tradindexed: unable to remove cache entry for %s",
>              HashToText(hash));
> +    } else {
> +        cache->count--;
> +    }
> }

Looks right to me.

> To reproduce the problem:

> * overcachesize: 4 in inn.conf

> % lsof -c innd | grep overview
> /home/news/spool/overview/group.index
> /home/news/spool/overview/group.index
> /home/news/spool/overview/f/s/z/fr.sci.zetetique.IDX
> /home/news/spool/overview/f/s/z/fr.sci.zetetique.DAT
> /home/news/spool/overview/n/l/f/news.lists.filters.IDX
> /home/news/spool/overview/n/l/f/news.lists.filters.DAT
> /home/news/spool/overview/f/t/fr.test.IDX
> /home/news/spool/overview/f/s/p/fr.sci.philo.IDX
> /home/news/spool/overview/f/s/p/fr.sci.philo.DAT
> /home/news/spool/overview/f/t/fr.test.DAT

> * post a supersedes in fr.test

> % lsof -c innd | grep overview
> /home/news/spool/overview/group.index
> /home/news/spool/overview/group.index
> /home/news/spool/overview/n/l/f/news.lists.filters.IDX
> /home/news/spool/overview/n/l/f/news.lists.filters.DAT
> /home/news/spool/overview/f/t/fr.test.IDX
> /home/news/spool/overview/f/c/o/l/c/fr.comp.os.linux.configuration.IDX
> /home/news/spool/overview/f/c/o/l/c/fr.comp.os.linux.configuration.DAT
> /home/news/spool/overview/f/t/fr.test.DAT

> We've lost one file.

> * post two other supersedes in fr.test

> % lsof -c innd | grep overview
> /home/news/spool/overview/group.index
> /home/news/spool/overview/group.index
> /home/news/spool/overview/f/t/fr.test.IDX
> /home/news/spool/overview/f/t/fr.test.DAT

> And we have "tradindexed: unable to find oldest cache entry" for every
> subsequent article received.

Ideally, it would be great to capture this as an additional test case in
the test suite.

> I have a remaining question:  is it normal that lsof returns deleted entries?
> I for instance had a CAF file (I assume removed during expiry):
>   /home/news/spool/articles/timecaf-02/9a/4b33.CF.20401 (deleted)
> or also .IDX-NEW and .DAT-NEW files...  Are they supposed to remain visible
> in lsof?

Well, it's normal for lsof as long as they're still held open by the
process.  I'm not sure off-hand why innd would be holding open file
descriptors pointing to deleted files, although I suppose it makes sense
with a largish overview cache for overview files deleted during expire.

-- 
Russ Allbery (rra at stanford.edu)             <http://www.eyrie.org/~eagle/>

    Please send questions to the list rather than mailing me directly.
     <http://www.eyrie.org/~eagle/faqs/questions.html> explains why.