Fix repository cache never closing repository

Repository has a usage counter that is initialized to 1 at
instantiation and this counter is decremented when Repository.close
method is called. There is also a Repository.incrementOpen method that
RepositoryCache uses to increment the usage count when it's returning a
repository that is already opened.

The problem was that RepositoryCache was incrementing the usage count
for repositories that it just opened or registered. The usage count was
2 when it should have been 1.

Incrementing usage count is now only be done for repository that are
served from the cache.

This bug is causing slow memory increase of our Gerrit server until the
server become slow. Even if the RepositoryCache is using SoftReference,
it seems that the JVM is not garbage collecting the repositories because
it's not yet on the edge of being out of memory.

To test this change, I replicated all repositories(11k) from Gerrit
master to one slave. The Gerrit master used memory after this test was
10GB without this change and 3.5GB with.

Change-Id: I86c7b36174e384f106b51fe92f306018fd1dbdf0
Signed-off-by: Hugo Arès <hugo.ares@ericsson.com>

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositoryCacheTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositoryCacheTest.java
index 6c6292558e59557d7eb218816716331fde793d2a..8f30fd0821e64ff377d2881d51e3a4c9e731dd87 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositoryCacheTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositoryCacheTest.java
@@ -173,4 +173,25 @@ public class RepositoryCacheTest extends RepositoryTestCase {
                assertEquals(0, RepositoryCache.getRegisteredKeys().size());
        }
 
+       @Test
+       public void testRepositoryUsageCount() throws Exception {
+               FileKey loc = FileKey.exact(db.getDirectory(), db.getFS());
+               Repository d2 = RepositoryCache.open(loc);
+               assertEquals(1, d2.useCnt.get());
+               RepositoryCache.open(FileKey.exact(loc.getFile(), db.getFS()));
+               assertEquals(2, d2.useCnt.get());
+               d2.close();
+               assertEquals(1, d2.useCnt.get());
+               d2.close();
+               assertEquals(0, d2.useCnt.get());
+       }
+
+       @Test
+       public void testRepositoryUsageCountWithRegisteredRepository() {
+               assertEquals(1, ((Repository) db).useCnt.get());
+               RepositoryCache.register(db);
+               assertEquals(1, ((Repository) db).useCnt.get());
+               db.close();
+               assertEquals(0, ((Repository) db).useCnt.get());
+       }
 }

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java
index f8266133a632b0c68be70245e4958b1ca0003b8a..5546b790e8cac193ab9c3f81cc2e897b3b121187 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java
@@ -110,7 +110,8 @@ public abstract class Repository implements AutoCloseable {
                return globalListeners;
        }
 
-       private final AtomicInteger useCnt = new AtomicInteger(1);
+       /** Use counter */
+       final AtomicInteger useCnt = new AtomicInteger(1);
 
        /** Metadata directory holding the repository's critical files. */
        private final File gitDir;

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RepositoryCache.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RepositoryCache.java
index 23cc264c1cf9786d74f9e1f35049144ccb464a59..58771857b6e2404fa5a8293552ad8e33a78a6ac5 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RepositoryCache.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RepositoryCache.java
@@ -196,15 +196,17 @@ public class RepositoryCache {
                                        db = location.open(mustExist);
                                        ref = new SoftReference<Repository>(db);
                                        cacheMap.put(location, ref);
+                               } else {
+                                       db.incrementOpen();
                                }
                        }
+               } else {
+                       db.incrementOpen();
                }
-               db.incrementOpen();
                return db;
        }
 
        private void registerRepository(final Key location, final Repository db) {
-               db.incrementOpen();
                SoftReference<Repository> newRef = new SoftReference<Repository>(db);
                Reference<Repository> oldRef = cacheMap.put(location, newRef);
                Repository oldDb = oldRef != null ? oldRef.get() : null;