Do not cache negative results from open changes / timestamp lookups
When the open changes and ts caches are failing to load their
respective data from the repository, the caching of the failure
may have disruptive consequences.
If the failure was just the result of an intermediate replication
event (e.g. a new patch-set has been been replicated but its associated
/meta is still pending) the negative caching would result in making
the ref not discoverable.
The management of the loading failures from the cache is already
coded in the ForProjectWrapper and therefore there is no need to
duplicate a similar logic in the caches as well.
Bug: Issue 299486879
Change-Id: Ic8015238d1f61ad6c47ebdbf6519533dbfa7ec04
diff --git a/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/ChangesTsCache.java b/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/ChangesTsCache.java
index cfd7a0f..248d347 100644
--- a/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/ChangesTsCache.java
+++ b/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/ChangesTsCache.java
@@ -14,10 +14,8 @@
package com.googlesource.gerrit.modules.gitrefsfilter;
import com.google.common.cache.CacheLoader;
-import com.google.common.flogger.FluentLogger;
import com.google.gerrit.server.cache.CacheModule;
import com.google.gerrit.server.notedb.ChangeNotes;
-import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.inject.Inject;
import com.google.inject.Module;
import com.google.inject.Singleton;
@@ -38,7 +36,6 @@
@Singleton
static class Loader extends CacheLoader<ChangeCacheKey, Long> {
- private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final ChangeNotes.Factory changeNotesFactory;
@Inject
@@ -48,17 +45,11 @@
@Override
public Long load(ChangeCacheKey key) throws Exception {
- try {
- return changeNotesFactory
- .createChecked(key.repo(), key.project(), key.changeId(), key.changeRevision())
- .getChange()
- .getLastUpdatedOn()
- .getTime();
- } catch (NoSuchChangeException e) {
- logger.atFine().withCause(e).log(
- "Change %d does not exist: returning zero epoch", key.changeId());
- return 0L;
- }
+ return changeNotesFactory
+ .createChecked(key.repo(), key.project(), key.changeId(), key.changeRevision())
+ .getChange()
+ .getLastUpdatedOn()
+ .getTime();
}
}
}
diff --git a/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/OpenChangesCache.java b/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/OpenChangesCache.java
index 8e1c371..d8dd16b 100644
--- a/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/OpenChangesCache.java
+++ b/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/OpenChangesCache.java
@@ -14,10 +14,8 @@
package com.googlesource.gerrit.modules.gitrefsfilter;
import com.google.common.cache.CacheLoader;
-import com.google.common.flogger.FluentLogger;
import com.google.gerrit.server.cache.CacheModule;
import com.google.gerrit.server.notedb.ChangeNotes;
-import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.inject.Inject;
import com.google.inject.Module;
import com.google.inject.Singleton;
@@ -38,7 +36,6 @@
@Singleton
static class Loader extends CacheLoader<ChangeCacheKey, Boolean> {
- private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final ChangeNotes.Factory changeNotesFactory;
@Inject
@@ -48,16 +45,10 @@
@Override
public Boolean load(ChangeCacheKey key) throws Exception {
- try {
- ChangeNotes changeNotes =
- changeNotesFactory.createChecked(
- key.repo(), key.project(), key.changeId(), key.changeRevision());
- return changeNotes.getChange().getStatus().isOpen();
- } catch (NoSuchChangeException e) {
- logger.atFine().withCause(e).log(
- "Change %d does not exist: hiding from the advertised refs", key.changeId());
- return false;
- }
+ ChangeNotes changeNotes =
+ changeNotesFactory.createChecked(
+ key.repo(), key.project(), key.changeId(), key.changeRevision());
+ return changeNotes.getChange().getStatus().isOpen();
}
}
}
