diff options
5 files changed, 419 insertions, 13 deletions
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositoryCacheConfigTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositoryCacheConfigTest.java new file mode 100644 index 0000000000..52cc9fb030 --- /dev/null +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositoryCacheConfigTest.java @@ -0,0 +1,107 @@ +/* + * Copyright (C) 2016 Ericsson + * and other copyright owners as documented in the project's IP log. + * + * This program and the accompanying materials are made available + * under the terms of the Eclipse Distribution License v1.0 which + * accompanies this distribution, is reproduced below, and is + * available at http://www.eclipse.org/org/documents/edl-v10.php + * + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following + * conditions are met: + * + * - Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials provided + * with the distribution. + * + * - Neither the name of the Eclipse Foundation, Inc. nor the + * names of its contributors may be used to endorse or promote + * products derived from this software without specific prior + * written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND + * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, + * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER + * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF + * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package org.eclipse.jgit.lib; + +import static org.eclipse.jgit.lib.RepositoryCacheConfig.AUTO_CLEANUP_DELAY; +import static org.eclipse.jgit.lib.RepositoryCacheConfig.NO_CLEANUP; +import static org.junit.Assert.assertEquals; + +import java.util.concurrent.TimeUnit; + +import org.eclipse.jgit.errors.ConfigInvalidException; +import org.junit.Before; +import org.junit.Test; + +public class RepositoryCacheConfigTest { + + private RepositoryCacheConfig config; + + @Before + public void setUp() { + config = new RepositoryCacheConfig(); + } + + @Test + public void testDefaultValues() { + assertEquals(TimeUnit.HOURS.toMillis(1), config.getExpireAfter()); + assertEquals(config.getExpireAfter() / 10, config.getCleanupDelay()); + } + + @Test + public void testCleanupDelay() { + config.setCleanupDelay(TimeUnit.HOURS.toMillis(1)); + assertEquals(TimeUnit.HOURS.toMillis(1), config.getCleanupDelay()); + } + + @Test + public void testAutoCleanupDelay() { + config.setExpireAfter(TimeUnit.MINUTES.toMillis(20)); + config.setCleanupDelay(AUTO_CLEANUP_DELAY); + assertEquals(TimeUnit.MINUTES.toMillis(20), config.getExpireAfter()); + assertEquals(config.getExpireAfter() / 10, config.getCleanupDelay()); + } + + @Test + public void testAutoCleanupDelayShouldBeMax10minutes() { + config.setExpireAfter(TimeUnit.HOURS.toMillis(10)); + assertEquals(TimeUnit.HOURS.toMillis(10), config.getExpireAfter()); + assertEquals(TimeUnit.MINUTES.toMillis(10), config.getCleanupDelay()); + } + + @Test + public void testDisabledCleanupDelay() { + config.setCleanupDelay(NO_CLEANUP); + assertEquals(NO_CLEANUP, config.getCleanupDelay()); + } + + @Test + public void testFromConfig() throws ConfigInvalidException { + Config otherConfig = new Config(); + otherConfig.fromText("[core]\nrepositoryCacheExpireAfter=1000\n" + + "repositoryCacheCleanupDelay=500"); + config.fromConfig(otherConfig); + assertEquals(1000, config.getExpireAfter()); + assertEquals(500, config.getCleanupDelay()); + } +} 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 a1cec2d914..6bea320120 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 @@ -195,17 +195,81 @@ public class RepositoryCacheTest extends RepositoryTestCase { assertEquals(0, ((Repository) db).useCnt.get()); } - public void testRepositoryUnregisteringWhenClosing() throws Exception { + @Test + public void testRepositoryNotUnregisteringWhenClosing() throws Exception { FileKey loc = FileKey.exact(db.getDirectory(), db.getFS()); Repository d2 = RepositoryCache.open(loc); assertEquals(1, d2.useCnt.get()); assertThat(RepositoryCache.getRegisteredKeys(), hasItem(FileKey.exact(db.getDirectory(), db.getFS()))); assertEquals(1, RepositoryCache.getRegisteredKeys().size()); - d2.close(); - assertEquals(0, d2.useCnt.get()); - assertEquals(0, RepositoryCache.getRegisteredKeys().size()); + assertEquals(1, RepositoryCache.getRegisteredKeys().size()); + assertTrue(RepositoryCache.isCached(d2)); + } + + @Test + public void testRepositoryUnregisteringWhenExpired() throws Exception { + Repository repoA = createBareRepository(); + Repository repoB = createBareRepository(); + Repository repoC = createBareRepository(); + RepositoryCache.register(repoA); + RepositoryCache.register(repoB); + RepositoryCache.register(repoC); + + assertEquals(3, RepositoryCache.getRegisteredKeys().size()); + assertTrue(RepositoryCache.isCached(repoA)); + assertTrue(RepositoryCache.isCached(repoB)); + assertTrue(RepositoryCache.isCached(repoC)); + + // fake that repoA was closed more than 1 hour ago (default expiration + // time) + repoA.close(); + repoA.closedAt.set(System.currentTimeMillis() - 65 * 60 * 1000); + // close repoB but this one will not be expired + repoB.close(); + + assertEquals(3, RepositoryCache.getRegisteredKeys().size()); + assertTrue(RepositoryCache.isCached(repoA)); + assertTrue(RepositoryCache.isCached(repoB)); + assertTrue(RepositoryCache.isCached(repoC)); + + RepositoryCache.clearExpired(); + + assertEquals(2, RepositoryCache.getRegisteredKeys().size()); + assertFalse(RepositoryCache.isCached(repoA)); + assertTrue(RepositoryCache.isCached(repoB)); + assertTrue(RepositoryCache.isCached(repoC)); + } + + @Test + public void testReconfigure() throws InterruptedException { + RepositoryCache.register(db); + assertTrue(RepositoryCache.isCached(db)); + db.close(); + assertTrue(RepositoryCache.isCached(db)); + + // Actually, we would only need to validate that + // WorkQueue.getExecutor().scheduleWithFixedDelay is called with proper + // values but since we do not have a mock library, we test + // reconfiguration from a black box perspective. I.e. reconfigure + // expireAfter and cleanupDelay to 1 ms and wait until the Repository + // is evicted to prove that reconfiguration worked. + RepositoryCacheConfig config = new RepositoryCacheConfig(); + config.setExpireAfter(1); + config.setCleanupDelay(1); + config.install(); + + // Instead of using a fixed waiting time, start with small and increase: + // sleep 1, 2, 4, 8, 16, ..., 1024 ms + // This wait will time out after 2048 ms + for (int i = 0; i <= 10; i++) { + Thread.sleep(1 << i); + if (!RepositoryCache.isCached(db)) { + return; + } + } + fail("Repository should have been evicted from cache"); } } 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 5703dddf1d..9711fda5cc 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java @@ -63,6 +63,7 @@ import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicLong; import org.eclipse.jgit.annotations.NonNull; import org.eclipse.jgit.annotations.Nullable; @@ -113,6 +114,8 @@ public abstract class Repository implements AutoCloseable { /** Use counter */ final AtomicInteger useCnt = new AtomicInteger(1); + final AtomicLong closedAt = new AtomicLong(); + /** Metadata directory holding the repository's critical files. */ private final File gitDir; @@ -864,8 +867,11 @@ public abstract class Repository implements AutoCloseable { /** Decrement the use count, and maybe close resources. */ public void close() { if (useCnt.decrementAndGet() == 0) { - doClose(); - RepositoryCache.unregister(this); + if (RepositoryCache.isCached(this)) { + closedAt.set(System.currentTimeMillis()); + } else { + doClose(); + } } } 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 22b5fcd112..29ef084830 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RepositoryCache.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RepositoryCache.java @@ -52,17 +52,26 @@ import java.util.Collection; import java.util.Iterator; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.ScheduledThreadPoolExecutor; +import java.util.concurrent.TimeUnit; +import org.eclipse.jgit.annotations.NonNull; import org.eclipse.jgit.errors.RepositoryNotFoundException; import org.eclipse.jgit.internal.storage.file.FileRepository; import org.eclipse.jgit.util.FS; import org.eclipse.jgit.util.IO; import org.eclipse.jgit.util.RawParseUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** Cache of active {@link Repository} instances. */ public class RepositoryCache { private static final RepositoryCache cache = new RepositoryCache(); + private final static Logger LOG = LoggerFactory + .getLogger(RepositoryCache.class); + /** * Open an existing repository, reusing a cached instance if possible. * <p> @@ -138,10 +147,10 @@ public class RepositoryCache { * @param db * repository to unregister. */ - public static void close(final Repository db) { + public static void close(@NonNull final Repository db) { if (db.getDirectory() != null) { FileKey key = FileKey.exact(db.getDirectory(), db.getFS()); - cache.unregisterAndCloseRepository(key); + cache.unregisterAndCloseRepository(key, db); } } @@ -187,20 +196,69 @@ public class RepositoryCache { return cache.getKeys(); } + static boolean isCached(@NonNull Repository repo) { + File gitDir = repo.getDirectory(); + if (gitDir == null) { + return false; + } + FileKey key = new FileKey(gitDir, repo.getFS()); + Reference<Repository> repoRef = cache.cacheMap.get(key); + return repoRef != null && repoRef.get() == repo; + } + /** Unregister all repositories from the cache. */ public static void clear() { cache.clearAll(); } + static void clearExpired() { + cache.clearAllExpired(); + } + + static void reconfigure(RepositoryCacheConfig repositoryCacheConfig) { + cache.configureEviction(repositoryCacheConfig); + } + private final ConcurrentHashMap<Key, Reference<Repository>> cacheMap; private final Lock[] openLocks; + private ScheduledFuture<?> cleanupTask; + + private volatile long expireAfter; + private RepositoryCache() { cacheMap = new ConcurrentHashMap<Key, Reference<Repository>>(); openLocks = new Lock[4]; - for (int i = 0; i < openLocks.length; i++) + for (int i = 0; i < openLocks.length; i++) { openLocks[i] = new Lock(); + } + configureEviction(new RepositoryCacheConfig()); + } + + private void configureEviction( + RepositoryCacheConfig repositoryCacheConfig) { + expireAfter = repositoryCacheConfig.getExpireAfter(); + ScheduledThreadPoolExecutor scheduler = WorkQueue.getExecutor(); + synchronized (scheduler) { + if (cleanupTask != null) { + cleanupTask.cancel(false); + } + long delay = repositoryCacheConfig.getCleanupDelay(); + if (delay == RepositoryCacheConfig.NO_CLEANUP) { + return; + } + cleanupTask = scheduler.scheduleWithFixedDelay(new Runnable() { + @Override + public void run() { + try { + cache.clearAllExpired(); + } catch (Throwable e) { + LOG.error(e.getMessage(), e); + } + } + }, delay, delay, TimeUnit.MILLISECONDS); + } } @SuppressWarnings("resource") @@ -239,10 +297,20 @@ public class RepositoryCache { return oldRef != null ? oldRef.get() : null; } - private void unregisterAndCloseRepository(final Key location) { - Repository oldDb = unregisterRepository(location); - if (oldDb != null) { - oldDb.close(); + private boolean isExpired(Repository db) { + return db != null && db.useCnt.get() == 0 + && (System.currentTimeMillis() - db.closedAt.get() > expireAfter); + } + + private void unregisterAndCloseRepository(final Key location, + Repository db) { + synchronized (lockFor(location)) { + if (isExpired(db)) { + Repository oldDb = unregisterRepository(location); + if (oldDb != null) { + oldDb.close(); + } + } } } @@ -250,6 +318,15 @@ public class RepositoryCache { return new ArrayList<Key>(cacheMap.keySet()); } + private void clearAllExpired() { + for (Reference<Repository> ref : cacheMap.values()) { + Repository db = ref.get(); + if (isExpired(db)) { + RepositoryCache.close(db); + } + } + } + private void clearAll() { for (int stage = 0; stage < 2; stage++) { for (Iterator<Map.Entry<Key, Reference<Repository>>> i = cacheMap diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RepositoryCacheConfig.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RepositoryCacheConfig.java new file mode 100644 index 0000000000..428dea3e67 --- /dev/null +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RepositoryCacheConfig.java @@ -0,0 +1,152 @@ +/* + * Copyright (C) 2016 Ericsson + * and other copyright owners as documented in the project's IP log. + * + * This program and the accompanying materials are made available + * under the terms of the Eclipse Distribution License v1.0 which + * accompanies this distribution, is reproduced below, and is + * available at http://www.eclipse.org/org/documents/edl-v10.php + * + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following + * conditions are met: + * + * - Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials provided + * with the distribution. + * + * - Neither the name of the Eclipse Foundation, Inc. nor the + * names of its contributors may be used to endorse or promote + * products derived from this software without specific prior + * written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND + * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, + * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER + * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF + * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +package org.eclipse.jgit.lib; + +import java.util.concurrent.TimeUnit; + +/** + * Configuration parameters for JVM-wide repository cache used by JGit. + * + * @since 4.4 + */ +public class RepositoryCacheConfig { + + /** + * Set cleanupDelayMillis to this value in order to switch off time-based + * cache eviction. The JVM can still expire cache entries when heap memory + * runs low. + */ + public static final long NO_CLEANUP = 0; + + /** + * Set cleanupDelayMillis to this value in order to auto-set it to minimum + * of 1/10 of expireAfterMillis and 10 minutes + */ + public static final long AUTO_CLEANUP_DELAY = -1; + + private long expireAfterMillis; + + private long cleanupDelayMillis; + + /** Create a default configuration. */ + public RepositoryCacheConfig() { + expireAfterMillis = TimeUnit.HOURS.toMillis(1); + cleanupDelayMillis = AUTO_CLEANUP_DELAY; + } + + /** + * @return the time an unused repository should expired and be evicted from + * the RepositoryCache in milliseconds. <b>Default is 1 hour.</b> + */ + public long getExpireAfter() { + return expireAfterMillis; + } + + /** + * @param expireAfterMillis + * the time an unused repository should expired and be evicted + * from the RepositoryCache in milliseconds. + */ + public void setExpireAfter(long expireAfterMillis) { + this.expireAfterMillis = expireAfterMillis; + } + + /** + * @return the delay between the periodic cleanup of expired repository in + * milliseconds. <b>Default is minimum of 1/10 of expireAfterMillis + * and 10 minutes</b> + */ + public long getCleanupDelay() { + if (cleanupDelayMillis < 0) { + return Math.min(expireAfterMillis / 10, + TimeUnit.MINUTES.toMillis(10)); + } + return cleanupDelayMillis; + } + + /** + * @param cleanupDelayMillis + * the delay between the periodic cleanup of expired repository + * in milliseconds. Set it to {@link #AUTO_CLEANUP_DELAY} to + * automatically derive cleanup delay from expireAfterMillis. + * <p> + * Set it to {@link #NO_CLEANUP} in order to switch off cache + * expiration. + * <p> + * If cache expiration is switched off the JVM still can evict + * cache entries when the JVM is running low on available heap + * memory. + */ + public void setCleanupDelay(long cleanupDelayMillis) { + this.cleanupDelayMillis = cleanupDelayMillis; + } + + /** + * Update properties by setting fields from the configuration. + * <p> + * If a property is not defined in the configuration, then it is left + * unmodified. + * + * @param config + * configuration to read properties from. + * @return {@code this}. + */ + public RepositoryCacheConfig fromConfig(Config config) { + setExpireAfter( + config.getTimeUnit("core", null, "repositoryCacheExpireAfter", //$NON-NLS-1$//$NON-NLS-2$ + getExpireAfter(), TimeUnit.MILLISECONDS)); + setCleanupDelay( + config.getTimeUnit("core", null, "repositoryCacheCleanupDelay", //$NON-NLS-1$ //$NON-NLS-2$ + AUTO_CLEANUP_DELAY, TimeUnit.MILLISECONDS)); + return this; + } + + /** + * Install this configuration as the live settings. + * <p> + * The new configuration is applied immediately. + */ + public void install() { + RepositoryCache.reconfigure(this); + } +} |