From 83235432e7fd789261cad7729bf3febfc168cd6f Mon Sep 17 00:00:00 2001 From: =?utf8?q?Hugo=20Ar=C3=A8s?= Date: Fri, 4 Sep 2015 15:32:28 -0400 Subject: [PATCH] Fix repository cache never closing repository MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 --- .../eclipse/jgit/lib/RepositoryCacheTest.java | 21 +++++++++++++++++++ .../src/org/eclipse/jgit/lib/Repository.java | 3 ++- .../org/eclipse/jgit/lib/RepositoryCache.java | 6 ++++-- 3 files changed, 27 insertions(+), 3 deletions(-) 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 6c6292558e..8f30fd0821 100644 --- 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 f8266133a6..5546b790e8 100644 --- 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 23cc264c1c..58771857b6 100644 --- 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(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 newRef = new SoftReference(db); Reference oldRef = cacheMap.put(location, newRef); Repository oldDb = oldRef != null ? oldRef.get() : null; -- 2.39.5