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 d3d7514..a1c83ec 100644
--- a/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/ChangesTsCache.java
+++ b/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/ChangesTsCache.java
@@ -14,11 +14,9 @@
package com.googlesource.gerrit.modules.gitrefsfilter;
import com.google.common.cache.CacheLoader;
-import com.google.common.flogger.FluentLogger;
import com.google.gerrit.reviewdb.server.ReviewDb;
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.Provider;
@@ -47,7 +45,6 @@
@Singleton
static class Loader extends CacheLoader<ChangeCacheKey, Long> {
- private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final ChangeNotes.Factory changeNotesFactory;
private final ChangesTsCache changesTsCache;
@@ -59,24 +56,16 @@
@Override
public Long load(ChangeCacheKey key) throws Exception {
- try {
- Long res =
- changeNotesFactory
- .createChecked(
- changesTsCache.dbProvider.get(),
- key.repo(),
- key.project(),
- key.changeId(),
- key.changeRevision())
- .getChange()
- .getLastUpdatedOn()
- .getTime();
- return res;
- } catch (NoSuchChangeException e) {
- logger.atFine().withCause(e).log(
- "Change %d does not exist: returning zero epoch", key.changeId().get());
- return 0L;
- }
+ return changeNotesFactory
+ .createChecked(
+ changesTsCache.dbProvider.get(),
+ 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 7091f5f..8c50d44 100644
--- a/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/OpenChangesCache.java
+++ b/src/main/java/com/googlesource/gerrit/modules/gitrefsfilter/OpenChangesCache.java
@@ -14,11 +14,9 @@
package com.googlesource.gerrit.modules.gitrefsfilter;
import com.google.common.cache.CacheLoader;
-import com.google.common.flogger.FluentLogger;
import com.google.gerrit.reviewdb.server.ReviewDb;
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.Provider;
@@ -47,7 +45,6 @@
@Singleton
static class Loader extends CacheLoader<ChangeCacheKey, Boolean> {
- private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final ChangeNotes.Factory changeNotesFactory;
private final OpenChangesCache openChangesCache;
@@ -59,20 +56,14 @@
@Override
public Boolean load(ChangeCacheKey key) throws Exception {
- try {
- ChangeNotes changeNotes =
- changeNotesFactory.createChecked(
- openChangesCache.dbProvider.get(),
- 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().get());
- return false;
- }
+ ChangeNotes changeNotes =
+ changeNotesFactory.createChecked(
+ openChangesCache.dbProvider.get(),
+ key.repo(),
+ key.project(),
+ key.changeId(),
+ key.changeRevision());
+ return changeNotes.getChange().getStatus().isOpen();
}
}
}
