From 2461978aab5da1c4c863c95f89f8944ab9111c76 Mon Sep 17 00:00:00 2001 From: contrueCT Date: Sun, 8 Nov 2026 02:37:33 +0800 Subject: [PATCH 1/4] fix(core): avoid caching empty edge batches --- .../backend/cache/CachedGraphTransaction.java | 4 ++- .../cache/CachedGraphTransactionTest.java | 32 ++++++++++++++++--- 2 files changed, 30 insertions(+), 6 deletions(-) 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..07a24cbe40 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,9 @@ protected QueryResults fetchEdgeBatch(Query query) { } QueryResults fetched = super.fetchEdgeBatch(query); if (!fetched.batches().hasNext()) { - this.cacheEdgeBatch(cacheKey, batchLabel, Collections.emptyList()); + // An empty backend response may be transient (for example while + // a distributed owner is being refreshed). Never turn it into a + // negative cache entry that survives until cache expiry. 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..308dc5e841 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 @@ -747,7 +747,9 @@ public void testEdgeBatchCacheRoundTripsThroughOffheap() throws Exception { Edge warm = QueryResults.one(transaction.queryEdgesByVertex(first.id())); Assert.assertEquals(edge.id(), warm.id()); Assert.assertTrue(offheap.hits() > hits); - Mockito.verify(store, Mockito.never()).query(Mockito.any(Query.class)); + // Positive edge candidates are served from cache. A nested query + // that previously returned no batch is deliberately retried. + Mockito.verify(store, Mockito.atMost(1)).query(Mockito.any(Query.class)); } finally { transaction.close(); AutoCloseable nativeCache = Whitebox.getInternalState(offheap, "cache"); @@ -839,7 +841,7 @@ public void testOversizedEdgeBatchReturnsEveryCandidateWithoutCaching() { } @Test - public void testEmptyEdgeBatchesRespectBatchCountLimit() { + public void testEmptyEdgeBatchesAreNotCached() { this.persistEdges(1); ConditionQuery root = this.edgeCacheRequest(); Id label = this.graph.edgeLabel("person_know_person").id(); @@ -860,9 +862,29 @@ public void testEmptyEdgeBatchesRespectBatchCountLimit() { for (int i = 0; i < 100; i++) { Assert.assertTrue(this.fetchEdgeIds(transaction, batches.get(i)).isEmpty()); } - Mockito.verify(store, Mockito.never()).query(Mockito.any(Query.class)); - Assert.assertTrue(this.fetchEdgeIds(transaction, batches.get(100)).isEmpty()); - Mockito.verify(store, Mockito.times(1)).query(Mockito.any(Query.class)); + Mockito.verify(store, Mockito.times(100)).query(Mockito.any(Query.class)); + } finally { + transaction.close(); + } + } + + @Test + public void testTransientEmptyEdgeBatchIsNotReused() { + List ids = this.persistEdges(1); + ConditionQuery root = this.edgeCacheRequest(); + BackendStore store = Mockito.spy(this.params.loadGraphStore()); + 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(); } From 8f90a7c6a46b8da924fd2ef4d4ce6312926fd80e Mon Sep 17 00:00:00 2001 From: contrueCT Date: Sun, 8 Nov 2026 02:44:38 +0800 Subject: [PATCH 2/4] docs: add edge-cache flow diagram --- .../pr-diagrams/edge-cache-empty-result.svg | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 .github/pr-diagrams/edge-cache-empty-result.svg 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..fbcf414f21 --- /dev/null +++ b/.github/pr-diagrams/edge-cache-empty-result.svg @@ -0,0 +1,38 @@ + + Edge cache empty-result flow + A transient empty backend response is not cached, while a later positive edge response is cached. + + + + + + + + + + Edge query cache policy + + Query + same key + + + Cache miss + backend read + + + Empty batch + transient read + + + Skip cache + retry later + + + Edge batch + backend recovers + + + Cache positive + serve future hits + + From 87d2cff15c781f7dfe7504b5254fc66115404cb1 Mon Sep 17 00:00:00 2001 From: contrueCT Date: Sun, 8 Nov 2026 20:13:07 +0800 Subject: [PATCH 3/4] fix(core): keep empty edge cache safe for hstore --- .../backend/cache/CachedGraphTransaction.java | 10 +++- .../cache/CachedGraphTransactionTest.java | 59 +++++++++++++++++-- 2 files changed, 60 insertions(+), 9 deletions(-) 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 07a24cbe40..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,9 +406,13 @@ protected QueryResults fetchEdgeBatch(Query query) { } QueryResults fetched = super.fetchEdgeBatch(query); if (!fetched.batches().hasNext()) { - // An empty backend response may be transient (for example while - // a distributed owner is being refreshed). Never turn it into a - // negative cache entry that survives until cache expiry. + // 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 308dc5e841..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 @@ -747,9 +747,7 @@ public void testEdgeBatchCacheRoundTripsThroughOffheap() throws Exception { Edge warm = QueryResults.one(transaction.queryEdgesByVertex(first.id())); Assert.assertEquals(edge.id(), warm.id()); Assert.assertTrue(offheap.hits() > hits); - // Positive edge candidates are served from cache. A nested query - // that previously returned no batch is deliberately retried. - Mockito.verify(store, Mockito.atMost(1)).query(Mockito.any(Query.class)); + Mockito.verify(store, Mockito.never()).query(Mockito.any(Query.class)); } finally { transaction.close(); AutoCloseable nativeCache = Whitebox.getInternalState(offheap, "cache"); @@ -841,7 +839,7 @@ public void testOversizedEdgeBatchReturnsEveryCandidateWithoutCaching() { } @Test - public void testEmptyEdgeBatchesAreNotCached() { + public void testEmptyEdgeBatchesRespectBatchCountLimit() { this.persistEdges(1); ConditionQuery root = this.edgeCacheRequest(); Id label = this.graph.edgeLabel("person_know_person").id(); @@ -862,17 +860,20 @@ public void testEmptyEdgeBatchesAreNotCached() { for (int i = 0; i < 100; i++) { Assert.assertTrue(this.fetchEdgeIds(transaction, batches.get(i)).isEmpty()); } - Mockito.verify(store, Mockito.times(100)).query(Mockito.any(Query.class)); + Mockito.verify(store, Mockito.never()).query(Mockito.any(Query.class)); + Assert.assertTrue(this.fetchEdgeIds(transaction, batches.get(100)).isEmpty()); + Mockito.verify(store, Mockito.times(1)).query(Mockito.any(Query.class)); } finally { transaction.close(); } } @Test - public void testTransientEmptyEdgeBatchIsNotReused() { + 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 { @@ -890,6 +891,52 @@ public void testTransientEmptyEdgeBatchIsNotReused() { } } + @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); From 4069a59a9618ff4c26f047a852c3897eebfc9984 Mon Sep 17 00:00:00 2001 From: contrueCT Date: Sun, 8 Nov 2026 20:15:15 +0800 Subject: [PATCH 4/4] docs: show backend-aware empty cache policy --- .../pr-diagrams/edge-cache-empty-result.svg | 55 ++++++++++--------- 1 file changed, 30 insertions(+), 25 deletions(-) diff --git a/.github/pr-diagrams/edge-cache-empty-result.svg b/.github/pr-diagrams/edge-cache-empty-result.svg index fbcf414f21..f39b5aa8f0 100644 --- a/.github/pr-diagrams/edge-cache-empty-result.svg +++ b/.github/pr-diagrams/edge-cache-empty-result.svg @@ -1,6 +1,6 @@ - - Edge cache empty-result flow - A transient empty backend response is not cached, while a later positive edge response is cached. + + Backend-aware empty edge cache flow + Local backends retain empty-result caching; HStore skips empty-result caching so later reads can recover. @@ -9,30 +9,35 @@ - - Edge query cache policy + + Empty edge result policy - Query - same key + Edge query + cache miss - Cache miss - backend read + Backend read + empty batch - - Empty batch - transient read - - - Skip cache - retry later - - - Edge batch - backend recovers - - - Cache positive - serve future hits - + + 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