diff --git a/.github/pr-diagrams/edge-cache-empty-result.svg b/.github/pr-diagrams/edge-cache-empty-result.svg new file mode 100644 index 0000000000..f39b5aa8f0 --- /dev/null +++ b/.github/pr-diagrams/edge-cache-empty-result.svg @@ -0,0 +1,43 @@ + + Backend-aware empty edge cache flow + Local backends retain empty-result caching; HStore skips empty-result caching so later reads can recover. + + + + + + + + + + Empty edge result policy + + Edge query + cache miss + + + Backend read + empty batch + + + Backend type + local / HStore + + + local + HStore + + Cache empty + stable local miss + + + Fast repeats + one backend read + + Skip cache + retry after refresh + + + No stale miss + more backend reads + diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/cache/CachedGraphTransaction.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/cache/CachedGraphTransaction.java index e6efbc94f0..d34087d774 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/cache/CachedGraphTransaction.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/cache/CachedGraphTransaction.java @@ -406,7 +406,13 @@ protected QueryResults fetchEdgeBatch(Query query) { } QueryResults fetched = super.fetchEdgeBatch(query); if (!fetched.batches().hasNext()) { - this.cacheEdgeBatch(cacheKey, batchLabel, Collections.emptyList()); + // A distributed backend can transiently return an empty batch + // while an owner or partition leader is being refreshed. Do not + // turn that response into a negative cache entry. Local backends + // retain the existing negative cache for stable empty queries. + if (!this.store().provider().isHstore()) { + this.cacheEdgeBatch(cacheKey, batchLabel, Collections.emptyList()); + } return fetched; } return fetched.mapBatches(batch -> { diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/cache/CachedGraphTransactionTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/cache/CachedGraphTransactionTest.java index a18bcbec6d..833466c506 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/cache/CachedGraphTransactionTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/cache/CachedGraphTransactionTest.java @@ -868,6 +868,75 @@ public void testEmptyEdgeBatchesRespectBatchCountLimit() { } } + @Test + public void testTransientEmptyEdgeBatchIsNotReusedOnHstore() { + List ids = this.persistEdges(1); + ConditionQuery root = this.edgeCacheRequest(); + BackendStore store = Mockito.spy(this.params.loadGraphStore()); + this.markAsHstore(store); + CachedGraphTransaction transaction = new CachedGraphTransaction(this.params, + store); + try { + transaction.clearCache(null, false); + Mockito.doReturn(Collections.emptyIterator()) + .when(store).query(Mockito.any(Query.class)); + Assert.assertTrue(this.fetchEdgeIds(transaction, root).isEmpty()); + + Mockito.clearInvocations(store); + Mockito.doCallRealMethod().when(store).query(Mockito.any(Query.class)); + this.assertEdgeIds(ids, this.fetchEdgeIds(transaction, root)); + Mockito.verify(store).query(Mockito.any(Query.class)); + } finally { + transaction.close(); + } + } + + @Test + public void testRepeatedEmptyEdgeCostDiffersByBackendType() { + this.persistEdges(1); + ConditionQuery root = this.edgeCacheRequest(); + BackendStore local = Mockito.spy(this.params.loadGraphStore()); + CachedGraphTransaction localTransaction = new CachedGraphTransaction( + this.params, local); + BackendStore distributed = Mockito.spy(this.params.loadGraphStore()); + this.markAsHstore(distributed); + CachedGraphTransaction distributedTransaction = new CachedGraphTransaction( + this.params, distributed); + try { + localTransaction.clearCache(null, false); + distributedTransaction.clearCache(null, false); + Mockito.doReturn(Collections.emptyIterator()) + .when(local).query(Mockito.any(Query.class)); + Mockito.doReturn(Collections.emptyIterator()) + .when(distributed).query(Mockito.any(Query.class)); + Mockito.clearInvocations(local, distributed); + + for (int i = 0; i < 100; i++) { + Assert.assertTrue(this.fetchEdgeIds(localTransaction, root).isEmpty()); + } + Mockito.verify(local, Mockito.times(1)).query(Mockito.any(Query.class)); + + // Both transactions use the graph-level cache. Clear the local + // negative entry before measuring distributed reads. + distributedTransaction.clearCache(null, false); + Mockito.clearInvocations(distributed); + for (int i = 0; i < 100; i++) { + Assert.assertTrue(this.fetchEdgeIds(distributedTransaction, root).isEmpty()); + } + Mockito.verify(distributed, Mockito.times(100)) + .query(Mockito.any(Query.class)); + } finally { + localTransaction.close(); + distributedTransaction.close(); + } + } + + private void markAsHstore(BackendStore store) { + BackendStoreProvider provider = Mockito.spy(store.provider()); + Mockito.doReturn(true).when(provider).isHstore(); + Whitebox.setInternalState(store, "provider", provider); + } + private List persistEdges(int count) { HugeVertex first = this.newVertex(IdGenerator.of(1L)); this.cache.addVertex(first);