From 6b0469bcc90094fdb68417f13360c88a9e4eb4e9 Mon Sep 17 00:00:00 2001 From: contrueCT Date: Sat, 5 Sep 2026 11:43:36 +0800 Subject: [PATCH 1/7] fix(server): preserve query result batch boundaries --- .../backend/cache/CachedGraphTransaction.java | 195 ++++---- .../backend/page/PageEntryIterator.java | 111 ++--- .../hugegraph/backend/page/QueryList.java | 87 ++-- .../backend/query/ConditionQuery.java | 29 +- .../hugegraph/backend/query/QueryBatch.java | 255 ++++++++++ .../backend/query/QueryResultContext.java | 118 +++++ .../hugegraph/backend/query/QueryResults.java | 443 ++++++++---------- .../backend/tx/GraphTransaction.java | 283 +++++------ .../hugegraph/backend/page/QueryListTest.java | 231 +++++++++ .../backend/tx/GraphTransactionTest.java | 104 ++-- .../apache/hugegraph/unit/UnitTestSuite.java | 8 +- .../cache/CachedGraphTransactionTest.java | 231 ++++++++- .../hugegraph/unit/core/QueryResultsTest.java | 268 ++++++++--- 13 files changed, 1584 insertions(+), 779 deletions(-) create mode 100644 hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/QueryBatch.java create mode 100644 hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/QueryResultContext.java create mode 100644 hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/backend/page/QueryListTest.java 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 1543d73151..3680977b4d 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 @@ -20,7 +20,6 @@ import java.lang.reflect.Array; import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.Iterator; import java.util.List; import java.util.Set; @@ -32,6 +31,8 @@ import org.apache.hugegraph.backend.id.Id; import org.apache.hugegraph.backend.query.IdQuery; import org.apache.hugegraph.backend.query.Query; +import org.apache.hugegraph.backend.query.QueryBatch; +import org.apache.hugegraph.backend.query.QueryResultContext; import org.apache.hugegraph.backend.query.QueryResults; import org.apache.hugegraph.backend.store.BackendMutation; import org.apache.hugegraph.backend.store.BackendStore; @@ -316,135 +317,131 @@ private boolean needCacheVertex(HugeVertex vertex) { @Override @Watched(prefix = "graphcache") - protected Iterator queryVerticesFromBackend(Query query) { - if (this.enableCacheVertex() && - query.idsSize() > 0 && query.conditionsSize() == 0 && - !queryNeedsPostFilter(query)) { - return this.queryVerticesByIds((IdQuery) query); - } else { - return super.queryVerticesFromBackend(query); + protected QueryResults fetchVertexBatch(Query query) { + if (!this.enableCacheVertex() || query.idsSize() == 0 || query.conditionsSize() != 0) { + return super.fetchVertexBatch(query); } - } - - @Watched(prefix = "graphcache") - private Iterator queryVerticesByIds(IdQuery query) { - if (query.idsSize() == 1) { - Id vertexId = query.ids().iterator().next(); - HugeVertex vertex = (HugeVertex) this.verticesCache.get(vertexId); - if (vertex != null) { - if (!vertex.expired()) { - return QueryResults.iterator(vertex); - } - this.verticesCache.invalidate(vertexId); - } - Iterator rs = super.queryVerticesFromBackend(query); - vertex = QueryResults.one(rs); - if (vertex == null) { - return QueryResults.emptyIterator(); - } - if (needCacheVertex(vertex)) { - this.verticesCache.update(vertex.id(), vertex); - } - return QueryResults.iterator(vertex); - } - - IdQuery newQuery = new IdQuery(HugeType.VERTEX, query); + QueryResultContext context = new QueryResultContext(query); + IdQuery missing = new IdQuery(query.resultType(), query); List vertices = new ArrayList<>(); - for (Id vertexId : query.ids()) { - HugeVertex vertex = (HugeVertex) this.verticesCache.get(vertexId); - if (vertex == null) { - newQuery.query(vertexId); - } else if (vertex.expired()) { - newQuery.query(vertexId); - this.verticesCache.invalidate(vertexId); + for (Id id : query.ids()) { + HugeVertex vertex = (HugeVertex) this.verticesCache.get(id); + if (vertex == null || vertex.expired()) { + missing.query(id); + if (vertex != null) { + this.verticesCache.invalidate(id); + } } else { vertices.add(vertex); } } - - // Join results from cache and backend - ExtendableIterator results = new ExtendableIterator<>(); - if (!vertices.isEmpty()) { - results.extend(vertices.iterator()); - } else { - // Just use the origin query if find none from the cache - newQuery = query; - } - - if (!newQuery.empty()) { - Iterator rs = super.queryVerticesFromBackend(newQuery); - // Generally there are not too much data with id query - ListIterator listIterator = QueryResults.toList(rs); - for (HugeVertex vertex : listIterator.list()) { - // Skip large vertex - if (needCacheVertex(vertex)) { + if (!missing.empty()) { + QueryResults fetched = super.fetchVertexBatch(vertices.isEmpty() ? query : missing); + ListIterator candidates = QueryResults.toList(fetched.iterator()); + for (HugeVertex vertex : candidates.list()) { + if (this.needCacheVertex(vertex)) { this.verticesCache.update(vertex.id(), vertex); } + vertices.add(vertex); } - results.extend(listIterator); } - - return results; + // Keep hits and misses in one logical batch for filtering and ID ordering. + return this.filterExpiredBatches(new QueryResults<>(vertices.iterator(), context)); } @Override - @Watched(prefix = "graphcache") - protected Iterator queryEdgesFromBackend(Query query) { + protected QueryResults queryEdgesFromMemory(Query query) { RamTable ramtable = this.params().ramtable(); if (ramtable != null && ramtable.matched(query)) { - return ramtable.query(query); + return new QueryResults<>(ramtable.query(query), query); } + return null; + } - if (!this.enableCacheEdge() || query.empty() || query.paging() || - query.bigCapacity() || queryNeedsPostFilter(query)) { - // Don't cache all-edge, paging, large, or post-filtered queries - return super.queryEdgesFromBackend(query); + @Override + @Watched(prefix = "graphcache") + protected QueryResults fetchEdgeBatch(Query query) { + QueryResultContext context = new QueryResultContext(query); + List chain = context.queries(); + Query request = chain.get(chain.size() - 1); + if (!this.enableCacheEdge() || request.empty() || request.paging() || request.bigCapacity()) { + return super.fetchEdgeBatch(query); } - - Id cacheKey = new QueryId(query); - Object value = this.edgesCache.get(cacheKey); - @SuppressWarnings("unchecked") - Collection edges = (Collection) value; - if (value != null) { - for (HugeEdge edge : edges) { + Id cacheKey = new QueryId(request); + Id batchKey = new QueryId(query); + CachedEdgeQuery group = new CachedEdgeQuery(this.edgesCache.get(cacheKey)); + Collection cached = group.get(batchKey); + if (cached != null) { + for (HugeEdge edge : cached) { if (edge.expired()) { this.edgesCache.invalidate(cacheKey); - value = null; + cached = null; break; } } } - - if (value != null) { - // Not cached or the cache expired - return edges.iterator(); + if (cached != null) { + return this.filterExpiredBatches(new QueryResults<>(cached.iterator(), context)); } + QueryResults fetched = super.fetchEdgeBatch(query); + return fetched.mapBatches(batch -> { + Iterator source = batch.results(); + List candidates = new ArrayList<>(MAX_CACHE_EDGES_PER_QUERY + 1); + // Limit probing to this batch; never request another batch to fill the cache. + while (candidates.size() <= MAX_CACHE_EDGES_PER_QUERY && source.hasNext()) { + candidates.add(source.next()); + } + if (candidates.size() <= MAX_CACHE_EDGES_PER_QUERY) { + synchronized (this.edgesCache) { + CachedEdgeQuery existing = new CachedEdgeQuery(this.edgesCache.get(cacheKey)); + if (existing.put(batchKey, candidates)) { + this.edgesCache.update(cacheKey, existing.values); + } + } + } + return new QueryBatch<>( + new ExtendableIterator<>(candidates.iterator(), source), batch.context()); + }); + } - Iterator rs = super.queryEdgesFromBackend(query); - if (queryNeedsPostFilter(query)) { - // The backend query may promote query.optimized() through origin- - // query propagation, so re-check before caching - return rs; - } + /** Nested lists retain the existing off-heap cache's serialization support. */ + private static final class CachedEdgeQuery { - /* - * Iterator can't be cached, caching list instead - * there may be super node and too many edges in a query, - * try fetch a few of the head results and determine whether to cache. - */ - final int tryMax = 1 + MAX_CACHE_EDGES_PER_QUERY; - edges = new ArrayList<>(tryMax); - for (int i = 0; rs.hasNext() && i < tryMax; i++) { - edges.add(rs.next()); + // Alternating batch query strings and raw candidate lists; never store filter closures. + private final List values; + + @SuppressWarnings("unchecked") + private CachedEdgeQuery(Object cached) { + this.values = cached == null ? new ArrayList<>() : + new ArrayList<>((List) cached); } - if (edges.isEmpty()) { - this.edgesCache.update(cacheKey, Collections.emptyList()); - } else if (edges.size() <= MAX_CACHE_EDGES_PER_QUERY) { - this.edgesCache.update(cacheKey, edges); + @SuppressWarnings("unchecked") + public Collection get(Id batch) { + for (int i = 0; i < this.values.size(); i += 2) { + if (this.values.get(i).equals(batch.asString())) { + return (List) this.values.get(i + 1); + } + } + return null; } - return new ExtendableIterator<>(edges.iterator(), rs); + public boolean put(Id batch, List candidates) { + if (this.get(batch) != null) { + return false; + } + int size = candidates.size(); + for (int i = 1; i < this.values.size(); i += 2) { + size += ((List) this.values.get(i)).size(); + } + if (size > MAX_CACHE_EDGES_PER_QUERY || + this.values.size() / 2 >= MAX_CACHE_EDGES_PER_QUERY) { + return false; + } + this.values.add(batch.asString()); + this.values.add(new ArrayList<>(candidates)); + return true; + } } @Override diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/page/PageEntryIterator.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/page/PageEntryIterator.java index bbc93c79b6..55c5fa50bf 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/page/PageEntryIterator.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/page/PageEntryIterator.java @@ -17,112 +17,71 @@ package org.apache.hugegraph.backend.page; -import java.util.NoSuchElementException; +import java.util.Iterator; import org.apache.hugegraph.backend.query.Query; -import org.apache.hugegraph.backend.query.QueryResults; +import org.apache.hugegraph.backend.query.QueryBatch.BatchIterator; +import org.apache.hugegraph.backend.query.QueryBatch; import org.apache.hugegraph.exception.NotSupportException; -import org.apache.hugegraph.iterator.CIter; import org.apache.hugegraph.util.E; -import org.apache.tinkerpop.gremlin.structure.util.CloseableIterator; -public class PageEntryIterator implements CIter { +/** Produces pages without probing the following page to delimit the current one. */ +public class PageEntryIterator extends BatchIterator> { private final QueryList queries; private final long pageSize; private final PageInfo pageInfo; - private final QueryResults queryResults; // for upper layer - - private QueryList.PageResults pageResults; + private Iterator> pageBatches; private long remaining; public PageEntryIterator(QueryList queries, long pageSize) { this.queries = queries; this.pageSize = pageSize; - this.pageInfo = this.parsePageInfo(); - this.queryResults = new QueryResults<>(this, queries.parent()); - - this.pageResults = QueryList.PageResults.emptyIterator(); + this.pageInfo = PageInfo.fromString(queries.parent().pageWithoutCheck()); + E.checkState(this.pageInfo.offset() < queries.total(), + "Invalid page offset '%s' exceeds the size of IdHolderList", + this.pageInfo.offset()); this.remaining = queries.parent().limit(); } - private PageInfo parsePageInfo() { - String page = this.queries.parent().pageWithoutCheck(); - PageInfo pageInfo = PageInfo.fromString(page); - E.checkState(pageInfo.offset() < this.queries.total(), - "Invalid page '%s' with an offset '%s' exceeds " + - "the size of IdHolderList", page, pageInfo.offset()); - return pageInfo; - } - @Override - public boolean hasNext() { - if (this.pageResults.get().hasNext()) { - return true; - } - return this.fetch(); - } - - private boolean fetch() { - if ((this.remaining != Query.NO_LIMIT && this.remaining <= 0L) || - this.pageInfo.offset() >= this.queries.total()) { - return false; - } - - long pageSize = this.pageSize; - if (this.remaining != Query.NO_LIMIT && this.remaining < pageSize) { - pageSize = this.remaining; - } - this.closePageResults(); - this.pageResults = this.queries.fetchNext(this.pageInfo, pageSize); - assert this.pageResults != null; - this.queryResults.setQuery(this.pageResults.query()); - - if (this.pageResults.get().hasNext()) { - if (!this.pageResults.hasNextPage()) { - this.pageInfo.increase(); + protected QueryBatch fetch() throws Exception { + while (true) { + if (this.pageBatches != null && this.pageBatches.hasNext()) { + return this.pageBatches.next(); + } + Iterator> previous = this.pageBatches; + this.pageBatches = null; + QueryBatch.closeAll(previous); + if ((this.remaining != Query.NO_LIMIT && this.remaining <= 0L) || + this.pageInfo.offset() >= this.queries.total()) { + return null; + } + long size = this.remaining == Query.NO_LIMIT ? this.pageSize : + Math.min(this.pageSize, this.remaining); + QueryList.PageResults page = this.queries.fetchNext(this.pageInfo, size); + this.pageBatches = page.results().batches(); + if (page.hasNextPage()) { + this.pageInfo.page(page.page()); } else { - this.pageInfo.page(this.pageResults.page()); + this.pageInfo.increase(); + } + if (this.remaining != Query.NO_LIMIT) { + this.remaining -= page.total(); } - this.remaining -= this.pageResults.total(); - return true; - } else { - this.pageInfo.increase(); - return this.fetch(); - } - } - - private void closePageResults() { - if (this.pageResults != QueryList.PageResults.EMPTY) { - CloseableIterator.closeIterator(this.pageResults.get()); } } @Override - public R next() { - if (!this.hasNext()) { - throw new NoSuchElementException(); - } - return this.pageResults.get().next(); + protected void closeResources() throws Exception { + QueryBatch.closeAll(this.pageBatches, this.queries); } @Override public Object metadata(String meta, Object... args) { if (PageInfo.PAGE.equals(meta)) { - if (this.pageInfo.offset() >= this.queries.total()) { - return null; - } - return this.pageInfo; + return this.pageInfo.offset() >= this.queries.total() ? null : this.pageInfo; } throw new NotSupportException("Invalid meta '%s'", meta); } - - @Override - public void close() throws Exception { - this.closePageResults(); - } - - public QueryResults results() { - return this.queryResults; - } } diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/page/QueryList.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/page/QueryList.java index 327c4aab47..fa6c6bed72 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/page/QueryList.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/page/QueryList.java @@ -25,15 +25,17 @@ import org.apache.hugegraph.backend.id.Id; import org.apache.hugegraph.backend.page.IdHolder.BatchIdHolder; import org.apache.hugegraph.backend.page.IdHolder.FixedIdHolder; -import org.apache.hugegraph.backend.query.ConditionQuery; import org.apache.hugegraph.backend.query.ConditionQuery.OptimizedType; +import org.apache.hugegraph.backend.query.ConditionQuery; import org.apache.hugegraph.backend.query.IdQuery; import org.apache.hugegraph.backend.query.Query; +import org.apache.hugegraph.backend.query.QueryBatch.BatchIterator; +import org.apache.hugegraph.backend.query.QueryBatch; import org.apache.hugegraph.backend.query.QueryResults; import org.apache.hugegraph.util.Bytes; import org.apache.hugegraph.util.E; -public final class QueryList { +public final class QueryList implements AutoCloseable { private final Query parent; // The size of each page fetched by the inner page @@ -87,16 +89,32 @@ public QueryResults fetch(int pageSize) { if (this.parent.paging()) { @SuppressWarnings("resource") // closed by QueryResults PageEntryIterator iter = new PageEntryIterator<>(this, pageSize); - /* - * NOTE: PageEntryIterator query will change every fetch time. - * QueryResults tracks this change and restores input-id order - * within each page without fetching later pages eagerly. - */ - return iter.results(); + return QueryResults.fromBatches(iter); } - // Fetch all results once - return QueryResults.flatMap(this.queries.iterator(), FlattenQuery::iterator); + Iterator> source = this.queries.iterator(); + return QueryResults.flatMap(new BatchIterator>() { + @Override + protected FlattenQuery fetch() { + return source.hasNext() ? source.next() : null; + } + + @Override + protected void closeResources() throws Exception { + QueryList.this.close(); + } + }, FlattenQuery::iterator); + } + + @Override + public void close() throws Exception { + List resources = new ArrayList<>(); + for (FlattenQuery query : this.queries) { + if (query instanceof QueryList.IndexQuery) { + resources.addAll(((IndexQuery) query).holders); + } + } + QueryBatch.closeAll(resources.toArray()); } PageResults fetchNext(PageInfo pageInfo, long pageSize) { @@ -179,7 +197,7 @@ public PageResults iterator(int index, String page, long pageSize) { QueryResults fetched = results.toList(); PageState pageState = PageInfo.pageState(results.iterator()); - return new PageResults<>(fetched, pageState); + return new PageResults<>(fetched, query, pageState); } @Override @@ -220,7 +238,6 @@ public QueryResults iterator() { private QueryResults each(IdHolder holder) { assert !holder.paging(); Query bindQuery = holder.query(); - this.updateResultsFilter(bindQuery); this.updateOffsetIfNeeded(bindQuery); // Iterate by all @@ -237,7 +254,7 @@ private QueryResults each(IdHolder holder) { * in order by ids weight. In addition all the ids (IdQuery) * can be collected by upper layer. */ - return this.queryByIndexIds(ids, holder.keepOrder()); + return this.queryByIndexIds(bindQuery, ids, holder.keepOrder()); } // Iterate by batch @@ -259,7 +276,7 @@ private QueryResults each(IdHolder holder) { return null; } - return this.queryByIndexIds(ids, holder.keepOrder()); + return this.queryByIndexIds(bindQuery, ids, holder.keepOrder()); }); } @@ -270,16 +287,14 @@ public PageResults iterator(int index, String page, long pageSize) { "Invalid page index %s", index); IdHolder holder = this.holders.get(index); Query bindQuery = holder.query(); - this.updateResultsFilter(bindQuery); PageIds pageIds = holder.fetchNext(page, pageSize); if (pageIds.empty()) { return PageResults.emptyIterator(); } - QueryResults results = this.queryByIndexIds(pageIds.ids(), - holder.keepOrder()); - - return new PageResults<>(results, pageIds.pageState()); + IdQuery query = this.indexIdQuery(bindQuery, pageIds.ids(), holder.keepOrder()); + QueryResults results = fetcher().apply(query); + return new PageResults<>(results, query, pageIds.pageState()); } @Override @@ -303,34 +318,33 @@ private void updateOffsetIfNeeded(Query query) { query.copyOffset(parent); } - private void updateResultsFilter(Query query) { - while (query != null) { - if (query instanceof ConditionQuery) { - ((ConditionQuery) query).updateResultsFilter(); - return; - } - query = query.originQuery(); - } + private QueryResults queryByIndexIds(Query bindQuery, Set ids, + boolean inOrder) { + return fetcher().apply(this.indexIdQuery(bindQuery, ids, inOrder)); } - private QueryResults queryByIndexIds(Set ids, boolean inOrder) { - IdQuery query = new IdQuery(parent(), ids); + private IdQuery indexIdQuery(Query bindQuery, Set ids, boolean inOrder) { + // The holder can query an index table; fetch graph elements by ID. + IdQuery query = new IdQuery(parent().resultType(), bindQuery); + query.query(ids); query.mustSortByInput(inOrder); - return fetcher().apply(query); + return query; } } public static class PageResults { public static final PageResults EMPTY = new PageResults<>( - QueryResults.empty(), + QueryResults.empty(), null, PageState.EMPTY); private final QueryResults results; private final PageState pageState; + private final Query query; - public PageResults(QueryResults results, PageState pageState) { + public PageResults(QueryResults results, Query query, PageState pageState) { this.results = results; + this.query = query; this.pageState = pageState; } @@ -344,10 +358,11 @@ public boolean hasNextPage() { } public Query query() { - List queries = this.results.queries(); - E.checkState(queries.size() == 1, - "Expect query size 1, but got: %s", queries); - return queries.get(0); + return this.query; + } + + public QueryResults results() { + return this.results; } public String page() { diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/ConditionQuery.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/ConditionQuery.java index 097e98df19..6da8da23db 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/ConditionQuery.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/ConditionQuery.java @@ -611,6 +611,10 @@ public Condition.Relation copyRelationAndUpdateQuery(Object key) { @Override public boolean test(HugeElement element) { + return this.test(element, this.resultsFilter); + } + + public boolean test(HugeElement element, ResultsFilter filter) { if (!this.ids().isEmpty() && !super.test(element)) { return false; } @@ -622,8 +626,8 @@ public boolean test(HugeElement element) { * We can't use sub-query results-filter here for fresh element which is * not committed to backend store, because it's not from a sub-query. */ - if (this.resultsFilter != null && !element.fresh()) { - return this.resultsFilter.test(element); + if (filter != null && !element.fresh()) { + return filter.test(element); } /* @@ -705,25 +709,8 @@ public void registerResultsFilter(ResultsFilter filter) { this.resultsFilter = filter; } - public void updateResultsFilter() { - Query originQuery = this.originQuery(); - if (originQuery instanceof ConditionQuery) { - ConditionQuery originCQ = ((ConditionQuery) originQuery); - if (this.resultsFilter != null) { - originCQ.updateResultsFilter(this.resultsFilter); - } else { - originCQ.updateResultsFilter(); - } - } - } - - protected void updateResultsFilter(ResultsFilter filter) { - this.resultsFilter = filter; - Query originQuery = this.originQuery(); - if (originQuery instanceof ConditionQuery) { - ConditionQuery originCQ = ((ConditionQuery) originQuery); - originCQ.updateResultsFilter(filter); - } + ResultsFilter resultsFilter() { + return this.resultsFilter; } public ConditionQuery originConditionQuery() { diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/QueryBatch.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/QueryBatch.java new file mode 100644 index 0000000000..997375b04b --- /dev/null +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/QueryBatch.java @@ -0,0 +1,255 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hugegraph.backend.query; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.NoSuchElementException; +import java.util.function.BiPredicate; +import java.util.function.Function; + +import org.apache.hugegraph.HugeException; +import org.apache.hugegraph.backend.id.Id; +import org.apache.hugegraph.iterator.CIter; +import org.apache.hugegraph.iterator.Metadatable; +import org.apache.hugegraph.type.Idfiable; +import org.apache.hugegraph.util.InsertionOrderUtil; + +/** A single query's results, with exclusive ownership of its iterator chain. */ +public final class QueryBatch implements AutoCloseable { + + private final BatchIterator results; + private final QueryResultContext context; + + public QueryBatch(Iterator results, QueryResultContext context) { + this.context = context; + this.results = new BatchIterator() { + @Override + protected R fetch() { + return results.hasNext() ? results.next() : null; + } + + @Override + protected void closeResources() throws Exception { + closeAll(results); + } + + @Override + public Object metadata(String meta, Object... args) { + return metadataOf(results, meta, args); + } + }; + } + + public Iterator results() { + return this.results; + } + + public QueryResultContext context() { + return this.context; + } + + public QueryBatch map(Function mapper) { + return this.flatMap(value -> { + T mapped = mapper.apply(value); + return mapped == null ? Collections.emptyIterator() : + Collections.singleton(mapped).iterator(); + }); + } + + public QueryBatch flatMap(Function> mapper) { + Iterator origin = this.results; + return new QueryBatch<>(new BatchIterator() { + private Iterator child; + + @Override + protected T fetch() throws Exception { + while (true) { + if (this.child != null && this.child.hasNext()) { + T value = this.child.next(); + if (value != null) { + return value; + } + continue; + } + Iterator previous = this.child; + this.child = null; + closeAll(previous); + if (!origin.hasNext()) { + return null; + } + this.child = mapper.apply(origin.next()); + } + } + + @Override + protected void closeResources() throws Exception { + Iterator previous = this.child; + this.child = null; + closeAll(previous, origin); + } + + @Override + public Object metadata(String meta, Object... args) { + return metadataOf(origin, meta, args); + } + }, this.context); + } + + public QueryBatch filter(BiPredicate predicate) { + return this.map(value -> predicate.test(this.context, value) ? value : null); + } + + @SuppressWarnings("unchecked") + public QueryBatch keepInputOrder() { + if (!this.context.mustSortByInputIds()) { + return (QueryBatch) this; + } + List values = new ArrayList<>(); + QueryResults.fillList((Iterator) this.results, values); + List ids = this.context.inputIds(); + if (ids.size() <= 1) { + return new QueryBatch<>(values.iterator(), this.context); + } + Map byId = InsertionOrderUtil.newMap(); + for (T value : values) { + byId.put(value.id(), value); + Query.checkForceCapacity(byId.size()); + } + if (byId.size() > ids.size()) { + // A partial ID description cannot order every returned element. + return new QueryBatch<>(values.iterator(), this.context); + } + List ordered = new ArrayList<>(values.size()); + for (Id id : ids) { + T value = byId.remove(id); + if (value != null) { + ordered.add(value); + } + } + ordered.addAll(byId.values()); + return new QueryBatch<>(ordered.iterator(), this.context); + } + + @Override + public void close() throws Exception { + this.results.close(); + } + + /** Internal iterators use null as exhaustion, like the existing mappers. */ + public abstract static class BatchIterator implements CIter { + + private T current; + private boolean closed; + + protected abstract T fetch() throws Exception; + + protected abstract void closeResources() throws Exception; + + @Override + public final boolean hasNext() { + if (this.closed) { + return false; + } + if (this.current != null) { + return true; + } + try { + this.current = this.fetch(); + if (this.current == null) { + this.close(); + return false; + } + return true; + } catch (Throwable failure) { + try { + this.close(); + } catch (Throwable closing) { + if (closing != failure) { + failure.addSuppressed(closing); + } + } + throw propagate(failure); + } + } + + @Override + public final T next() { + if (!this.hasNext()) { + throw new NoSuchElementException(); + } + T value = this.current; + this.current = null; + return value; + } + + @Override + public final void close() throws Exception { + if (!this.closed) { + this.closed = true; + this.current = null; + this.closeResources(); + } + } + + @Override + public Object metadata(String meta, Object... args) { + return null; + } + } + + public static Object metadataOf(Object iterator, String meta, Object... args) { + return iterator instanceof Metadatable ? + ((Metadatable) iterator).metadata(meta, args) : null; + } + + public static void closeAll(Object... resources) throws Exception { + Throwable failure = null; + for (Object resource : resources) { + if (!(resource instanceof AutoCloseable)) { + continue; + } + try { + ((AutoCloseable) resource).close(); + } catch (Throwable closing) { + if (failure == null) { + failure = closing; + } else if (failure != closing) { + failure.addSuppressed(closing); + } + } + } + if (failure instanceof Exception) { + throw (Exception) failure; + } + if (failure != null) { + throw (Error) failure; + } + } + + public static RuntimeException propagate(Throwable failure) { + if (failure instanceof Error) { + throw (Error) failure; + } + return failure instanceof RuntimeException ? (RuntimeException) failure : + new HugeException("Failed to iterate query results", failure); + } +} diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/QueryResultContext.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/QueryResultContext.java new file mode 100644 index 0000000000..68200795a0 --- /dev/null +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/QueryResultContext.java @@ -0,0 +1,118 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hugegraph.backend.query; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +import org.apache.hugegraph.backend.id.Id; +import org.apache.hugegraph.backend.query.ConditionQuery.OptimizedType; +import org.apache.hugegraph.backend.query.ConditionQuery.ResultsFilter; + +/** Decisions captured for one batch; queries retain shared index cleanup data. */ +public final class QueryResultContext { + + private final List queries; + private final List inputIds; + private final boolean mustSortByInputIds; + private final ConditionQuery matchQuery; + private final ResultsFilter resultsFilter; + private final OptimizedType optimizedType; + private final boolean showExpired; + private final boolean showHidden; + private final boolean showDeleting; + + public QueryResultContext(Query query) { + this(query, false); + } + + public QueryResultContext(Query query, boolean inputOrderSatisfied) { + List chain = new ArrayList<>(); + ConditionQuery match = null; + ResultsFilter filter = null; + OptimizedType optimized = OptimizedType.NONE; + Query visibility = query; + for (Query current = query; current != null; current = current.originQuery()) { + chain.add(current); + visibility = current; + if (current instanceof ConditionQuery) { + ConditionQuery condition = (ConditionQuery) current; + if (optimized == OptimizedType.NONE) { + optimized = condition.optimized(); + } + if (filter == null) { + filter = condition.resultsFilter(); + } + if (current.resultType().isGraph()) { + match = condition; + } + } + } + this.queries = Collections.unmodifiableList(chain); + this.inputIds = Collections.unmodifiableList(new ArrayList<>(query.ids())); + this.mustSortByInputIds = !inputOrderSatisfied && query instanceof IdQuery && + ((IdQuery) query).mustSortByInput(); + this.matchQuery = match; + this.resultsFilter = filter; + this.optimizedType = optimized; + this.showExpired = visibility.showExpired(); + this.showHidden = visibility.showHidden(); + this.showDeleting = visibility.showDeleting(); + } + + public List queries() { + return this.queries; + } + + public List inputIds() { + return this.inputIds; + } + + public boolean mustSortByInputIds() { + return this.mustSortByInputIds; + } + + public ConditionQuery matchQuery() { + return this.matchQuery; + } + + public ResultsFilter resultsFilter() { + return this.resultsFilter; + } + + public OptimizedType optimizedType() { + return this.optimizedType; + } + + public boolean conditionFilterRequired() { + return this.optimizedType != OptimizedType.NONE; + } + + public boolean showExpired() { + return this.showExpired; + } + + public boolean showHidden() { + return this.showHidden; + } + + public boolean showDeleting() { + return this.showDeleting; + } +} diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/QueryResults.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/QueryResults.java index 01c2024e45..b149c91984 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/QueryResults.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/QueryResults.java @@ -18,234 +18,303 @@ package org.apache.hugegraph.backend.query; import java.util.ArrayList; -import java.util.Collection; import java.util.Collections; import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.NoSuchElementException; -import java.util.Set; +import java.util.function.BiPredicate; import java.util.function.Function; import org.apache.hugegraph.HugeException; import org.apache.hugegraph.backend.id.Id; +import org.apache.hugegraph.backend.query.QueryBatch.BatchIterator; import org.apache.hugegraph.iterator.CIter; -import org.apache.hugegraph.iterator.FlatMapperIterator; import org.apache.hugegraph.iterator.ListIterator; -import org.apache.hugegraph.iterator.MapperIterator; -import org.apache.hugegraph.iterator.WrappedIterator; import org.apache.hugegraph.perf.PerfUtil.Watched; import org.apache.hugegraph.type.Idfiable; -import org.apache.hugegraph.util.E; -import org.apache.hugegraph.util.InsertionOrderUtil; -import org.apache.tinkerpop.gremlin.structure.util.CloseableIterator; +/** A lazy stream of query batches. Only the final consumer flattens the stream. */ public class QueryResults { - private static final Iterator EMPTY_ITERATOR = new EmptyIterator<>(); - - private static final QueryResults EMPTY = new QueryResults<>( - emptyIterator(), Query.NONE); - - private final Iterator results; + private final Iterator> batches; private final List queries; - private List currentQueries; - private long queryVersion; + private final Object metadata; + private Iterator results; public QueryResults(Iterator results, Query query) { - this(results); - this.addQuery(query); + this(results, new QueryResultContext(query)); } - private QueryResults(Iterator results) { - this.results = results; - this.queries = InsertionOrderUtil.newList(); - this.currentQueries = Collections.emptyList(); - this.queryVersion = 0L; + public QueryResults(Iterator results, QueryResultContext context) { + QueryBatch batch = new QueryBatch<>(results, context); + this.batches = new BatchIterator>() { + private boolean fetched; + + @Override + protected QueryBatch fetch() { + if (this.fetched) { + return null; + } + this.fetched = true; + return batch; + } + + @Override + protected void closeResources() throws Exception { + batch.close(); + } + }; + this.queries = new ArrayList<>(Collections.singletonList(context.queries().get(0))); + this.metadata = batch.results(); } - public void setQuery(Query query) { - if (!this.queries.isEmpty()) { - this.queries.clear(); - } - this.addQuery(query); + private QueryResults(Iterator> batches, Object metadata) { + this.batches = batches; + this.metadata = metadata; + this.queries = new ArrayList<>(); } - private void addQuery(Query query) { - E.checkNotNull(query, "query"); - this.addQueries(Collections.singletonList(query)); + public static QueryResults fromBatches(Iterator> batches) { + return new QueryResults<>(batches, batches); } - private void addQueries(List queries) { - assert !queries.isEmpty(); - for (Query query : queries) { - E.checkNotNull(query, "query"); - this.queries.add(query); - } - this.currentQueries = new ArrayList<>(queries); - this.queryVersion++; + public Iterator> batches() { + return this.batches; } public Iterator iterator() { + if (this.results == null) { + this.results = new BatchIterator() { + private QueryBatch active; + + @Override + protected R fetch() throws Exception { + while (true) { + if (this.active != null && this.active.results().hasNext()) { + return this.active.results().next(); + } + QueryBatch previous = this.active; + this.active = null; + QueryBatch.closeAll(previous); + if (!batches.hasNext()) { + return null; + } + this.active = batches.next(); + queries.add(this.active.context().queries().get(0)); + } + } + + @Override + protected void closeResources() throws Exception { + QueryBatch.closeAll(this.active, batches); + } + + @Override + public Object metadata(String meta, Object... args) { + return QueryBatch.metadataOf(metadata, meta, args); + } + }; + } return this.results; } - public R one() { - return one(this.results); + public QueryResults mapBatches(Function, QueryBatch> mapper) { + Iterator> origin = this.batches; + return new QueryResults<>(new BatchIterator>() { + private QueryBatch active; + + @Override + protected QueryBatch fetch() throws Exception { + QueryBatch.closeAll(this.active); + this.active = null; + if (!origin.hasNext()) { + return null; + } + QueryBatch batch = origin.next(); + this.active = batch; + QueryBatch mapped = mapper.apply(batch); + this.active = mapped; + return mapped; + } + + @Override + protected void closeResources() throws Exception { + QueryBatch.closeAll(this.active, origin); + } + }, this.metadata); } - public QueryResults toList() { - QueryResults fetched = new QueryResults<>(toList(this.results)); - fetched.addQueries(this.queries); - return fetched; + public QueryResults map(Function mapper) { + return this.mapBatches(batch -> batch.map(mapper)); } - public List queries() { - return Collections.unmodifiableList(this.queries); + public QueryResults flatMap(Function> mapper) { + return this.mapBatches(batch -> batch.flatMap(mapper)); } - public Iterator keepInputOrderIfNeeded( - Iterator origin) { - if (!origin.hasNext()) { - // None result found - return origin; - } - if (!mustSortByInputIds(this.currentQueries)) { - return origin; - } - return new InputOrderIterator<>(this, origin); + public QueryResults filter(BiPredicate predicate) { + return this.mapBatches(batch -> batch.filter(predicate)); } - private static boolean mustSortByInputIds(List queries) { - assert !queries.isEmpty() : queries; - for (Query query : queries) { - if (query instanceof IdQuery && - ((IdQuery) query).mustSortByInput()) { - return true; - } - } - return false; + public QueryResults keepInputOrderIfNeeded() { + return this.mapBatches(QueryBatch::keepInputOrder); + } + + public R one() { + return one(this.iterator()); } - @SuppressWarnings("unused") - private boolean bigCapacity() { - assert !this.queries.isEmpty(); - for (Query query : this.queries) { - if (query.bigCapacity()) { - return true; + public List queries() { + return Collections.unmodifiableList(this.queries); + } + + public QueryResults toList() { + List> fetched = new ArrayList<>(); + long count = 0L; + Throwable failure = null; + try { + while (this.batches.hasNext()) { + QueryBatch batch = this.batches.next(); + ListIterator values = toList(batch.results()); + count += values.list().size(); + Query.checkForceCapacity(count); + fetched.add(new QueryBatch<>(values, batch.context())); + Query.checkForceCapacity(fetched.size()); } + } catch (Throwable e) { + failure = e; + throw QueryBatch.propagate(e); + } finally { + close(this.batches, failure); } - return false; + QueryResults result = new QueryResults<>(fetched.iterator(), this.metadata); + result.queries.addAll(this.queries); + return result; } - private static Collection queryIds(List queries) { - assert !queries.isEmpty(); - if (queries.size() == 1) { - return queries.get(0).ids(); - } + public static QueryResults flatMap( + Iterator inputs, Function> mapper) { + return fromBatches(new BatchIterator>() { + private QueryResults child; + + @Override + protected QueryBatch fetch() throws Exception { + while (true) { + if (this.child != null && this.child.batches.hasNext()) { + return this.child.batches.next(); + } + QueryResults previous = this.child; + this.child = null; + QueryBatch.closeAll(previous == null ? null : previous.batches); + if (!inputs.hasNext()) { + return null; + } + this.child = mapper.apply(inputs.next()); + } + } - Set ids = InsertionOrderUtil.newSet(); - for (Query query : queries) { - ids.addAll(query.ids()); - } - return ids; + @Override + protected void closeResources() throws Exception { + QueryBatch.closeAll(this.child == null ? null : this.child.batches, inputs); + } + }); } @Watched public static ListIterator toList(Iterator iterator) { + Throwable failure = null; try { return new ListIterator<>(Query.DEFAULT_CAPACITY, iterator); + } catch (Throwable e) { + failure = e; + throw QueryBatch.propagate(e); } finally { - CloseableIterator.closeIterator(iterator); + close(iterator, failure); } } @Watched public static void fillList(Iterator iterator, List list) { + Throwable failure = null; try { while (iterator.hasNext()) { - T result = iterator.next(); - list.add(result); + list.add(iterator.next()); Query.checkForceCapacity(list.size()); } + } catch (Throwable e) { + failure = e; + throw QueryBatch.propagate(e); } finally { - CloseableIterator.closeIterator(iterator); + close(iterator, failure); } } @Watched - public static void fillMap(Iterator iterator, - Map map) { + public static void fillMap(Iterator iterator, Map map) { + Throwable failure = null; try { while (iterator.hasNext()) { - T result = iterator.next(); - assert result.id() != null; - map.put(result.id(), result); + T value = iterator.next(); + map.put(value.id(), value); Query.checkForceCapacity(map.size()); } + } catch (Throwable e) { + failure = e; + throw QueryBatch.propagate(e); } finally { - CloseableIterator.closeIterator(iterator); + close(iterator, failure); } } - public static QueryResults flatMap( - Iterator iterator, Function> func) { - @SuppressWarnings("unchecked") - QueryResults[] qr = new QueryResults[1]; - qr[0] = new QueryResults<>(new FlatMapperIterator<>(iterator, i -> { - QueryResults results = func.apply(i); - if (results == null || !results.iterator().hasNext()) { - return null; - } - return new QueryTrackingIterator<>(qr[0], results); - })); - return qr[0]; - } - - private long queryVersion() { - return this.queryVersion; - } - - private List currentQueries() { - return new ArrayList<>(this.currentQueries); - } - @Watched public static T one(Iterator iterator) { + Throwable failure = null; try { if (iterator.hasNext()) { - T result = iterator.next(); + T value = iterator.next(); if (iterator.hasNext()) { - throw new HugeException("Expect just one result, " + - "but got at least two: [%s, %s]", - result, iterator.next()); + throw new HugeException("Expect just one result, but got at least two: [%s, %s]", + value, iterator.next()); } - return result; + return value; } + return null; + } catch (Throwable e) { + failure = e; + throw QueryBatch.propagate(e); } finally { - CloseableIterator.closeIterator(iterator); + close(iterator, failure); + } + } + + private static void close(Object iterator, Throwable failure) { + try { + QueryBatch.closeAll(iterator); + } catch (Throwable closing) { + if (failure == null) { + throw QueryBatch.propagate(closing); + } + if (closing != failure) { + failure.addSuppressed(closing); + } } - return null; } - public static Iterator iterator(T elem) { - return new OneIterator<>(elem); + public static Iterator iterator(T value) { + return new OneIterator<>(value); } - @SuppressWarnings("unchecked") public static QueryResults empty() { - return (QueryResults) EMPTY; + return new QueryResults(QueryResults.emptyIterator(), Query.NONE); } - @SuppressWarnings("unchecked") public static Iterator emptyIterator() { - return (Iterator) EMPTY_ITERATOR; + return new EmptyIterator<>(); } public interface Fetcher extends Function> { - } - private static class EmptyIterator implements CIter { @Override @@ -304,118 +373,4 @@ public void close() throws Exception { } } - private static class QueryTrackingIterator - extends WrappedIterator { - - private final QueryResults parent; - private final QueryResults child; - private long childQueryVersion; - - public QueryTrackingIterator(QueryResults parent, - QueryResults child) { - this.parent = parent; - this.child = child; - this.childQueryVersion = -1L; - } - - @Override - protected Iterator originIterator() { - return this.child.iterator(); - } - - @Override - protected boolean fetch() { - Iterator origin = this.child.iterator(); - if (!origin.hasNext()) { - return false; - } - R result = origin.next(); - long queryVersion = this.child.queryVersion(); - if (this.childQueryVersion != queryVersion) { - this.parent.addQueries(this.child.currentQueries()); - this.childQueryVersion = queryVersion; - } - assert this.current == none(); - this.current = result; - return true; - } - } - - private static class InputOrderIterator - extends WrappedIterator { - - private final QueryResults queryResults; - private final Iterator origin; - private Iterator currentBatch; - - public InputOrderIterator(QueryResults queryResults, - Iterator origin) { - this.queryResults = queryResults; - this.origin = origin; - this.currentBatch = Collections.emptyIterator(); - } - - @Override - protected Iterator originIterator() { - return this.origin; - } - - @Override - protected boolean fetch() { - while (true) { - if (this.currentBatch.hasNext()) { - assert this.current == none(); - this.current = this.currentBatch.next(); - return true; - } - if (!this.origin.hasNext()) { - return false; - } - this.currentBatch = this.fetchBatch(); - } - } - - private Iterator fetchBatch() { - long queryVersion = this.queryResults.queryVersion(); - List queries = this.queryResults.currentQueries(); - List results = InsertionOrderUtil.newList(); - do { - results.add(this.origin.next()); - Query.checkForceCapacity(results.size()); - } while (this.origin.hasNext() && - queryVersion == this.queryResults.queryVersion()); - - if (!mustSortByInputIds(queries)) { - return results.iterator(); - } - Collection ids = queryIds(queries); - if (ids.size() <= 1) { - return results.iterator(); - } - - Map byId = InsertionOrderUtil.newMap(); - for (T result : results) { - assert result.id() != null; - byId.put(result.id(), result); - } - if (byId.size() > ids.size()) { - /* - * The current query only describes part of this segment. - * Preserve backend order because it can't fully define the - * order of every returned result. - */ - return results.iterator(); - } - - List ordered = new ArrayList<>(results.size()); - for (Id id : ids) { - T result = byId.remove(id); - if (result != null) { - ordered.add(result); - } - } - ordered.addAll(byId.values()); - return ordered.iterator(); - } - } } diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/tx/GraphTransaction.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/tx/GraphTransaction.java index d47b2825c9..3c2879c3c0 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/tx/GraphTransaction.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/tx/GraphTransaction.java @@ -32,7 +32,6 @@ import java.util.function.Consumer; import java.util.function.Function; import java.util.stream.Collectors; -import java.util.stream.Stream; import org.apache.commons.collections.CollectionUtils; import org.apache.hugegraph.HugeException; @@ -45,14 +44,16 @@ import org.apache.hugegraph.backend.page.IdHolderList; import org.apache.hugegraph.backend.page.PageInfo; import org.apache.hugegraph.backend.page.QueryList; -import org.apache.hugegraph.backend.query.Aggregate; import org.apache.hugegraph.backend.query.Aggregate.AggregateFunc; +import org.apache.hugegraph.backend.query.Aggregate; import org.apache.hugegraph.backend.query.Condition; -import org.apache.hugegraph.backend.query.ConditionQuery; import org.apache.hugegraph.backend.query.ConditionQuery.OptimizedType; +import org.apache.hugegraph.backend.query.ConditionQuery; import org.apache.hugegraph.backend.query.ConditionQueryFlatten; import org.apache.hugegraph.backend.query.IdQuery; import org.apache.hugegraph.backend.query.Query; +import org.apache.hugegraph.backend.query.QueryBatch; +import org.apache.hugegraph.backend.query.QueryResultContext; import org.apache.hugegraph.backend.query.QueryResults; import org.apache.hugegraph.backend.store.BackendEntry; import org.apache.hugegraph.backend.store.BackendMutation; @@ -853,26 +854,38 @@ public Iterator queryVertices(Query query) { } protected Iterator queryVerticesFromBackend(Query query) { - assert query.resultType().isVertex(); + return this.queryVertexBatchesFromBackend(query).iterator(); + } - QueryResults results = this.query(query); - Iterator entries = results.iterator(); + private QueryResults queryVertexBatchesFromBackend(Query query) { + assert query.resultType().isVertex(); + if (!(query instanceof ConditionQuery)) { + return this.processBatches(this.fetchVertexBatch(query)); + } + QueryList queries = this.optimizeQueries(query, this::fetchVertexBatch); + return this.processBatches(queries.empty() ? QueryResults.empty() : + queries.fetch(this.pageSize)); + } - Iterator vertices = new MapperIterator<>(entries, - this::parseEntry); - vertices = this.filterExpiredResultFromBackend(query, vertices); - vertices = this.filterUnmatchedRecords(vertices, query); + private QueryResults backendBatches(Query query) { + QueryResults results = super.query(query); + QueryResultContext context = new QueryResultContext( + query, this.storeFeatures().supportsQuerySortByInputIds()); + return results.mapBatches(batch -> new QueryBatch<>(batch.results(), context)); + } - if (!this.store().features().supportsQuerySortByInputIds()) { - // There is no id in BackendEntry, so sort after deserialization - vertices = results.keepInputOrderIfNeeded(vertices); - } - return vertices; + protected QueryResults fetchVertexBatch(Query query) { + return this.filterExpiredBatches(this.backendBatches(query).map(this::parseEntry)); } private Iterator queryValidVerticesFromBackend(Query query) { - Iterator results = this.queryVerticesFromBackend(query); - return this.filterInvalidRecords(results, query); + return this.queryVertexBatchesFromBackend(query) + .filter(this::filterInvalidRecord).iterator(); + } + + private QueryResults processBatches(QueryResults batches) { + batches = batches.filter(this::filterUnmatchedRecord); + return batches.keepInputOrderIfNeeded(); } @Watched(prefix = "graph") @@ -1051,87 +1064,77 @@ public Iterator queryEdges(Query query) { } protected Iterator queryEdgesFromBackend(Query query) { - assert query.resultType().isEdge(); + return this.queryEdgeBatchesFromBackend(query).iterator(); + } + + protected QueryResults queryEdgesFromMemory(Query query) { + return null; + } + private QueryResults queryEdgeBatchesFromBackend(Query query) { + assert query.resultType().isEdge(); + QueryResults memory = this.queryEdgesFromMemory(query); + if (memory != null) { + return memory; + } if (query instanceof ConditionQuery && !query.paging()) { - // TODO: support: paging + parent label boolean supportIn = this.storeFeatures().supportsQueryWithInCondition(); - // consider multi labels + properties, - // see org.apache.hugegraph.core.EdgeCoreTest.testQueryInEdgesOfVertexByLabels - Stream flattenedQueries = - ConditionQueryFlatten.flatten((ConditionQuery) query, supportIn).stream(); - - Stream> edgeIterators = flattenedQueries.map(cq -> { + List flattened = ConditionQueryFlatten.flatten( + (ConditionQuery) query, supportIn); + Function> fetcher = cq -> { Id label = cq.condition(HugeKeys.LABEL); if (this.storeFeatures().supportsFatherAndSubEdgeLabel() && - label != null && - graph().edgeLabel(label).isFather() && + label != null && graph().edgeLabel(label).isFather() && cq.condition(HugeKeys.SUB_LABEL) == null && cq.condition(HugeKeys.OWNER_VERTEX) != null && cq.condition(HugeKeys.DIRECTION) != null && matchEdgeSortKeys(cq, false, this.graph())) { - // g.V("V.id").outE("parentLabel").has("sortKey","value") - return parentElQueryWithSortKeys( - graph().edgeLabel(label), graph().edgeLabels(), cq); - } else { - return queryEdgesFromBackendInternal(cq); + EdgeLabel parent = graph().edgeLabel(label); + Iterator children = graph().edgeLabels().stream() + .filter(el -> el.edgeLabelType().sub() && + el.fatherId().equals(parent.id())).iterator(); + return QueryResults.flatMap(children, child -> { + ConditionQuery subQuery = cq.copy(); + subQuery.eq(HugeKeys.SUB_LABEL, child.id()); + return this.queryEdgeBatchesFromBackend(subQuery); + }); } - }); - - return edgeIterators.reduce(ExtendableIterator::concat) - .orElse(Collections.emptyIterator()); + return this.queryEdgeBatchesFromBackendInternal(cq); + }; + // Preserve immediate validation for a single query without activating a sibling. + if (flattened.size() == 1) { + return fetcher.apply(flattened.get(0)); + } + return QueryResults.flatMap(flattened.iterator(), fetcher); } - - return queryEdgesFromBackendInternal(query); + return this.queryEdgeBatchesFromBackendInternal(query); } private Iterator queryValidEdgesFromBackend(Query query) { - Iterator results = this.queryEdgesFromBackend(query); - return this.filterInvalidRecords(results, query); + return this.queryEdgeBatchesFromBackend(query) + .filter(this::filterInvalidRecord).iterator(); } - private Iterator queryEdgesFromBackendInternal(Query query) { - assert query.resultType().isEdge(); - - QueryResults results = this.query(query); - Iterator entries = results.iterator(); + private QueryResults queryEdgeBatchesFromBackendInternal(Query query) { + if (!(query instanceof ConditionQuery)) { + return this.processBatches(this.fetchEdgeBatch(query)); + } + QueryList queries = this.optimizeQueries(query, this::fetchEdgeBatch); + return this.processBatches(queries.empty() ? QueryResults.empty() : + queries.fetch(this.pageSize)); + } - Iterator edges = new FlatMapperIterator<>(entries, entry -> { - // Edges are in a vertex + protected QueryResults fetchEdgeBatch(Query query) { + QueryResults edges = this.backendBatches(query).flatMap(entry -> { HugeVertex vertex = this.parseEntry(entry); if (vertex == null) { return null; } assert query.idsSize() != 1 || vertex.getEdges().size() == 1; - /* - * Copy to avoid ConcurrentModificationException when removing edge - * because HugeEdge.remove() will update edges in owner vertex - */ - return new ListIterator<>(ImmutableList.copyOf(vertex.getEdges())); + // Removing an edge may change its owner's collection during iteration. + return ImmutableList.copyOf(vertex.getEdges()).iterator(); }); - - edges = this.filterExpiredResultFromBackend(query, edges); - edges = this.filterUnmatchedRecords(edges, query); - - if (!this.store().features().supportsQuerySortByInputIds()) { - // There is no id in BackendEntry, so sort after deserialization - edges = results.keepInputOrderIfNeeded(edges); - } - return edges; - } - - private Iterator parentElQueryWithSortKeys(EdgeLabel label, - Collection allEls, - ConditionQuery cq) { - return allEls.stream() - .filter(el -> el.edgeLabelType().sub() && el.fatherId().equals(label.id())) - .map(el -> { - ConditionQuery tempQuery = cq.copy(); - tempQuery.eq(HugeKeys.SUB_LABEL, el.id()); - return this.queryEdgesFromBackend(tempQuery); - }) - .reduce(Iterators::concat) - .orElse(Collections.emptyIterator()); + return this.filterExpiredBatches(edges); } @Watched(prefix = "graph") @@ -1882,56 +1885,29 @@ private void removeLeftIndexIfNeeded(Map vertices) { } } - private Iterator filterInvalidRecords( - Iterator results, - Query query) { - // Filter unused records - return new FilterIterator<>(results, elem -> { - warnLeftRecord(elem); - return !invalidRecord(elem, query); - }); + private boolean filterInvalidRecord(QueryResultContext context, HugeElement elem) { + warnLeftRecord(elem); + return !invalidRecord(elem, context); } - private Iterator filterUnmatchedRecords( - Iterator results, - Query query) { - /* - * Filter against the current index sub-query before restoring input - * order, since the order iterator may prefetch the next sub-query and - * update the results filter of the origin query. - * FilterIterator tests and buffers each element before its upstream - * iterator is advanced again, so the element is always tested with - * the results filter of the sub-query that produced it. - */ - return new FilterIterator<>(results, elem -> { - /* - * Preserve the original predicate order: hidden records and - * records of deleting labels must be handled by the downstream - * invalid-record filter without triggering left-index cleanup. - */ - if (invalidRecord(elem, query)) { - return true; - } - // Process results that query from left index or primary-key - // Only index query will come here - boolean matched = - query.resultType().isVertex() != elem.type().isVertex() || - rightResultFromIndexQuery(query, elem); - if (!matched) { - warnLeftRecord(elem); - } - return matched; - }); + private boolean filterUnmatchedRecord(QueryResultContext context, HugeElement elem) { + // Invisible records pass to the public visibility stage, without index cleanup. + if (invalidRecord(elem, context)) { + return true; + } + boolean matched = this.rightResultFromIndexQuery(context, elem); + if (!matched) { + // Rejected records never reach the public stage; raw callers keep this diagnostic too. + warnLeftRecord(elem); + } + return matched; } - private static boolean invalidRecord(HugeElement elem, Query query) { - // Filter hidden results - if (!query.showHidden() && Graph.Hidden.isHidden(elem.label())) { + private static boolean invalidRecord(HugeElement elem, QueryResultContext context) { + if (!context.showHidden() && Graph.Hidden.isHidden(elem.label())) { return true; } - // Filter vertices/edges of deleting label - return elem.schemaLabel().status().deleting() && - !query.showDeleting(); + return elem.schemaLabel().status().deleting() && !context.showDeleting(); } private static void warnLeftRecord(HugeElement elem) { @@ -1943,26 +1919,17 @@ private static void warnLeftRecord(HugeElement elem) { } } - private boolean rightResultFromIndexQuery(Query query, HugeElement elem) { - /* - * If query is ConditionQuery or query.originQuery() is ConditionQuery - * means it's index query - */ - if (!(query instanceof ConditionQuery)) { - if (query.originQuery() instanceof ConditionQuery) { - query = query.originQuery(); - } else { - return true; - } + private boolean rightResultFromIndexQuery(QueryResultContext context, HugeElement elem) { + ConditionQuery cq = context.matchQuery(); + if (cq == null || cq.resultType().isVertex() != elem.type().isVertex()) { + return true; } - - ConditionQuery cq = (ConditionQuery) query; if (cq.condition(HugeKeys.LABEL) != null && cq.resultType().isEdge()) { if (cq.conditions().size() == 1) { // g.E().hasLabel(xxx) return true; } - if (cq.optimized() == OptimizedType.INDEX) { + if (context.optimizedType() == OptimizedType.INDEX) { // g.E().hasLabel(xxx).has(yyy) // consider OptimizedType.INDEX_FILTER occurred in org.apache.hugegraph.core // .EdgeCoreTest.testQueryCount @@ -1976,7 +1943,7 @@ private boolean rightResultFromIndexQuery(Query query, HugeElement elem) { } } - if (!conditionQueryNeedsPostFilter(cq) || cq.test(elem)) { + if (!context.conditionFilterRequired() || cq.test(elem, context.resultsFilter())) { if (cq.existLeftIndex(elem.id())) { /* * Both have correct and left index, wo should return true @@ -1992,13 +1959,13 @@ private boolean rightResultFromIndexQuery(Query query, HugeElement elem) { /* Return true if: * 1.not query by index or by primary-key/sort-key - * (cq.optimized() == 0 means query just by sysprop) + * (context.optimizedType() == 0 means query just by sysprop) * 2.the result match all conditions */ return true; } - if (cq.optimized() == OptimizedType.INDEX) { + if (context.optimizedType() == OptimizedType.INDEX) { try { this.indexTx.asyncRemoveIndexLeft(cq, elem); } catch (Throwable e) { @@ -2009,45 +1976,13 @@ private boolean rightResultFromIndexQuery(Query query, HugeElement elem) { return false; } - protected static boolean queryNeedsPostFilter(Query query) { - while (query != null) { - if (query instanceof ConditionQuery) { - ConditionQuery cq = (ConditionQuery) query; - /* - * Search conditions need post-filtering even before query - * optimization marks the query with an optimized type. - */ - boolean edgeIndexWithLabel = - cq.resultType().isEdge() && - cq.optimized() == OptimizedType.INDEX && - cq.condition(HugeKeys.LABEL) != null; - if (cq.hasSearchCondition() || - (conditionQueryNeedsPostFilter(cq) && - !edgeIndexWithLabel)) { - return true; - } - } - query = query.originQuery(); - } - return false; - } - - private static boolean conditionQueryNeedsPostFilter(ConditionQuery query) { - return query.optimized() != OptimizedType.NONE; - } - - private Iterator filterExpiredResultFromBackend( - Query query, Iterator results) { - if (this.store().features().supportsTtl() || query.showExpired()) { - return results; - } - // Filter expired vertices/edges with TTL - return new FilterIterator<>(results, elem -> { - if (elem.expired()) { - DeleteExpiredJob.asyncDeleteExpiredObject(this.graph(), elem); - return false; + protected QueryResults filterExpiredBatches(QueryResults batches) { + return batches.filter((context, elem) -> { + if (this.storeFeatures().supportsTtl() || context.showExpired() || !elem.expired()) { + return true; } - return true; + DeleteExpiredJob.asyncDeleteExpiredObject(this.graph(), elem); + return false; }); } diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/backend/page/QueryListTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/backend/page/QueryListTest.java new file mode 100644 index 0000000000..43201dd803 --- /dev/null +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/backend/page/QueryListTest.java @@ -0,0 +1,231 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hugegraph.backend.page; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.Iterator; +import java.util.List; +import java.util.Set; + +import org.apache.hugegraph.backend.id.Id; +import org.apache.hugegraph.backend.id.IdGenerator; +import org.apache.hugegraph.backend.page.IdHolder.BatchIdHolder; +import org.apache.hugegraph.backend.page.IdHolder.FixedIdHolder; +import org.apache.hugegraph.backend.page.IdHolder.PagingIdHolder; +import org.apache.hugegraph.backend.query.ConditionQuery.OptimizedType; +import org.apache.hugegraph.backend.query.ConditionQuery; +import org.apache.hugegraph.backend.query.QueryResults; +import org.apache.hugegraph.backend.serializer.TextBackendEntry; +import org.apache.hugegraph.backend.store.BackendEntry; +import org.apache.hugegraph.testutil.Assert; +import org.apache.hugegraph.type.HugeType; +import org.apache.hugegraph.type.Idfiable; +import org.apache.hugegraph.util.InsertionOrderUtil; +import org.junit.Test; + +public class QueryListTest { + + @Test + public void testOrderingDoesNotFetchNextIndexPage() { + ConditionQuery query = new ConditionQuery(HugeType.VERTEX); + query.page(""); + query.limit(4); + int[] pageFetches = {0}; + int[] backendFetches = {0}; + IdHolderList holders = new IdHolderList(true); + holders.add(new PagingIdHolder(query, page -> { + Assert.assertEquals(pageFetches[0] == 0 ? "" : + new PageState(new byte[]{1}, 0, 2).toString(), page.page()); + Assert.assertEquals(2L, page.limit()); + int offset = pageFetches[0]++ * 2; + Set ids = InsertionOrderUtil.newSet(); + ids.add(IdGenerator.of(offset + 2L)); + ids.add(IdGenerator.of(offset + 1L)); + return new PageIds(ids, new PageState( + pageFetches[0] == 1 ? new byte[]{1} : new byte[0], 0, 2)); + }, true)); + QueryList queries = new QueryList<>(query, batch -> { + backendFetches[0]++; + List items = new ArrayList<>(); + batch.ids().stream().sorted().forEach(id -> items.add(new Item(id))); + return new QueryResults<>(items.iterator(), batch); + }); + queries.add(holders, 2); + QueryResults results = queries.fetch(2); + Iterator ordered = results.keepInputOrderIfNeeded().iterator(); + Assert.assertEquals(IdGenerator.of(2L), ordered.next().id()); + Assert.assertEquals(IdGenerator.of(1L), ordered.next().id()); + Assert.assertEquals(1, pageFetches[0]); + Assert.assertEquals(1, backendFetches[0]); + PageInfo nextPage = PageInfo.fromString(PageInfo.pageInfo(ordered)); + Assert.assertEquals(0, nextPage.offset()); + Assert.assertEquals(new PageState(new byte[]{1}, 0, 2).toString(), nextPage.page()); + Assert.assertEquals(1, pageFetches[0]); + Assert.assertEquals(IdGenerator.of(4L), ordered.next().id()); + Assert.assertEquals(IdGenerator.of(3L), ordered.next().id()); + Assert.assertFalse(ordered.hasNext()); + Assert.assertEquals(2, pageFetches[0]); + Assert.assertEquals(2, backendFetches[0]); + Assert.assertNull(PageInfo.pageInfo(ordered)); + } + + @Test + public void testLimitStopsAtPageBoundaryAndRetainsCursor() { + ConditionQuery query = new ConditionQuery(HugeType.VERTEX); + query.page(""); + query.limit(1L); + int[] fetches = {0}; + PageState next = new PageState(new byte[]{7}, 0, 1); + IdHolderList holders = new IdHolderList(true); + holders.add(new PagingIdHolder(query, page -> { + Assert.assertEquals("", page.page()); + Assert.assertEquals(1L, page.limit()); + fetches[0]++; + return new PageIds(ids(1L), next); + })); + QueryList list = new QueryList<>(query, batch -> + new QueryResults<>(Collections.singletonList(new Item(IdGenerator.of(1L))) + .iterator(), batch)); + list.add(holders, 2L); + Iterator values = list.fetch(2).iterator(); + Assert.assertEquals(IdGenerator.of(1L), values.next().id()); + Assert.assertFalse(values.hasNext()); + Assert.assertEquals(1, fetches[0]); + Assert.assertEquals(next.toString(), PageInfo.fromString(PageInfo.pageInfo(values)).page()); + } + + @Test + public void testEmptyHolderAdvancesToNextHolder() { + ConditionQuery query = new ConditionQuery(HugeType.VERTEX); + query.page(""); + query.limit(2L); + int[] fetches = {0}; + IdHolderList holders = new IdHolderList(true); + holders.add(new PagingIdHolder(query, page -> PageIds.EMPTY)); + holders.add(new PagingIdHolder(query, page -> { + Assert.assertEquals("", page.page()); + fetches[0]++; + return new PageIds(ids(1L), PageState.EMPTY); + })); + QueryList list = new QueryList<>(query, batch -> + new QueryResults<>(Collections.singletonList(new Item(IdGenerator.of(1L))) + .iterator(), batch)); + list.add(holders, 2L); + Iterator values = list.fetch(2).iterator(); + Assert.assertEquals(IdGenerator.of(1L), values.next().id()); + Assert.assertFalse(values.hasNext()); + Assert.assertNull(PageInfo.pageInfo(values)); + Assert.assertEquals(1, fetches[0]); + } + + @Test + public void testCopiedSearchFiltersAndEmptyHoldersStayLocal() { + ConditionQuery root = new ConditionQuery(HugeType.VERTEX); + root.optimized(OptimizedType.INDEX_FILTER); + ConditionQuery first = root.copy(); + ConditionQuery second = root.copy(); + ConditionQuery.ResultsFilter firstFilter = element -> true; + ConditionQuery.ResultsFilter secondFilter = element -> false; + first.registerResultsFilter(firstFilter); + second.registerResultsFilter(secondFilter); + IdHolderList holders = new IdHolderList(false); + holders.add(new FixedIdHolder(first.copy(), ids(2L, 1L))); + holders.add(new FixedIdHolder(first.copy(), Collections.emptySet())); + holders.add(new FixedIdHolder(second.copy(), ids(4L, 3L))); + int[] fetched = {0}; + QueryList list = new QueryList<>(root, query -> { + Assert.assertEquals(HugeType.VERTEX, query.resultType()); + fetched[0]++; + List values = new ArrayList<>(); + query.ids().forEach(id -> values.add(new Item(id))); + return new QueryResults<>(values.iterator(), query); + }); + list.add(holders, 2L); + List accepted = new ArrayList<>(); + QueryResults results = list.fetch(2).filter((context, item) -> { + boolean firstBatch = item.id().asLong() <= 2L; + Assert.assertSame(firstBatch ? firstFilter : secondFilter, context.resultsFilter()); + Assert.assertEquals(firstBatch ? 1 : 2, fetched[0]); + Assert.assertSame(root, context.matchQuery()); + return item.id().asLong() % 2 == 0; + }); + results.iterator().forEachRemaining(item -> accepted.add(item.id())); + Assert.assertEquals(Arrays.asList(IdGenerator.of(2L), IdGenerator.of(4L)), accepted); + Assert.assertEquals(2, fetched[0]); + } + + @Test + public void testMultipleIndexHoldersFetchOneIdBatchAtATime() { + ConditionQuery root = new ConditionQuery(HugeType.VERTEX); + IdHolderList holders = new IdHolderList(false); + int[] idFetches = {0}; + for (long start : new long[]{1L, 5L}) { + List entries = new ArrayList<>(); + for (long id = start; id < start + 4L; id++) { + entries.add(new TextBackendEntry(HugeType.VERTEX, IdGenerator.of(id))); + } + Iterator source = entries.iterator(); + ConditionQuery index = new ConditionQuery(HugeType.SECONDARY_INDEX, root); + holders.add(new BatchIdHolder(index, source, size -> { + idFetches[0]++; + Set ids = InsertionOrderUtil.newSet(); + while (ids.size() < size && source.hasNext()) { + ids.add(source.next().id()); + } + return ids; + }, true)); + } + QueryList list = new QueryList<>(root, query -> { + Assert.assertEquals(HugeType.VERTEX, query.resultType()); + List values = new ArrayList<>(); + query.ids().forEach(id -> values.add(0, new Item(id))); + return new QueryResults<>(values.iterator(), query); + }); + list.add(holders, 2L); + Iterator values = list.fetch(2).keepInputOrderIfNeeded().iterator(); + for (long id = 1L; id <= 8L; id++) { + Assert.assertEquals(IdGenerator.of(id), values.next().id()); + Assert.assertEquals((int) ((id + 1L) / 2L), idFetches[0]); + } + Assert.assertFalse(values.hasNext()); + } + + private static Set ids(Long... values) { + Set ids = InsertionOrderUtil.newSet(); + for (long value : values) { + ids.add(IdGenerator.of(value)); + } + return ids; + } + + private static final class Item implements Idfiable { + + private final Id id; + + private Item(Id id) { + this.id = id; + } + + @Override + public Id id() { + return this.id; + } + } +} diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/backend/tx/GraphTransactionTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/backend/tx/GraphTransactionTest.java index 85b3c0e76a..cb50197614 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/backend/tx/GraphTransactionTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/backend/tx/GraphTransactionTest.java @@ -17,74 +17,64 @@ package org.apache.hugegraph.backend.tx; -import java.util.Collections; - -import org.apache.hugegraph.backend.id.Id; import org.apache.hugegraph.backend.id.IdGenerator; -import org.apache.hugegraph.backend.query.Condition; -import org.apache.hugegraph.backend.query.ConditionQuery; import org.apache.hugegraph.backend.query.ConditionQuery.OptimizedType; +import org.apache.hugegraph.backend.query.ConditionQuery; import org.apache.hugegraph.backend.query.IdQuery; -import org.apache.hugegraph.backend.query.Query; +import org.apache.hugegraph.backend.query.QueryResultContext; import org.apache.hugegraph.testutil.Assert; import org.apache.hugegraph.type.HugeType; -import org.apache.hugegraph.type.define.HugeKeys; import org.junit.Test; public class GraphTransactionTest { @Test - public void testQueryNeedsPostFilter() { - Id key = IdGenerator.of(1); - ConditionQuery search = new ConditionQuery(HugeType.EDGE); - search.query(Condition.textContains(key, "word")); - - Assert.assertTrue(GraphTransaction.queryNeedsPostFilter(search)); - IdQuery searchIds = new IdQuery(search, IdGenerator.of(2)); - Assert.assertTrue(GraphTransaction.queryNeedsPostFilter(searchIds)); - - ConditionQuery searchAny = new ConditionQuery(HugeType.EDGE); - searchAny.query(Condition.textContainsAny( - key, Collections.singleton("word"))); - Assert.assertTrue(GraphTransaction.queryNeedsPostFilter(searchAny)); - - ConditionQuery exact = new ConditionQuery(HugeType.EDGE); - exact.query(Condition.eq(key, "word")); - Assert.assertFalse(GraphTransaction.queryNeedsPostFilter(exact)); - exact.optimized(OptimizedType.INDEX_FILTER); - Assert.assertTrue(GraphTransaction.queryNeedsPostFilter(exact)); - - ConditionQuery index = new ConditionQuery(HugeType.EDGE); - index.query(Condition.eq(key, "word")); - index.optimized(OptimizedType.INDEX); - Assert.assertTrue(GraphTransaction.queryNeedsPostFilter(index)); - - ConditionQuery labelIndex = new ConditionQuery(HugeType.EDGE); - labelIndex.query(Condition.eq(HugeKeys.LABEL, IdGenerator.of(2))); - labelIndex.query(Condition.eq(key, "word")); - labelIndex.optimized(OptimizedType.INDEX); - Assert.assertFalse(GraphTransaction.queryNeedsPostFilter(labelIndex)); - IdQuery labelIndexIds = new IdQuery(labelIndex, IdGenerator.of(2)); - Assert.assertFalse(GraphTransaction.queryNeedsPostFilter(labelIndexIds)); - - ConditionQuery vertexLabelIndex = - new ConditionQuery(HugeType.VERTEX); - vertexLabelIndex.query(Condition.eq(HugeKeys.LABEL, - IdGenerator.of(2))); - vertexLabelIndex.query(Condition.eq(key, "word")); - vertexLabelIndex.optimized(OptimizedType.INDEX); - Assert.assertTrue(GraphTransaction.queryNeedsPostFilter( - vertexLabelIndex)); + public void testBatchDecisionsRemainFixedAfterOriginChanges() { + ConditionQuery root = new ConditionQuery(HugeType.VERTEX); + root.optimized(OptimizedType.PRIMARY_KEY); + root.showHidden(true); + root.showDeleting(true); + root.showExpired(true); + IdQuery query = new IdQuery(root, IdGenerator.of(1L)); + QueryResultContext context = new QueryResultContext(query); + root.optimized(OptimizedType.INDEX_FILTER); + root.showHidden(false); + root.showDeleting(false); + root.showExpired(false); + query.resetIds(); + query.mustSortByInput(false); + Assert.assertEquals(OptimizedType.PRIMARY_KEY, context.optimizedType()); + Assert.assertTrue(context.conditionFilterRequired()); + Assert.assertTrue(context.showHidden()); + Assert.assertTrue(context.showDeleting()); + Assert.assertTrue(context.showExpired()); + Assert.assertTrue(context.mustSortByInputIds()); + Assert.assertEquals(IdGenerator.of(1L), context.inputIds().get(0)); + Assert.assertEquals(1, context.inputIds().size()); + Assert.assertSame(root, context.matchQuery()); + } - ConditionQuery primaryKey = new ConditionQuery(HugeType.VERTEX); - primaryKey.optimized(OptimizedType.PRIMARY_KEY); - Assert.assertTrue(GraphTransaction.queryNeedsPostFilter(primaryKey)); + @Test + public void testSiblingOptimizationDoesNotReplaceBatchDecision() { + ConditionQuery root = new ConditionQuery(HugeType.EDGE); + ConditionQuery first = root.copy(); + first.optimized(OptimizedType.INDEX_FILTER); + ConditionQuery second = root.copy(); + second.optimized(OptimizedType.INDEX); + Assert.assertEquals(OptimizedType.INDEX_FILTER, root.optimized()); + QueryResultContext context = new QueryResultContext( + new IdQuery(second, IdGenerator.of(1L))); + Assert.assertEquals(OptimizedType.INDEX, context.optimizedType()); + Assert.assertSame(root, context.matchQuery()); + } - ConditionQuery sortKeys = new ConditionQuery(HugeType.EDGE); - sortKeys.query(Condition.eq(key, "word")); - sortKeys.optimized(OptimizedType.SORT_KEYS); - Assert.assertTrue(GraphTransaction.queryNeedsPostFilter(sortKeys)); - Assert.assertFalse(GraphTransaction.queryNeedsPostFilter( - new Query(HugeType.EDGE))); + @Test + public void testDirectIdsHaveNoConditionFilter() { + IdQuery query = new IdQuery(HugeType.EDGE, IdGenerator.of(1L)); + QueryResultContext context = new QueryResultContext(query); + Assert.assertNull(context.matchQuery()); + Assert.assertFalse(context.conditionFilterRequired()); + Assert.assertNull(context.resultsFilter()); + Assert.assertFalse(new QueryResultContext(query, true).mustSortByInputIds()); } } diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java index d48738b840..85b33b4661 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java @@ -17,12 +17,13 @@ package org.apache.hugegraph.unit; -import org.apache.hugegraph.backend.tx.GraphIndexTransactionTest; -import org.apache.hugegraph.backend.tx.GraphTransactionTest; import org.apache.hugegraph.api.auth.GraphSpaceAuthPayloadTest; import org.apache.hugegraph.api.auth.GraphSpaceGroupAPITest; import org.apache.hugegraph.auth.StandardAuthManagerV2Test; import org.apache.hugegraph.auth.WsAndHttpBasicAuthHandlerTest; +import org.apache.hugegraph.backend.page.QueryListTest; +import org.apache.hugegraph.backend.tx.GraphIndexTransactionTest; +import org.apache.hugegraph.backend.tx.GraphTransactionTest; import org.apache.hugegraph.core.RoleElectionStateMachineTest; import org.apache.hugegraph.meta.EtcdMetaDriverTest; import org.apache.hugegraph.meta.MetaManagerSchemaCacheClearEventTest; @@ -58,7 +59,6 @@ import org.apache.hugegraph.unit.core.PageStateTest; import org.apache.hugegraph.unit.core.QueryResultsTest; import org.apache.hugegraph.unit.core.QueryTest; -import org.apache.hugegraph.unit.core.StandardHugeGraphClearBackendTest; import org.apache.hugegraph.unit.core.RangeTest; import org.apache.hugegraph.unit.core.RolePermissionTest; import org.apache.hugegraph.unit.core.RowLockTest; @@ -66,6 +66,7 @@ import org.apache.hugegraph.unit.core.SecurityManagerTest; import org.apache.hugegraph.unit.core.SerialEnumTest; import org.apache.hugegraph.unit.core.ServerInfoManagerTest; +import org.apache.hugegraph.unit.core.StandardHugeGraphClearBackendTest; import org.apache.hugegraph.unit.core.SystemSchemaStoreTest; import org.apache.hugegraph.unit.core.TaskSchedulerServerInfoTest; import org.apache.hugegraph.unit.core.TraversalUtilTest; @@ -156,6 +157,7 @@ GraphTransactionTest.class, QueryTest.class, QueryResultsTest.class, + QueryListTest.class, RangeTest.class, SecurityManagerTest.class, RolePermissionTest.class, 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 e01a8d799d..668689f4fb 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 @@ -18,8 +18,11 @@ package org.apache.hugegraph.unit.cache; import java.lang.reflect.Field; +import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.Iterator; +import java.util.List; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; @@ -30,30 +33,51 @@ import org.apache.hugegraph.HugeGraphParams; import org.apache.hugegraph.backend.cache.Cache; import org.apache.hugegraph.backend.cache.CachedGraphTransaction; +import org.apache.hugegraph.backend.cache.OffheapCache; import org.apache.hugegraph.backend.id.Id; import org.apache.hugegraph.backend.id.IdGenerator; import org.apache.hugegraph.backend.query.Condition; -import org.apache.hugegraph.backend.query.ConditionQuery; import org.apache.hugegraph.backend.query.ConditionQuery.OptimizedType; +import org.apache.hugegraph.backend.query.ConditionQuery; +import org.apache.hugegraph.backend.query.IdQuery; import org.apache.hugegraph.backend.query.Query; +import org.apache.hugegraph.backend.query.QueryResultContext; +import org.apache.hugegraph.backend.query.QueryResults; +import org.apache.hugegraph.backend.store.BackendStore; import org.apache.hugegraph.backend.store.BackendStoreProvider; +import org.apache.hugegraph.backend.store.ram.RamTable; import org.apache.hugegraph.event.EventHub; import org.apache.hugegraph.event.EventListener; import org.apache.hugegraph.schema.VertexLabel; import org.apache.hugegraph.structure.HugeEdge; +import org.apache.hugegraph.structure.HugeElement; import org.apache.hugegraph.structure.HugeVertex; import org.apache.hugegraph.structure.HugeVertexProperty; import org.apache.hugegraph.testutil.Assert; import org.apache.hugegraph.testutil.Whitebox; import org.apache.hugegraph.type.HugeType; +import org.apache.hugegraph.type.define.Directions; +import org.apache.hugegraph.type.define.HugeKeys; import org.apache.hugegraph.type.define.IdStrategy; import org.apache.hugegraph.type.define.SchemaStatus; import org.apache.hugegraph.unit.BaseUnitTest; import org.apache.hugegraph.unit.FakeObjects; import org.apache.hugegraph.util.Events; +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.core.LogEvent; +import org.apache.logging.log4j.core.LoggerContext; +import org.apache.logging.log4j.core.appender.AbstractAppender; +import org.apache.logging.log4j.core.config.Configuration; +import org.apache.logging.log4j.core.config.LoggerConfig; +import org.apache.logging.log4j.core.config.Property; +import org.apache.tinkerpop.gremlin.structure.Edge; +import org.apache.tinkerpop.gremlin.structure.T; +import org.apache.tinkerpop.gremlin.structure.Vertex; import org.junit.After; import org.junit.Before; import org.junit.Test; +import org.mockito.Mockito; public class CachedGraphTransactionTest extends BaseUnitTest { @@ -528,6 +552,193 @@ public void testEdgeCacheClearWhenDeleteVertex() { Assert.assertFalse(cache.queryEdgesByVertex(IdGenerator.of(2)).hasNext()); } + @Test + public void testWarmPrimaryKeyCandidatesAreFilteredForEachQuery() { + this.graph.schema().propertyKey("key").asText().create(); + this.graph.schema().propertyKey("age").asInt().create(); + this.graph.schema().vertexLabel("primary").properties("key", "age") + .primaryKeys("key").create(); + Vertex vertex = this.graph.addVertex(T.label, "primary", "key", "marko", "age", 20); + this.graph.tx().commit(); + this.cache.clearCache(null, false); + Cache vertices = Whitebox.getInternalState(this.cache, "verticesCache"); + ConditionQuery miss = this.primaryQuery(21); + Assert.assertFalse(this.cache.queryVertices(miss).hasNext()); + Assert.assertEquals(1L, vertices.size()); + long hits = vertices.hits(); + Vertex found = QueryResults.one(this.cache.queryVertices(this.primaryQuery(20))); + Assert.assertEquals(vertex.id(), found.id()); + Assert.assertEquals(hits + 1L, vertices.hits()); + Assert.assertFalse(this.cache.queryVertices(this.primaryQuery(21)).hasNext()); + Assert.assertEquals(hits + 2L, vertices.hits()); + } + + private ConditionQuery primaryQuery(int age) { + ConditionQuery query = new ConditionQuery(HugeType.VERTEX); + query.eq(HugeKeys.LABEL, this.graph.vertexLabel("primary").id()); + query.query(Condition.eq(this.graph.propertyKey("key").id(), "marko")); + query.query(Condition.eq(this.graph.propertyKey("age").id(), age)); + return query; + } + + @Test + public void testMixedVertexCacheHitsRetainInputOrder() { + HugeVertex first = this.newVertex(IdGenerator.of(1L)); + HugeVertex second = this.newVertex(IdGenerator.of(2L)); + HugeVertex third = this.newVertex(IdGenerator.of(3L)); + for (HugeVertex vertex : Arrays.asList(first, second, third)) { + this.cache.addVertex(vertex); + } + this.cache.commit(); + this.cache.clearCache(null, false); + QueryResults.one(this.cache.queryVertices(second.id())); + Cache vertices = Whitebox.getInternalState(this.cache, "verticesCache"); + long hits = vertices.hits(); + IdQuery query = new IdQuery(HugeType.VERTEX); + query.query(third.id()).query(second.id()).query(first.id()); + List ids = new ArrayList<>(); + this.cache.queryVertices(query).forEachRemaining(vertex -> ids.add(vertex.id())); + Assert.assertEquals(Arrays.asList(third.id(), second.id(), first.id()), ids); + Assert.assertEquals(hits + 1L, vertices.hits()); + } + + @Test + public void testColdSpecialVertexQueryRetainsResultType() { + BackendStore store = Mockito.spy(this.params.loadGraphStore()); + CachedGraphTransaction transaction = new CachedGraphTransaction(this.params, store); + try { + transaction.clearCache(null, false); + for (HugeType type : Arrays.asList(HugeType.TASK, HugeType.SERVER)) { + IdQuery query = new IdQuery(type); + query.query(IdGenerator.of(999L)); + Mockito.doAnswer(invocation -> { + Query fetched = invocation.getArgument(0); + Assert.assertEquals(type, fetched.resultType()); + Assert.assertEquals(1, fetched.idsSize()); + return Collections.emptyIterator(); + }).when(store).query(Mockito.any(Query.class)); + Assert.assertFalse(transaction.queryVertices(query).hasNext()); + } + Mockito.verify(store, Mockito.times(2)).query(Mockito.any(Query.class)); + } finally { + transaction.close(); + } + } + + @Test + public void testEdgeBatchCacheRoundTripsThroughOffheap() throws Exception { + HugeVertex first = this.newVertex(IdGenerator.of(1L)); + HugeVertex second = this.newVertex(IdGenerator.of(2L)); + this.cache.addVertex(first); + this.cache.addVertex(second); + this.cache.commit(); + HugeEdge edge = this.newEdge(first, second); + this.cache.addEdge(edge); + this.cache.commit(); + BackendStore store = Mockito.spy(this.params.loadGraphStore()); + CachedGraphTransaction transaction = new CachedGraphTransaction(this.params, store); + OffheapCache offheap = new OffheapCache(this.graph, 100, 1024, 1); + offheap.enableMetrics(true); + Whitebox.setInternalState(transaction, "edgesCache", offheap); + try { + Edge cold = QueryResults.one(transaction.queryEdgesByVertex(first.id())); + Assert.assertEquals(edge.id(), cold.id()); + Assert.assertEquals(1L, offheap.size()); + long hits = offheap.hits(); + Mockito.clearInvocations(store); + 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)); + } finally { + transaction.close(); + AutoCloseable nativeCache = Whitebox.getInternalState(offheap, "cache"); + nativeCache.close(); + } + } + + @Test + public void testRamTableHitDoesNotReadBackendBeforeOptimization() { + HugeVertex first = this.newVertex(IdGenerator.of(1L)); + HugeVertex second = this.newVertex(IdGenerator.of(2L)); + HugeEdge edge = this.newEdge(first, second); + RamTable table = new RamTable(this.graph, 16, 16); + table.addEdge(true, 1L, 2L, Directions.OUT, (int) edge.schemaLabel().id().asLong()); + HugeGraphParams params = Mockito.spy(this.params); + Mockito.doReturn(table).when(params).ramtable(); + BackendStore store = Mockito.spy(this.params.loadGraphStore()); + CachedGraphTransaction transaction = new CachedGraphTransaction(params, store); + try { + transaction.clearCache(null, false); + ConditionQuery query = new ConditionQuery(HugeType.EDGE); + query.eq(HugeKeys.OWNER_VERTEX, first.id()); + query.eq(HugeKeys.DIRECTION, Directions.OUT); + query.eq(HugeKeys.LABEL, edge.schemaLabel().id()); + Mockito.clearInvocations(store); + Edge found = + QueryResults.one(transaction.queryEdges(query)); + Assert.assertEquals(edge.id(), found.id()); + Mockito.verify(store, Mockito.never()).query(Mockito.any(Query.class)); + } finally { + transaction.close(); + } + } + + @Test + public void testRejectedUndefinedRecordWarnsOnceForPublicAndRawQueries() { + this.newVertex(IdGenerator.of(1L)); + Id id = IdGenerator.of(999L); + Cache vertices = Whitebox.getInternalState(this.cache, "verticesCache"); + vertices.update(id, HugeVertex.undefined(this.graph, id)); + ConditionQuery origin = new ConditionQuery(HugeType.VERTEX); + origin.query(Condition.eq(this.graph.propertyKey("name").id(), "absent")); + origin.optimized(OptimizedType.PRIMARY_KEY); + origin.showHidden(true); + String loggerName = "org.apache.hugegraph.backend.tx.AbstractTransaction"; + LoggerContext logging = (LoggerContext) LogManager.getContext(false); + Configuration configuration = logging.getConfiguration(); + LoggerConfig previous = configuration.getLoggers().get(loggerName); + WarningCounter counter = new WarningCounter(); + counter.start(); + LoggerConfig logger = new LoggerConfig(loggerName, Level.WARN, false); + logger.addAppender(counter, Level.WARN, null); + configuration.removeLogger(loggerName); + configuration.addLogger(loggerName, logger); + logging.updateLoggers(); + try { + Assert.assertFalse(this.cache.queryVertices(new IdQuery(origin, id)).hasNext()); + Assert.assertEquals(1, counter.count); + Iterator raw = Whitebox.invoke( + CachedGraphTransaction.class, new Class[]{Query.class}, + "queryVerticesFromBackend", this.cache, new IdQuery(origin, id)); + Assert.assertFalse(raw.hasNext()); + Assert.assertEquals(2, counter.count); + } finally { + configuration.removeLogger(loggerName); + if (previous != null) { + configuration.addLogger(loggerName, previous); + } + logging.updateLoggers(); + counter.stop(); + } + } + + private static final class WarningCounter extends AbstractAppender { + + private int count; + + private WarningCounter() { + super("BatchWarningCounter", null, null, false, Property.EMPTY_ARRAY); + } + + @Override + public void append(LogEvent event) { + if (event.getMessage().getFormattedMessage().startsWith("Left record is found:")) { + this.count++; + } + } + } + @Test public void testPostFilterDefersDeletingLabelToInvalidFilter() { HugeVertex vertex = this.newVertex(IdGenerator.of(1)); @@ -538,18 +749,16 @@ public void testPostFilterDefersDeletingLabelToInvalidFilter() { query.query(Condition.eq(name, "marko")); query.optimized(OptimizedType.INDEX); - Class[] classes = new Class[]{Iterator.class, Query.class}; - Iterator unmatched = Whitebox.invoke( + Class[] classes = new Class[]{QueryResultContext.class, HugeElement.class}; + QueryResultContext context = new QueryResultContext(query); + boolean unmatched = Whitebox.invoke( CachedGraphTransaction.class, classes, - "filterUnmatchedRecords", this.cache, - Collections.singleton(vertex).iterator(), query); - Assert.assertTrue(unmatched.hasNext()); - - Iterator invalid = Whitebox.invoke( + "filterUnmatchedRecord", this.cache, context, vertex); + Assert.assertTrue(unmatched); + boolean invalid = Whitebox.invoke( CachedGraphTransaction.class, classes, - "filterInvalidRecords", this.cache, - Collections.singleton(vertex).iterator(), query); - Assert.assertFalse(invalid.hasNext()); + "filterInvalidRecord", this.cache, context, vertex); + Assert.assertFalse(invalid); } @Test diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/QueryResultsTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/QueryResultsTest.java index 0ee2cf1678..654c4366c6 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/QueryResultsTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/QueryResultsTest.java @@ -19,17 +19,19 @@ import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.Iterator; import java.util.List; -import java.util.NoSuchElementException; import java.util.Set; -import java.util.function.Consumer; import org.apache.hugegraph.backend.id.Id; import org.apache.hugegraph.backend.id.IdGenerator; +import org.apache.hugegraph.backend.query.ConditionQuery; import org.apache.hugegraph.backend.query.IdQuery; import org.apache.hugegraph.backend.query.Query; +import org.apache.hugegraph.backend.query.QueryResultContext; import org.apache.hugegraph.backend.query.QueryResults; +import org.apache.hugegraph.exception.LimitExceedException; import org.apache.hugegraph.testutil.Assert; import org.apache.hugegraph.type.HugeType; import org.apache.hugegraph.type.Idfiable; @@ -40,6 +42,17 @@ public class QueryResultsTest { + @Test + public void testMaterializationCapacityAppliesAcrossBatches() { + Query query = new Query(HugeType.VERTEX); + List values = Collections.nCopies((int) Query.DEFAULT_CAPACITY / 2 + 1, 1); + QueryResults results = QueryResults.flatMap(Arrays.asList(1, 2).iterator(), + ignored -> new QueryResults<>(values.iterator(), query)); + Assert.assertThrows(LimitExceedException.class, () -> { + results.toList(); + }); + } + @Test public void testKeepInputOrderForPagingIdQuery() { Id id1 = IdGenerator.of(1L); @@ -60,9 +73,7 @@ public void testKeepInputOrderForPagingIdQuery() { idQuery); List orderedIds = new ArrayList<>(); - results.keepInputOrderIfNeeded( - Arrays.asList(new TestIdfiable(id1), - new TestIdfiable(id2)).iterator()) + results.keepInputOrderIfNeeded().iterator() .forEachRemaining(item -> orderedIds.add(item.id())); Assert.assertEquals(ImmutableList.of(id2, id1), orderedIds); @@ -88,7 +99,7 @@ public void testKeepInputOrderAcrossBatches() { ImmutableList.of(first, second).iterator(), result -> result); List orderedIds = new ArrayList<>(); - results.keepInputOrderIfNeeded(results.iterator()) + results.keepInputOrderIfNeeded().iterator() .forEachRemaining(item -> orderedIds.add(item.id())); List expected = new ArrayList<>(); @@ -105,7 +116,7 @@ public void testKeepBackendOrderWhenQueryOnlyDescribesPartOfResults() { ImmutableList.of(1L, 2L, 3L)); List orderedIds = new ArrayList<>(); - results.keepInputOrderIfNeeded(results.iterator()) + results.keepInputOrderIfNeeded().iterator() .forEachRemaining(item -> orderedIds.add(item.id())); Assert.assertEquals(ImmutableList.of(IdGenerator.of(1L), @@ -116,29 +127,186 @@ public void testKeepBackendOrderWhenQueryOnlyDescribesPartOfResults() { @Test public void testKeepInputOrderDoesNotDrainFollowingPages() { - IdQuery firstQuery = queryOf(2L, 1L); - IdQuery secondQuery = queryOf(4L, 3L); - @SuppressWarnings("unchecked") - QueryResults[] holder = new QueryResults[1]; - PagingIterator origin = new PagingIterator( - ImmutableList.of(new TestIdfiable(IdGenerator.of(1L)), - new TestIdfiable(IdGenerator.of(2L)), - new TestIdfiable(IdGenerator.of(3L)), - new TestIdfiable(IdGenerator.of(4L))), - 2, - query -> holder[0].setQuery(query), - ImmutableList.of(firstQuery, secondQuery)); - holder[0] = new QueryResults<>(origin, firstQuery); - - Iterator ordered = - holder[0].keepInputOrderIfNeeded(holder[0].iterator()); - + CountingIterator first = new CountingIterator(1L, 2L); + CountingIterator second = new CountingIterator(3L, 4L); + QueryResults results = QueryResults.flatMap( + ImmutableList.of(0, 1).iterator(), index -> index == 0 ? + new QueryResults<>(first, queryOf(2L, 1L)) : + new QueryResults<>(second, queryOf(4L, 3L))); + Iterator ordered = results.keepInputOrderIfNeeded().iterator(); Assert.assertEquals(IdGenerator.of(2L), ordered.next().id()); Assert.assertEquals(IdGenerator.of(1L), ordered.next().id()); - Assert.assertEquals(2, origin.consumed()); + Assert.assertEquals(2, first.consumed); + Assert.assertEquals(0, second.consumed); Assert.assertEquals(IdGenerator.of(4L), ordered.next().id()); Assert.assertEquals(IdGenerator.of(3L), ordered.next().id()); Assert.assertFalse(ordered.hasNext()); + Assert.assertEquals(1, first.closed); + Assert.assertEquals(1, second.closed); + } + + @Test + public void testOrderingRequirementIsLocalToEachBatch() { + IdQuery firstQuery = queryOf(1L); + firstQuery.mustSortByInput(false); + QueryResults first = new QueryResults<>( + new CountingIterator(1L), firstQuery); + QueryResults second = resultsOf( + ImmutableList.of(3L, 2L), ImmutableList.of(2L, 3L)); + QueryResults results = QueryResults.flatMap( + ImmutableList.of(first, second).iterator(), result -> result); + List ids = new ArrayList<>(); + results.keepInputOrderIfNeeded().iterator() + .forEachRemaining(item -> ids.add(item.id())); + Assert.assertEquals(ImmutableList.of(IdGenerator.of(1L), + IdGenerator.of(3L), + IdGenerator.of(2L)), ids); + } + + @Test + public void testOrderingDoesNotActivateFollowingQuery() { + int[] fetches = {0}; + QueryResults results = QueryResults.flatMap( + ImmutableList.of(0, 1).iterator(), index -> { + fetches[0]++; + return resultsOf(ImmutableList.of(2L, 1L), + ImmutableList.of(1L, 2L)); + }); + Iterator ordered = + results.keepInputOrderIfNeeded().iterator(); + Assert.assertEquals(IdGenerator.of(2L), ordered.next().id()); + Assert.assertEquals(1, fetches[0]); + Assert.assertEquals(IdGenerator.of(1L), ordered.next().id()); + Assert.assertEquals(1, fetches[0]); + Assert.assertTrue(ordered.hasNext()); + Assert.assertEquals(2, fetches[0]); + } + + @Test + public void testNullAndExpandedResultsKeepTheirBatchContext() { + int[] activated = {0}; + QueryResults results = QueryResults.flatMap( + ImmutableList.of(0, 1).iterator(), index -> { + activated[0]++; + return resultsOf(ImmutableList.of((long) index + 1), + ImmutableList.of((long) index + 1)); + }); + QueryResults expanded = results.flatMap(item -> + Arrays.asList(null, item, item).iterator()); + Iterator values = expanded.filter((context, item) -> { + Assert.assertTrue(context.inputIds().contains(item.id())); + Assert.assertEquals(item.id().asLong(), (long) activated[0]); + return true; + }).iterator(); + Assert.assertEquals(IdGenerator.of(1L), values.next().id()); + Assert.assertEquals(IdGenerator.of(1L), values.next().id()); + Assert.assertEquals(1, activated[0]); + Assert.assertEquals(IdGenerator.of(2L), values.next().id()); + Assert.assertEquals(IdGenerator.of(2L), values.next().id()); + Assert.assertFalse(values.hasNext()); + } + + @Test + public void testCloseDoesNotActivateRemainingBatch() throws Exception { + CountingIterator source = new CountingIterator(1L, 2L); + int[] activated = {0}; + QueryResults results = QueryResults.flatMap( + ImmutableList.of(0, 1).iterator(), index -> { + activated[0]++; + return new QueryResults<>(source, queryOf(1L, 2L)); + }); + Iterator values = results.map(item -> item).iterator(); + Assert.assertTrue(values.hasNext()); + ((AutoCloseable) values).close(); + ((AutoCloseable) values).close(); + Assert.assertFalse(values.hasNext()); + Assert.assertEquals(1, activated[0]); + Assert.assertEquals(1, source.closed); + } + + @Test + public void testMapperExceptionPreservesCloseFailure() { + CountingIterator source = new CountingIterator(1L); + RuntimeException failure = new IllegalArgumentException("mapper"); + source.closeFailure = new IllegalStateException("close"); + QueryResults results = new QueryResults<>(source, queryOf(1L)); + Iterator values = results.map(item -> { + throw failure; + }).iterator(); + try { + values.hasNext(); + Assert.fail("Expected mapper failure"); + } catch (IllegalArgumentException actual) { + Assert.assertSame(failure, actual); + Assert.assertArrayEquals(new Throwable[]{source.closeFailure}, actual.getSuppressed()); + } + Assert.assertEquals(1, source.closed); + } + + @Test + public void testEmptyBatchClosesBeforeNextSupplier() { + CountingIterator empty = new CountingIterator(); + CountingIterator source = new CountingIterator(1L); + QueryResults results = QueryResults.flatMap( + ImmutableList.of(0, 1).iterator(), index -> { + if (index == 1) { + Assert.assertEquals(1, empty.closed); + } + return new QueryResults<>(index == 0 ? empty : source, queryOf(1L)); + }); + Assert.assertEquals(IdGenerator.of(1L), results.one().id()); + Assert.assertEquals(1, empty.closed); + Assert.assertEquals(1, source.closed); + } + + @Test + public void testExpandedChildClosesExactlyOnceOnEarlyExit() throws Exception { + CountingIterator source = new CountingIterator(1L, 2L); + CountingIterator child = new CountingIterator(3L, 4L); + Iterator values = new QueryResults<>(source, queryOf(1L, 2L)) + .flatMap(item -> child).iterator(); + Assert.assertEquals(IdGenerator.of(3L), values.next().id()); + ((AutoCloseable) values).close(); + ((AutoCloseable) values).close(); + Assert.assertEquals(1, source.closed); + Assert.assertEquals(1, child.closed); + Assert.assertEquals(1, source.consumed); + Assert.assertEquals(1, child.consumed); + } + + @Test + public void testOrderedBatchThenUnorderedBatchPreservesBackendOrder() { + QueryResults first = resultsOf( + ImmutableList.of(2L, 1L), ImmutableList.of(1L, 2L)); + IdQuery secondQuery = queryOf(4L, 3L); + secondQuery.mustSortByInput(false); + QueryResults second = new QueryResults<>( + new CountingIterator(3L, 4L), secondQuery); + QueryResults results = QueryResults.flatMap( + ImmutableList.of(first, second).iterator(), result -> result); + List ids = new ArrayList<>(); + results.keepInputOrderIfNeeded().iterator() + .forEachRemaining(item -> ids.add(item.id())); + Assert.assertEquals(Arrays.asList(IdGenerator.of(2L), IdGenerator.of(1L), + IdGenerator.of(3L), IdGenerator.of(4L)), ids); + } + + @Test + public void testContextFindsFilterAboveCopiedQueries() { + ConditionQuery root = new ConditionQuery(HugeType.VERTEX); + ConditionQuery search = root.copy(); + ConditionQuery.ResultsFilter filter = element -> true; + search.registerResultsFilter(filter); + ConditionQuery copied = search.copy(); + QueryResultContext context = new QueryResultContext( + new IdQuery(copied, IdGenerator.of(1L))); + Assert.assertSame(filter, context.resultsFilter()); + Assert.assertSame(root, context.matchQuery()); + ConditionQuery other = root.copy(); + other.registerResultsFilter(element -> false); + Assert.assertSame(filter, context.resultsFilter()); + Assert.assertNotSame(context.resultsFilter(), + new QueryResultContext(other).resultsFilter()); } private static QueryResults resultsOf(List input, @@ -178,51 +346,35 @@ public Id id() { } } - private static final class PagingIterator - implements Iterator { + private static final class CountingIterator + implements Iterator, AutoCloseable { - private final List results; - private final int pageSize; - private final Consumer pageListener; - private final List queries; + private final Iterator values; + private int consumed; + private int closed; + private RuntimeException closeFailure; - private int current; - private int announcedPage; - - private PagingIterator(List results, int pageSize, - Consumer pageListener, - List queries) { - this.results = results; - this.pageSize = pageSize; - this.pageListener = pageListener; - this.queries = queries; - this.current = 0; - this.announcedPage = 0; + private CountingIterator(Long... values) { + this.values = Arrays.asList(values).iterator(); } @Override public boolean hasNext() { - if (this.current >= this.results.size()) { - return false; - } - int page = this.current / this.pageSize; - if (page != this.announcedPage) { - this.pageListener.accept(this.queries.get(page)); - this.announcedPage = page; - } - return true; + return this.values.hasNext(); } @Override public TestIdfiable next() { - if (!this.hasNext()) { - throw new NoSuchElementException(); - } - return this.results.get(this.current++); + this.consumed++; + return new TestIdfiable(IdGenerator.of(this.values.next())); } - private int consumed() { - return this.current; + @Override + public void close() { + this.closed++; + if (this.closeFailure != null) { + throw this.closeFailure; + } } } } From e22126d4c871ff91b9a3b5f89c0396d8e3c1a8c3 Mon Sep 17 00:00:00 2001 From: contrueCT Date: Sat, 5 Sep 2026 13:41:53 +0800 Subject: [PATCH 2/7] fix(server): preserve remaining query batches and bound diagnostics --- .../hugegraph/backend/query/QueryBatch.java | 7 ++ .../hugegraph/backend/query/QueryResults.java | 105 ++++++++++++++---- .../hugegraph/backend/page/QueryListTest.java | 28 +++++ .../hugegraph/unit/core/QueryResultsTest.java | 96 ++++++++++++++++ 4 files changed, 213 insertions(+), 23 deletions(-) diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/QueryBatch.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/QueryBatch.java index 997375b04b..e7177b0b6f 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/QueryBatch.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/QueryBatch.java @@ -164,6 +164,13 @@ public abstract static class BatchIterator implements CIter { protected abstract void closeResources() throws Exception; + final T peek() { + if (!this.hasNext()) { + throw new NoSuchElementException(); + } + return this.current; + } + @Override public final boolean hasNext() { if (this.closed) { diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/QueryResults.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/QueryResults.java index b149c91984..c9dddc5dfe 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/QueryResults.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/QueryResults.java @@ -37,7 +37,7 @@ /** A lazy stream of query batches. Only the final consumer flattens the stream. */ public class QueryResults { - private final Iterator> batches; + private final BatchIterator> batches; private final List queries; private final Object metadata; private Iterator results; @@ -48,7 +48,8 @@ public QueryResults(Iterator results, Query query) { public QueryResults(Iterator results, QueryResultContext context) { QueryBatch batch = new QueryBatch<>(results, context); - this.batches = new BatchIterator>() { + this.queries = new ArrayList<>(Collections.singletonList(context.queries().get(0))); + this.batches = this.trackBatches(new BatchIterator>() { private boolean fetched; @Override @@ -64,15 +65,47 @@ protected QueryBatch fetch() { protected void closeResources() throws Exception { batch.close(); } - }; - this.queries = new ArrayList<>(Collections.singletonList(context.queries().get(0))); + }); this.metadata = batch.results(); } private QueryResults(Iterator> batches, Object metadata) { - this.batches = batches; - this.metadata = metadata; this.queries = new ArrayList<>(); + this.batches = this.trackBatches(batches); + this.metadata = metadata; + } + + private BatchIterator> trackBatches(Iterator> origin) { + return new BatchIterator>() { + private QueryBatch active; + + @Override + protected QueryBatch fetch() throws Exception { + QueryBatch previous = this.active; + this.active = null; + QueryBatch.closeAll(previous); + if (!origin.hasNext()) { + return null; + } + this.active = origin.next(); + queries.clear(); + queries.add(this.active.context().queries().get(0)); + return this.active; + } + + @Override + protected void closeResources() throws Exception { + QueryBatch previous = this.active; + this.active = null; + queries.clear(); + QueryBatch.closeAll(previous, origin); + } + + @Override + public Object metadata(String meta, Object... args) { + return QueryBatch.metadataOf(origin, meta, args); + } + }; } public static QueryResults fromBatches(Iterator> batches) { @@ -85,29 +118,49 @@ public Iterator> batches() { public Iterator iterator() { if (this.results == null) { - this.results = new BatchIterator() { - private QueryBatch active; + this.results = new CIter() { + private boolean closed; @Override - protected R fetch() throws Exception { - while (true) { - if (this.active != null && this.active.results().hasNext()) { - return this.active.results().next(); - } - QueryBatch previous = this.active; - this.active = null; - QueryBatch.closeAll(previous); - if (!batches.hasNext()) { - return null; + public boolean hasNext() { + if (this.closed) { + return false; + } + try { + while (batches.hasNext()) { + // Leave the batch and its prefetched element in the shared cursor. + if (batches.peek().results().hasNext()) { + return true; + } + batches.next().close(); } - this.active = batches.next(); - queries.add(this.active.context().queries().get(0)); + this.close(); + return false; + } catch (Throwable failure) { + QueryResults.close(this, failure); + throw QueryBatch.propagate(failure); + } + } + + @Override + public R next() { + if (!this.hasNext()) { + throw new NoSuchElementException(); + } + try { + return batches.peek().results().next(); + } catch (Throwable failure) { + QueryResults.close(this, failure); + throw QueryBatch.propagate(failure); } } @Override - protected void closeResources() throws Exception { - QueryBatch.closeAll(this.active, batches); + public void close() throws Exception { + if (!this.closed) { + this.closed = true; + batches.close(); + } } @Override @@ -165,6 +218,10 @@ public R one() { return one(this.iterator()); } + /** + * Source query of the current batch. A known single source is available before + * activation; composed streams start empty. Closing clears the diagnostics. + */ public List queries() { return Collections.unmodifiableList(this.queries); } @@ -189,7 +246,9 @@ public QueryResults toList() { close(this.batches, failure); } QueryResults result = new QueryResults<>(fetched.iterator(), this.metadata); - result.queries.addAll(this.queries); + if (!fetched.isEmpty()) { + result.queries.add(fetched.get(0).context().queries().get(0)); + } return result; } diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/backend/page/QueryListTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/backend/page/QueryListTest.java index 43201dd803..4683f89d63 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/backend/page/QueryListTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/backend/page/QueryListTest.java @@ -42,6 +42,34 @@ public class QueryListTest { + @Test + public void testPagingRetainsOnlyCurrentQuery() throws Exception { + ConditionQuery query = new ConditionQuery(HugeType.VERTEX); + query.page(""); + query.limit(10000L); + int[] pages = {0}; + IdHolderList holders = new IdHolderList(true); + holders.add(new PagingIdHolder(query, page -> { + long id = ++pages[0]; + return new PageIds(ids(id), new PageState(new byte[]{1}, 0, 1)); + })); + QueryList list = new QueryList<>(query, batch -> new QueryResults<>( + Collections.singletonList(new Item(batch.ids().iterator().next())).iterator(), + batch)); + list.add(holders, 1L); + QueryResults results = list.fetch(1); + Iterator iterator = results.iterator(); + for (long id = 1L; id <= 10000L; id++) { + Assert.assertEquals(IdGenerator.of(id), iterator.next().id()); + Assert.assertEquals(1, results.queries().size()); + Assert.assertEquals(Collections.singletonList(IdGenerator.of(id)), + results.queries().get(0).ids()); + } + Assert.assertEquals(10000, pages[0]); + ((AutoCloseable) iterator).close(); + Assert.assertTrue(results.queries().isEmpty()); + } + @Test public void testOrderingDoesNotFetchNextIndexPage() { ConditionQuery query = new ConditionQuery(HugeType.VERTEX); diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/QueryResultsTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/QueryResultsTest.java index 654c4366c6..9e026a261a 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/QueryResultsTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/QueryResultsTest.java @@ -26,12 +26,15 @@ import org.apache.hugegraph.backend.id.Id; import org.apache.hugegraph.backend.id.IdGenerator; +import org.apache.hugegraph.backend.page.PageInfo; +import org.apache.hugegraph.backend.page.PageState; import org.apache.hugegraph.backend.query.ConditionQuery; import org.apache.hugegraph.backend.query.IdQuery; import org.apache.hugegraph.backend.query.Query; import org.apache.hugegraph.backend.query.QueryResultContext; import org.apache.hugegraph.backend.query.QueryResults; import org.apache.hugegraph.exception.LimitExceedException; +import org.apache.hugegraph.iterator.CIter; import org.apache.hugegraph.testutil.Assert; import org.apache.hugegraph.type.HugeType; import org.apache.hugegraph.type.Idfiable; @@ -42,6 +45,99 @@ public class QueryResultsTest { + @Test + public void testMaterializationKeepsPageMetadataAfterClosingSource() { + PageState page = new PageState(new byte[]{1, 2}, 0, 2); + CountingIterator source = new CountingIterator(1L, 2L); + CIter paged = new CIter() { + @Override + public boolean hasNext() { + return source.hasNext(); + } + + @Override + public TestIdfiable next() { + return source.next(); + } + + @Override + public void close() { + source.close(); + } + + @Override + public Object metadata(String meta, Object... args) { + Assert.assertEquals(PageInfo.PAGE, meta); + return page; + } + }; + QueryResults results = new QueryResults<>(paged, queryOf(1L, 2L)); + Assert.assertTrue(results.iterator().hasNext()); + QueryResults fetched = results.toList(); + Assert.assertEquals(1, source.closed); + Assert.assertSame(page, PageInfo.pageState(results.iterator())); + Assert.assertSame(page, PageInfo.pageState(fetched.iterator())); + List actual = new ArrayList<>(); + fetched.iterator().forEachRemaining(item -> actual.add(item.id())); + Assert.assertEquals(Arrays.asList(IdGenerator.of(1L), IdGenerator.of(2L)), actual); + Assert.assertSame(page, PageInfo.pageState(fetched.iterator())); + } + + @Test + public void testMaterializeAfterPeekAndPartialConsumption() { + for (int consumed = 0; consumed <= 2; consumed++) { + CountingIterator first = new CountingIterator(1L, 2L); + CountingIterator second = new CountingIterator(3L, 4L); + QueryResults results = QueryResults.flatMap( + Arrays.asList(0, 1).iterator(), index -> new QueryResults<>( + index == 0 ? first : second, + index == 0 ? queryOf(1L, 2L) : queryOf(3L, 4L))); + Iterator iterator = results.iterator(); + for (int i = 0; i < consumed; i++) { + Assert.assertEquals(IdGenerator.of(i + 1L), iterator.next().id()); + } + Assert.assertTrue(iterator.hasNext()); + QueryResults fetched = results.toList(); + List actual = new ArrayList<>(); + fetched.filter((context, item) -> { + Assert.assertTrue(context.inputIds().contains(item.id())); + return true; + }).iterator().forEachRemaining(item -> actual.add(item.id())); + List expected = new ArrayList<>(); + for (long id = consumed + 1L; id <= 4L; id++) { + expected.add(IdGenerator.of(id)); + } + Assert.assertEquals(expected, actual); + Assert.assertFalse(iterator.hasNext()); + Assert.assertEquals(1, first.closed); + Assert.assertEquals(1, second.closed); + } + } + + @Test + public void testMapAfterPeekKeepsCurrentBatch() { + CountingIterator source = new CountingIterator(1L, 2L); + QueryResults results = new QueryResults<>(source, queryOf(1L, 2L)); + Assert.assertTrue(results.iterator().hasNext()); + List actual = new ArrayList<>(); + results.map(TestIdfiable::id).iterator().forEachRemaining(actual::add); + Assert.assertEquals(Arrays.asList(IdGenerator.of(1L), IdGenerator.of(2L)), actual); + Assert.assertEquals(1, source.closed); + } + + @Test + public void testQueryDiagnosticsDoNotDuplicateAndClearOnClose() throws Exception { + IdQuery query = queryOf(1L, 2L); + QueryResults results = new QueryResults<>( + new CountingIterator(1L, 2L), query); + Assert.assertEquals(Collections.singletonList(query), results.queries()); + Iterator iterator = results.iterator(); + Assert.assertTrue(iterator.hasNext()); + Assert.assertEquals(Collections.singletonList(query), results.queries()); + ((AutoCloseable) iterator).close(); + Assert.assertTrue(results.queries().isEmpty()); + } + @Test public void testMaterializationCapacityAppliesAcrossBatches() { Query query = new Query(HugeType.VERTEX); From c694bb0a65a546d92da022a8d2d6106c98e95bdb Mon Sep 17 00:00:00 2001 From: contrueCT Date: Sat, 5 Sep 2026 14:18:38 +0800 Subject: [PATCH 3/7] fix(server): preserve empty backend page boundaries --- .../backend/cache/CachedGraphTransaction.java | 24 ++++++--- .../backend/page/PageEntryIterator.java | 6 +++ .../backend/tx/GraphTransaction.java | 5 ++ .../hugegraph/backend/page/QueryListTest.java | 51 +++++++++++++++++++ .../cache/CachedGraphTransactionTest.java | 28 ++++++++++ 5 files changed, 108 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 3680977b4d..abba5a2985 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 @@ -20,6 +20,7 @@ import java.lang.reflect.Array; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.Iterator; import java.util.List; import java.util.Set; @@ -337,6 +338,9 @@ protected QueryResults fetchVertexBatch(Query query) { } if (!missing.empty()) { QueryResults fetched = super.fetchVertexBatch(vertices.isEmpty() ? query : missing); + if (vertices.isEmpty() && !fetched.batches().hasNext()) { + return fetched; + } ListIterator candidates = QueryResults.toList(fetched.iterator()); for (HugeVertex vertex : candidates.list()) { if (this.needCacheVertex(vertex)) { @@ -384,6 +388,10 @@ protected QueryResults fetchEdgeBatch(Query query) { return this.filterExpiredBatches(new QueryResults<>(cached.iterator(), context)); } QueryResults fetched = super.fetchEdgeBatch(query); + if (!fetched.batches().hasNext()) { + this.cacheEdgeBatch(cacheKey, batchKey, Collections.emptyList()); + return fetched; + } return fetched.mapBatches(batch -> { Iterator source = batch.results(); List candidates = new ArrayList<>(MAX_CACHE_EDGES_PER_QUERY + 1); @@ -392,18 +400,22 @@ protected QueryResults fetchEdgeBatch(Query query) { candidates.add(source.next()); } if (candidates.size() <= MAX_CACHE_EDGES_PER_QUERY) { - synchronized (this.edgesCache) { - CachedEdgeQuery existing = new CachedEdgeQuery(this.edgesCache.get(cacheKey)); - if (existing.put(batchKey, candidates)) { - this.edgesCache.update(cacheKey, existing.values); - } - } + this.cacheEdgeBatch(cacheKey, batchKey, candidates); } return new QueryBatch<>( new ExtendableIterator<>(candidates.iterator(), source), batch.context()); }); } + private void cacheEdgeBatch(Id cacheKey, Id batchKey, List candidates) { + synchronized (this.edgesCache) { + CachedEdgeQuery existing = new CachedEdgeQuery(this.edgesCache.get(cacheKey)); + if (existing.put(batchKey, candidates)) { + this.edgesCache.update(cacheKey, existing.values); + } + } + } + /** Nested lists retain the existing off-heap cache's serialization support. */ private static final class CachedEdgeQuery { diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/page/PageEntryIterator.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/page/PageEntryIterator.java index 55c5fa50bf..84bf174f27 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/page/PageEntryIterator.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/page/PageEntryIterator.java @@ -61,6 +61,12 @@ protected QueryBatch fetch() throws Exception { Math.min(this.pageSize, this.remaining); QueryList.PageResults page = this.queries.fetchNext(this.pageInfo, size); this.pageBatches = page.results().batches(); + if (!this.pageBatches.hasNext()) { + // Preserve the raw-empty-page boundary. An existing batch + // emptied by parsing or TTL filtering still follows its cursor. + this.pageInfo.increase(); + continue; + } if (page.hasNextPage()) { this.pageInfo.page(page.page()); } else { diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/tx/GraphTransaction.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/tx/GraphTransaction.java index 3c2879c3c0..d011388e8e 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/tx/GraphTransaction.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/tx/GraphTransaction.java @@ -869,6 +869,11 @@ private QueryResults queryVertexBatchesFromBackend(Query query) { private QueryResults backendBatches(Query query) { QueryResults results = super.query(query); + if (!results.iterator().hasNext()) { + // No raw records means no batch. Filtering a nonempty source may + // still produce an empty batch, which must keep its page cursor. + return results; + } QueryResultContext context = new QueryResultContext( query, this.storeFeatures().supportsQuerySortByInputIds()); return results.mapBatches(batch -> new QueryBatch<>(batch.results(), context)); diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/backend/page/QueryListTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/backend/page/QueryListTest.java index 4683f89d63..947e67aa04 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/backend/page/QueryListTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/backend/page/QueryListTest.java @@ -42,6 +42,57 @@ public class QueryListTest { + @Test + public void testPageWithoutBackendBatchesEndsHolder() { + ConditionQuery query = new ConditionQuery(HugeType.VERTEX); + query.page(""); + query.limit(2L); + int[] firstFetches = {0}; + IdHolderList holders = new IdHolderList(true); + holders.add(new PagingIdHolder(query, page -> { + firstFetches[0]++; + return new PageIds(ids(1L), new PageState(new byte[]{1}, 0, 1)); + })); + holders.add(new PagingIdHolder(query, page -> new PageIds(ids(2L), PageState.EMPTY))); + QueryList list = new QueryList<>(query, batch -> { + if (batch.ids().contains(IdGenerator.of(1L))) { + return QueryResults.fromBatches(Collections.emptyIterator()); + } + return new QueryResults<>(Collections.singletonList(new Item(IdGenerator.of(2L))) + .iterator(), batch); + }); + list.add(holders, 1L); + Iterator results = list.fetch(1).iterator(); + Assert.assertEquals(IdGenerator.of(2L), results.next().id()); + Assert.assertFalse(results.hasNext()); + Assert.assertEquals(1, firstFetches[0]); + Assert.assertNull(PageInfo.pageInfo(results)); + } + + @Test + public void testFilteredEmptyBatchKeepsFollowingPage() { + ConditionQuery query = new ConditionQuery(HugeType.VERTEX); + query.page(""); + query.limit(2L); + int[] pages = {0}; + IdHolderList holders = new IdHolderList(true); + holders.add(new PagingIdHolder(query, page -> { + long id = ++pages[0]; + return new PageIds(ids(id), new PageState(id == 1L ? new byte[]{1} : new byte[0], 0, 1)); + })); + QueryList list = new QueryList<>(query, batch -> { + Item item = new Item(batch.ids().iterator().next()); + return new QueryResults<>(Collections.singletonList(item).iterator(), batch) + .filter((context, value) -> value.id().asLong() == 2L); + }); + list.add(holders, 1L); + Iterator results = list.fetch(1).iterator(); + Assert.assertEquals(IdGenerator.of(2L), results.next().id()); + Assert.assertFalse(results.hasNext()); + Assert.assertEquals(2, pages[0]); + Assert.assertNull(PageInfo.pageInfo(results)); + } + @Test public void testPagingRetainsOnlyCurrentQuery() throws Exception { ConditionQuery query = new ConditionQuery(HugeType.VERTEX); 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 668689f4fb..6e7e5d82b8 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 @@ -170,6 +170,34 @@ private HugeEdge newEdge(HugeVertex out, HugeVertex in) { return out.addEdge("person_know_person", in); } + @Test + public void testMissingVertexDoesNotCreateAnEmptyBatch() { + Query query = new IdQuery(HugeType.VERTEX, IdGenerator.of(999L)); + QueryResults results = Whitebox.invoke( + CachedGraphTransaction.class, new Class[]{Query.class}, + "fetchVertexBatch", this.cache, query); + Assert.assertFalse(results.batches().hasNext()); + } + + @Test + public void testExpiredVertexKeepsItsBatchAfterCacheMaterialization() throws InterruptedException { + this.graph.schema().vertexLabel("expiring").useCustomizeNumberId() + .ttl(1000L).create(); + Vertex vertex = this.graph.addVertex(T.label, "expiring", T.id, 1L); + this.graph.tx().commit(); + Thread.sleep(1100L); + // TTL is evaluated at transaction opening time, including internal fetches. + this.graph.tx().open(); + Assert.assertTrue(((HugeVertex) vertex).expired()); + Query query = new IdQuery(HugeType.VERTEX, (Id) vertex.id()); + QueryResults results = Whitebox.invoke( + CachedGraphTransaction.class, new Class[]{Query.class}, + "fetchVertexBatch", this.cache, query); + Assert.assertTrue(results.batches().hasNext()); + Assert.assertFalse(results.batches().next().results().hasNext()); + Assert.assertFalse(results.batches().hasNext()); + } + @Test public void testEventClear() throws Exception { CachedGraphTransaction cache = this.cache(); From 17788a4a93bdfad29ade1b844e54f3313480c63d Mon Sep 17 00:00:00 2001 From: contrueCT Date: Sat, 5 Sep 2026 22:19:00 +0800 Subject: [PATCH 4/7] fix(server): preserve paging metadata and isolate empty pages --- .../backend/cache/CachedGraphTransaction.java | 3 +- .../hugegraph/backend/page/QueryList.java | 8 +--- .../hugegraph/backend/page/QueryListTest.java | 13 +++++++ .../apache/hugegraph/core/VertexCoreTest.java | 38 +++++++++++++++++++ 4 files changed, 55 insertions(+), 7 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 abba5a2985..228298b831 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 @@ -319,7 +319,8 @@ private boolean needCacheVertex(HugeVertex vertex) { @Override @Watched(prefix = "graphcache") protected QueryResults fetchVertexBatch(Query query) { - if (!this.enableCacheVertex() || query.idsSize() == 0 || query.conditionsSize() != 0) { + if (!this.enableCacheVertex() || query.paging() || + query.idsSize() == 0 || query.conditionsSize() != 0) { return super.fetchVertexBatch(query); } QueryResultContext context = new QueryResultContext(query); diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/page/QueryList.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/page/QueryList.java index fa6c6bed72..059638a8e1 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/page/QueryList.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/page/QueryList.java @@ -334,10 +334,6 @@ private IdQuery indexIdQuery(Query bindQuery, Set ids, boolean inOrder) { public static class PageResults { - public static final PageResults EMPTY = new PageResults<>( - QueryResults.empty(), null, - PageState.EMPTY); - private final QueryResults results; private final PageState pageState; private final Query query; @@ -373,9 +369,9 @@ public long total() { return this.pageState.total(); } - @SuppressWarnings("unchecked") public static PageResults emptyIterator() { - return (PageResults) EMPTY; + // Batch cursors are single-use, including those for empty pages. + return new PageResults<>(QueryResults.empty(), null, PageState.EMPTY); } } } diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/backend/page/QueryListTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/backend/page/QueryListTest.java index 947e67aa04..6251ed27f9 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/backend/page/QueryListTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/backend/page/QueryListTest.java @@ -31,6 +31,7 @@ import org.apache.hugegraph.backend.page.IdHolder.PagingIdHolder; import org.apache.hugegraph.backend.query.ConditionQuery.OptimizedType; import org.apache.hugegraph.backend.query.ConditionQuery; +import org.apache.hugegraph.backend.query.QueryBatch; import org.apache.hugegraph.backend.query.QueryResults; import org.apache.hugegraph.backend.serializer.TextBackendEntry; import org.apache.hugegraph.backend.store.BackendEntry; @@ -42,6 +43,18 @@ public class QueryListTest { + @Test + public void testEmptyPagesHaveIndependentBatchLifecycles() { + Iterator> first = QueryList.PageResults.emptyIterator().results().batches(); + Iterator> second = QueryList.PageResults.emptyIterator().results().batches(); + Assert.assertTrue(first.hasNext()); + Assert.assertTrue(second.hasNext()); + Assert.assertFalse(second.next().results().hasNext()); + Assert.assertFalse(second.hasNext()); + Assert.assertFalse(first.next().results().hasNext()); + Assert.assertFalse(first.hasNext()); + } + @Test public void testPageWithoutBackendBatchesEndsHolder() { ConditionQuery query = new ConditionQuery(HugeType.VERTEX); diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/VertexCoreTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/VertexCoreTest.java index 80c4aef50b..335130ede5 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/VertexCoreTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/VertexCoreTest.java @@ -36,6 +36,7 @@ import org.apache.hugegraph.HugeException; import org.apache.hugegraph.HugeGraph; import org.apache.hugegraph.backend.BackendException; +import org.apache.hugegraph.backend.cache.CachedGraphTransaction; import org.apache.hugegraph.backend.id.Id; import org.apache.hugegraph.backend.id.Id.IdType; import org.apache.hugegraph.backend.id.IdGenerator; @@ -3348,6 +3349,43 @@ public void testQueryByPrimaryValuesAndProps() { Assert.assertEquals(0, vertices.size()); } + @Test + public void testQueryByPrimaryValuesInPageWithVertexCache() { + Assume.assumeTrue("Not support paging", storeFeatures().supportsQueryByPage()); + HugeGraph graph = graph(); + Vertex vertex = graph.addVertex(T.label, "person", "name", "marko", + "age", 29, "city", "Beijing"); + this.commitTx(); + CachedGraphTransaction cache = (CachedGraphTransaction) this.params().graphTransaction(); + cache.clearCache(HugeType.VERTEX, false); + + for (int i = 0; i < 2; i++) { + if (i == 1) { + Assert.assertEquals(vertex.id(), graph.vertices(vertex.id()).next().id()); + } + GraphTraversal results = graph.traversal().V() + .hasLabel("person").has("name", "marko").has("~page", "").limit(2); + List vertices = results.toList(); + Assert.assertEquals(1, vertices.size()); + Assert.assertEquals(vertex.id(), vertices.get(0).id()); + Assert.assertNull(TraversalUtil.page(results)); + CloseableIterator.closeIterator(results); + } + + GraphTraversal filtered = graph.traversal().V() + .hasLabel("person").has("name", "marko").has("age", 30) + .has("~page", "").limit(2); + Assert.assertFalse(filtered.hasNext()); + Assert.assertNull(TraversalUtil.page(filtered)); + CloseableIterator.closeIterator(filtered); + + GraphTraversal missing = graph.traversal().V() + .hasLabel("person").has("name", "missing").has("~page", "").limit(2); + Assert.assertFalse(missing.hasNext()); + Assert.assertNull(TraversalUtil.page(missing)); + CloseableIterator.closeIterator(missing); + } + @Test public void testQueryByPrimaryValuesAndPropsWithCachedVertex() { HugeGraph graph = graph(); From 05186a0e47d098c269877e31f14cf178ee399802 Mon Sep 17 00:00:00 2001 From: contrueCT Date: Sat, 5 Sep 2026 23:18:39 +0800 Subject: [PATCH 5/7] fix(server): cover batch lifecycle and register query helpers --- .../hugegraph/auth/HugeFactoryAuthProxy.java | 15 +- .../hugegraph/backend/page/QueryListTest.java | 84 +++++++++++ .../backend/tx/GraphTransactionTest.java | 142 ++++++++++++++++++ .../cache/CachedGraphTransactionTest.java | 130 ++++++++++++++++ .../hugegraph/unit/core/QueryResultsTest.java | 46 ++++++ 5 files changed, 413 insertions(+), 4 deletions(-) diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeFactoryAuthProxy.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeFactoryAuthProxy.java index c9ce7cca4f..5ff2925e69 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeFactoryAuthProxy.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeFactoryAuthProxy.java @@ -32,6 +32,7 @@ import org.apache.hugegraph.HugeGraph; import org.apache.hugegraph.StandardHugeGraph; import org.apache.hugegraph.backend.cache.CacheManager; +import org.apache.hugegraph.backend.cache.CachedGraphTransaction; import org.apache.hugegraph.backend.tx.AbstractTransaction; import org.apache.hugegraph.backend.tx.GraphTransaction; import org.apache.hugegraph.backend.tx.IndexableTransaction; @@ -272,13 +273,19 @@ private static void registerPrivateActions() { "verifyEdgesConditionQuery", "indexQuery", "joinTxRecords", "propertyUpdated", "parseEntry", "traverseByLabel", "reset", "queryVerticesByIds", - "filterInvalidRecords", "filterUnmatchedRecords", + "filterInvalidRecord", "filterUnmatchedRecord", "invalidRecord", "warnLeftRecord", "skipOffsetOrStopLimit", - "filterExpiredResultFromFromBackend", "queryEdgesByIds", + "filterExpiredBatches", "queryEdgesByIds", "matchEdgeSortKeys", "rightResultFromIndexQuery", - "queryNeedsPostFilter", - "conditionQueryNeedsPostFilter"); + "queryVertexBatchesFromBackend", "backendBatches", + "fetchVertexBatch", "processBatches", + "queryEdgeBatchesFromBackend", + "queryEdgeBatchesFromBackendInternal", + "queryEdgesFromMemory", "fetchEdgeBatch"); + Reflection.registerMethodsToFilter(CachedGraphTransaction.class, + "fetchVertexBatch", "fetchEdgeBatch", + "queryEdgesFromMemory", "cacheEdgeBatch"); Reflection.registerFieldsToFilter(IndexableTransaction.class, "$assertionsDisabled"); Reflection.registerMethodsToFilter(IndexableTransaction.class, "indexTransaction", "commit2Backend", "reset"); diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/backend/page/QueryListTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/backend/page/QueryListTest.java index 6251ed27f9..2b70d589b4 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/backend/page/QueryListTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/backend/page/QueryListTest.java @@ -35,6 +35,7 @@ import org.apache.hugegraph.backend.query.QueryResults; import org.apache.hugegraph.backend.serializer.TextBackendEntry; import org.apache.hugegraph.backend.store.BackendEntry; +import org.apache.hugegraph.iterator.CIter; import org.apache.hugegraph.testutil.Assert; import org.apache.hugegraph.type.HugeType; import org.apache.hugegraph.type.Idfiable; @@ -299,6 +300,89 @@ public void testMultipleIndexHoldersFetchOneIdBatchAtATime() { Assert.assertFalse(values.hasNext()); } + @Test + public void testClosingQueryListReleasesUnconsumedHolders() throws Exception { + this.assertHoldersClosed(false); + } + + @Test + public void testFetcherFailureReleasesAllHolders() throws Exception { + this.assertHoldersClosed(true); + } + + private void assertHoldersClosed(boolean failFetcher) throws Exception { + ConditionQuery root = new ConditionQuery(HugeType.VERTEX); + IdHolderList holders = new IdHolderList(false); + List sources = new ArrayList<>(); + for (long id = 1L; id <= 2L; id++) { + TrackedSource source = new TrackedSource(id); + sources.add(source); + holders.add(new BatchIdHolder(new ConditionQuery(HugeType.SECONDARY_INDEX, root), + source, size -> ids(source.next().id().asLong()))); + } + RuntimeException failure = new IllegalStateException("fetcher"); + QueryList list = new QueryList<>(root, query -> { + if (failFetcher) { + throw failure; + } + return new QueryResults<>(Collections.singletonList(new Item(query.ids().iterator().next())) + .iterator(), query); + }); + list.add(holders, 1L); + Iterator results = list.fetch(1).iterator(); + if (failFetcher) { + try { + results.hasNext(); + Assert.fail("Expected fetcher failure"); + } catch (IllegalStateException actual) { + Assert.assertSame(failure, actual); + } + Assert.assertEquals(1, sources.get(0).closed); + Assert.assertEquals(1, sources.get(1).closed); + } else { + Assert.assertTrue(results.hasNext()); + } + ((AutoCloseable) results).close(); + ((AutoCloseable) results).close(); + Assert.assertEquals(1, sources.get(0).read); + Assert.assertEquals(0, sources.get(1).read); + Assert.assertEquals(1, sources.get(0).closed); + Assert.assertEquals(1, sources.get(1).closed); + Assert.assertFalse(results.hasNext()); + } + + private static final class TrackedSource implements CIter { + + private final BackendEntry entry; + private int read; + private int closed; + + private TrackedSource(long id) { + this.entry = new TextBackendEntry(HugeType.VERTEX, IdGenerator.of(id)); + } + + @Override + public boolean hasNext() { + return this.read == 0; + } + + @Override + public BackendEntry next() { + this.read++; + return this.entry; + } + + @Override + public void close() { + this.closed++; + } + + @Override + public Object metadata(String meta, Object... args) { + return null; + } + } + private static Set ids(Long... values) { Set ids = InsertionOrderUtil.newSet(); for (long value : values) { diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/backend/tx/GraphTransactionTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/backend/tx/GraphTransactionTest.java index cb50197614..3cb79e3687 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/backend/tx/GraphTransactionTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/backend/tx/GraphTransactionTest.java @@ -17,14 +17,35 @@ package org.apache.hugegraph.backend.tx; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +import org.apache.hugegraph.HugeFactory; +import org.apache.hugegraph.HugeGraph; +import org.apache.hugegraph.HugeGraphParams; +import org.apache.hugegraph.backend.cache.Cache; +import org.apache.hugegraph.backend.cache.CachedGraphTransaction; +import org.apache.hugegraph.backend.id.Id; import org.apache.hugegraph.backend.id.IdGenerator; +import org.apache.hugegraph.backend.query.Condition; import org.apache.hugegraph.backend.query.ConditionQuery.OptimizedType; import org.apache.hugegraph.backend.query.ConditionQuery; import org.apache.hugegraph.backend.query.IdQuery; import org.apache.hugegraph.backend.query.QueryResultContext; +import org.apache.hugegraph.backend.tx.GraphIndexTransaction.RemoveLeftIndexJob; +import org.apache.hugegraph.job.EphemeralJob; +import org.apache.hugegraph.schema.VertexLabel; +import org.apache.hugegraph.structure.HugeVertex; import org.apache.hugegraph.testutil.Assert; +import org.apache.hugegraph.testutil.Whitebox; import org.apache.hugegraph.type.HugeType; +import org.apache.hugegraph.type.define.SchemaStatus; +import org.apache.hugegraph.unit.FakeObjects; +import org.apache.tinkerpop.gremlin.structure.T; +import org.apache.tinkerpop.gremlin.structure.Vertex; import org.junit.Test; +import org.mockito.Mockito; public class GraphTransactionTest { @@ -77,4 +98,125 @@ public void testDirectIdsHaveNoConditionFilter() { Assert.assertNull(context.resultsFilter()); Assert.assertFalse(new QueryResultContext(query, true).mustSortByInputIds()); } + @Test + public void testInvisibleCandidatesDoNotScheduleIndexCleanup() throws Exception { + try (FilterFixture fixture = new FilterFixture()) { + for (boolean hidden : new boolean[]{true, false}) { + VertexLabel label = new VertexLabel(fixture.graph, fixture.vertex.schemaLabel().id(), + hidden ? "~hidden" : "person"); + if (!hidden) { + label.status(SchemaStatus.DELETING); + } + HugeVertex candidate = new HugeVertex(fixture.graph, fixture.vertex.id(), label); + candidate.addProperty(fixture.graph.propertyKey("name"), "actual"); + fixture.vertices.update(candidate.id(), candidate); + ConditionQuery query = fixture.query("absent", OptimizedType.INDEX); + Assert.assertTrue(fixture.fetch(query).isEmpty()); + Assert.assertTrue(fixture.jobs.isEmpty()); + + // Making the same candidate visible must reach residual filtering and cleanup. + query.showHidden(hidden); + query.showDeleting(!hidden); + Assert.assertTrue(fixture.fetch(query).isEmpty()); + fixture.assertCleanup(query); + fixture.jobs.clear(); + } + } + } + + @Test + public void testResidualMismatchCleansOnlyIndexOptimizedQueries() throws Exception { + try (FilterFixture fixture = new FilterFixture()) { + for (OptimizedType type : Arrays.asList(OptimizedType.PRIMARY_KEY, + OptimizedType.INDEX_FILTER, + OptimizedType.INDEX)) { + ConditionQuery query = fixture.query("absent", type); + Assert.assertTrue(fixture.fetch(query).isEmpty()); + if (type == OptimizedType.INDEX) { + fixture.assertCleanup(query); + } else { + Assert.assertTrue(fixture.jobs.isEmpty()); + } + fixture.jobs.clear(); + } + } + } + + @Test + public void testMatchingCandidateStillSchedulesItsStaleIndexCleanup() throws Exception { + try (FilterFixture fixture = new FilterFixture()) { + ConditionQuery query = fixture.query("actual", OptimizedType.INDEX); + Id name = fixture.graph.propertyKey("name").id(); + query.recordIndexValue(name, fixture.vertex.id(), "actual"); + query.recordIndexValue(name, fixture.vertex.id(), "stale"); + query.selectedIndexField(name); + List found = fixture.fetch(query); + Assert.assertEquals(1, found.size()); + Assert.assertEquals(fixture.vertex.id(), found.get(0).id()); + Assert.assertTrue(query.existLeftIndex(fixture.vertex.id())); + fixture.assertCleanup(query); + } + } + + private static final class FilterFixture implements AutoCloseable { + + private final HugeGraph graph; + private final CachedGraphTransaction transaction; + private final HugeVertex vertex; + private final Cache vertices; + private final List> jobs; + + private FilterFixture() { + this.graph = HugeFactory.open(FakeObjects.newConfig()); + this.graph.schema().propertyKey("name").asText().create(); + this.graph.schema().vertexLabel("person").useCustomizeNumberId() + .properties("name").create(); + this.vertex = (HugeVertex) this.graph.addVertex(T.label, "person", T.id, 1L, + "name", "actual"); + this.graph.tx().commit(); + HugeGraphParams params = Whitebox.getInternalState(this.graph, "params"); + HugeGraphParams observed = Mockito.spy(params); + this.jobs = new ArrayList<>(); + Mockito.doAnswer(invocation -> { + this.jobs.add(invocation.getArgument(0)); + return null; + }).when(observed).submitEphemeralJob(Mockito.any()); + this.transaction = new CachedGraphTransaction(observed, params.loadGraphStore()); + this.transaction.clearCache(null, false); + this.vertices = Whitebox.getInternalState(this.transaction, "verticesCache"); + } + + private ConditionQuery query(String value, OptimizedType type) { + ConditionQuery query = new ConditionQuery(HugeType.VERTEX); + query.query(Condition.eq(this.graph.propertyKey("name").id(), value)); + query.optimized(type); + return query; + } + + private List fetch(ConditionQuery query) { + List found = new ArrayList<>(); + this.transaction.queryVertices(new IdQuery(query, this.vertex.id())) + .forEachRemaining(found::add); + return found; + } + + private void assertCleanup(ConditionQuery query) { + Assert.assertEquals(1, this.jobs.size()); + Assert.assertTrue(this.jobs.get(0) instanceof RemoveLeftIndexJob); + Assert.assertSame(query, Whitebox.getInternalState(this.jobs.get(0), "query")); + HugeVertex cleaned = Whitebox.getInternalState(this.jobs.get(0), "element"); + Assert.assertEquals(this.vertex.id(), cleaned.id()); + } + + @Override + public void close() throws Exception { + try { + this.transaction.close(); + } finally { + this.graph.clearBackend(); + this.graph.close(); + } + } + } + } 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 6e7e5d82b8..0e7b66f0da 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 @@ -22,6 +22,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.Iterator; +import java.util.LinkedHashSet; import java.util.List; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.CountDownLatch; @@ -34,6 +35,7 @@ import org.apache.hugegraph.backend.cache.Cache; import org.apache.hugegraph.backend.cache.CachedGraphTransaction; import org.apache.hugegraph.backend.cache.OffheapCache; +import org.apache.hugegraph.backend.id.EdgeId; import org.apache.hugegraph.backend.id.Id; import org.apache.hugegraph.backend.id.IdGenerator; import org.apache.hugegraph.backend.query.Condition; @@ -685,6 +687,134 @@ public void testEdgeBatchCacheRoundTripsThroughOffheap() throws Exception { } } + @Test + public void testEdgeCacheKeepsDistinctBatchesWithinTotalCandidateLimit() { + List ids = this.persistEdges(101); + ConditionQuery root = this.edgeCacheRequest(); + BackendStore store = Mockito.spy(this.params.loadGraphStore()); + CachedGraphTransaction transaction = new CachedGraphTransaction(this.params, store); + try { + transaction.clearCache(null, false); + for (int start : new int[]{0, 50}) { + List batch = ids.subList(start, start + 50); + IdQuery query = new IdQuery(root, new LinkedHashSet<>(batch)); + this.assertEdgeIds(batch, this.fetchEdgeIds(transaction, query)); + } + Mockito.clearInvocations(store); + for (int start : new int[]{50, 0}) { + List batch = ids.subList(start, start + 50); + IdQuery query = new IdQuery(root, new LinkedHashSet<>(batch)); + this.assertEdgeIds(batch, this.fetchEdgeIds(transaction, query)); + } + Mockito.verify(store, Mockito.never()).query(Mockito.any(Query.class)); + + List overflow = ids.subList(100, 101); + for (int i = 0; i < 2; i++) { + IdQuery query = new IdQuery(root, new LinkedHashSet<>(overflow)); + this.assertEdgeIds(overflow, this.fetchEdgeIds(transaction, query)); + } + Mockito.verify(store, Mockito.times(2)).query(Mockito.any(Query.class)); + Mockito.clearInvocations(store); + IdQuery cached = new IdQuery(root, new LinkedHashSet<>(ids.subList(0, 50))); + this.assertEdgeIds(ids.subList(0, 50), this.fetchEdgeIds(transaction, cached)); + Mockito.verify(store, Mockito.never()).query(Mockito.any(Query.class)); + } finally { + transaction.close(); + } + } + + @Test + public void testOversizedEdgeBatchReturnsEveryCandidateWithoutCaching() { + List all = this.persistEdges(102); + BackendStore store = Mockito.spy(this.params.loadGraphStore()); + CachedGraphTransaction transaction = new CachedGraphTransaction(this.params, store); + try { + transaction.clearCache(null, false); + for (int size : new int[]{101, 102}) { + List ids = all.subList(0, size); + IdQuery query = new IdQuery(this.edgeCacheRequest(), new LinkedHashSet<>(ids)); + this.assertEdgeIds(ids, this.fetchEdgeIds(transaction, query)); + Mockito.clearInvocations(store); + this.assertEdgeIds(ids, this.fetchEdgeIds(transaction, query)); + Mockito.verify(store, Mockito.times(1)).query(Mockito.any(Query.class)); + } + } finally { + transaction.close(); + } + } + + @Test + public void testEmptyEdgeBatchesRespectBatchCountLimit() { + this.persistEdges(1); + ConditionQuery root = this.edgeCacheRequest(); + Id label = this.graph.edgeLabel("person_know_person").id(); + List batches = new ArrayList<>(); + for (long id = 1000L; id <= 1100L; id++) { + Id edge = new EdgeId(IdGenerator.of(1L), Directions.OUT, label, label, + "", IdGenerator.of(id)); + batches.add(new IdQuery(root, edge)); + } + BackendStore store = Mockito.spy(this.params.loadGraphStore()); + CachedGraphTransaction transaction = new CachedGraphTransaction(this.params, store); + try { + transaction.clearCache(null, false); + for (IdQuery batch : batches) { + Assert.assertTrue(this.fetchEdgeIds(transaction, batch).isEmpty()); + } + Mockito.clearInvocations(store); + 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)); + } finally { + transaction.close(); + } + } + + private List persistEdges(int count) { + HugeVertex first = this.newVertex(IdGenerator.of(1L)); + this.cache.addVertex(first); + List targets = new ArrayList<>(); + for (long id = 2L; id < count + 2L; id++) { + HugeVertex vertex = new HugeVertex(this.graph, IdGenerator.of(id), first.schemaLabel()); + this.cache.addVertex(vertex); + targets.add(vertex); + } + this.cache.commit(); + List ids = new ArrayList<>(); + for (HugeVertex target : targets) { + HugeEdge edge = this.newEdge(first, target); + this.cache.addEdge(edge); + ids.add(edge.id()); + } + this.cache.commit(); + return ids; + } + + private ConditionQuery edgeCacheRequest() { + ConditionQuery query = new ConditionQuery(HugeType.EDGE); + query.eq(HugeKeys.OWNER_VERTEX, IdGenerator.of(1L)); + query.eq(HugeKeys.DIRECTION, Directions.OUT); + return query; + } + + private void assertEdgeIds(List expected, List actual) { + // Candidate-cache tests protect membership and cardinality, independent of backend ordering. + Assert.assertEquals(expected.size(), actual.size()); + Assert.assertEquals(new LinkedHashSet<>(expected), new LinkedHashSet<>(actual)); + } + + private List fetchEdgeIds(CachedGraphTransaction transaction, Query query) { + QueryResults results = Whitebox.invoke( + CachedGraphTransaction.class, new Class[]{Query.class}, + "fetchEdgeBatch", transaction, query); + List ids = new ArrayList<>(); + results.iterator().forEachRemaining(edge -> ids.add(edge.id())); + return ids; + } + @Test public void testRamTableHitDoesNotReadBackendBeforeOptimization() { HugeVertex first = this.newVertex(IdGenerator.of(1L)); diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/QueryResultsTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/QueryResultsTest.java index 9e026a261a..7521dd447c 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/QueryResultsTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/QueryResultsTest.java @@ -320,6 +320,44 @@ public void testCloseDoesNotActivateRemainingBatch() throws Exception { Assert.assertEquals(1, source.closed); } + @Test + public void testSourceFailuresCloseMappedResultsOnce() throws Exception { + for (boolean failHasNext : new boolean[]{true, false}) { + CountingIterator source = new CountingIterator(1L); + RuntimeException failure = new IllegalStateException("source"); + if (failHasNext) { + source.hasNextFailure = failure; + } else { + source.nextFailure = failure; + } + Iterator values = new QueryResults<>(source, queryOf(1L)) + .map(item -> item).iterator(); + try { + values.hasNext(); + Assert.fail("Expected source failure"); + } catch (IllegalStateException actual) { + Assert.assertSame(failure, actual); + } + Assert.assertEquals(1, source.closed); + Assert.assertFalse(values.hasNext()); + ((AutoCloseable) values).close(); + Assert.assertEquals(1, source.closed); + } + } + + @Test + public void testCloseBeforeMappedSourceActivation() throws Exception { + CountingIterator source = new CountingIterator(1L); + source.hasNextFailure = new IllegalStateException("Source must not be probed"); + Iterator values = new QueryResults<>(source, queryOf(1L)) + .map(item -> item).iterator(); + ((AutoCloseable) values).close(); + ((AutoCloseable) values).close(); + Assert.assertEquals(0, source.consumed); + Assert.assertEquals(1, source.closed); + Assert.assertFalse(values.hasNext()); + } + @Test public void testMapperExceptionPreservesCloseFailure() { CountingIterator source = new CountingIterator(1L); @@ -449,6 +487,8 @@ private static final class CountingIterator private int consumed; private int closed; private RuntimeException closeFailure; + private RuntimeException hasNextFailure; + private RuntimeException nextFailure; private CountingIterator(Long... values) { this.values = Arrays.asList(values).iterator(); @@ -456,11 +496,17 @@ private CountingIterator(Long... values) { @Override public boolean hasNext() { + if (this.hasNextFailure != null) { + throw this.hasNextFailure; + } return this.values.hasNext(); } @Override public TestIdfiable next() { + if (this.nextFailure != null) { + throw this.nextFailure; + } this.consumed++; return new TestIdfiable(IdGenerator.of(this.values.next())); } From cf40300275039052b93b76a786f95c55a6b92a34 Mon Sep 17 00:00:00 2001 From: contrueCT Date: Tue, 8 Sep 2026 11:18:23 +0800 Subject: [PATCH 6/7] refactor(server): remove unused page result accessors --- .../hugegraph/backend/page/QueryList.java | 18 ++++-------------- .../backend/query/QueryResultContext.java | 2 ++ 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/page/QueryList.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/page/QueryList.java index 059638a8e1..fae009736c 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/page/QueryList.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/page/QueryList.java @@ -197,7 +197,7 @@ public PageResults iterator(int index, String page, long pageSize) { QueryResults fetched = results.toList(); PageState pageState = PageInfo.pageState(results.iterator()); - return new PageResults<>(fetched, query, pageState); + return new PageResults<>(fetched, pageState); } @Override @@ -294,7 +294,7 @@ public PageResults iterator(int index, String page, long pageSize) { IdQuery query = this.indexIdQuery(bindQuery, pageIds.ids(), holder.keepOrder()); QueryResults results = fetcher().apply(query); - return new PageResults<>(results, query, pageIds.pageState()); + return new PageResults<>(results, pageIds.pageState()); } @Override @@ -336,27 +336,17 @@ public static class PageResults { private final QueryResults results; private final PageState pageState; - private final Query query; - public PageResults(QueryResults results, Query query, PageState pageState) { + public PageResults(QueryResults results, PageState pageState) { this.results = results; - this.query = query; this.pageState = pageState; } - public Iterator get() { - return this.results.iterator(); - } - public boolean hasNextPage() { return !Bytes.equals(this.pageState.position(), PageState.EMPTY_BYTES); } - public Query query() { - return this.query; - } - public QueryResults results() { return this.results; } @@ -371,7 +361,7 @@ public long total() { public static PageResults emptyIterator() { // Batch cursors are single-use, including those for empty pages. - return new PageResults<>(QueryResults.empty(), null, PageState.EMPTY); + return new PageResults<>(QueryResults.empty(), PageState.EMPTY); } } } diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/QueryResultContext.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/QueryResultContext.java index 68200795a0..27e6ec67ae 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/QueryResultContext.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/QueryResultContext.java @@ -48,6 +48,8 @@ public QueryResultContext(Query query, boolean inputOrderSatisfied) { ResultsFilter filter = null; OptimizedType optimized = OptimizedType.NONE; Query visibility = query; + // The nearest filter and optimization describe how this batch was fetched; + // the outermost graph condition retains the complete request to match. for (Query current = query; current != null; current = current.originQuery()) { chain.add(current); visibility = current; From 9ad4a27e796187260e45dff63d01905ffa718d43 Mon Sep 17 00:00:00 2001 From: contrueCT Date: Wed, 9 Sep 2026 10:58:38 +0800 Subject: [PATCH 7/7] refactor(server): reduce query batch allocation overhead --- .../backend/cache/CachedGraphTransaction.java | 28 +++++++++++------- .../backend/page/PageEntryIterator.java | 6 ++-- .../hugegraph/backend/query/QueryBatch.java | 29 +++++++++++++++---- .../backend/tx/GraphTransaction.java | 15 +++++----- .../cache/CachedGraphTransactionTest.java | 28 ++++++++++++++++++ .../hugegraph/unit/core/QueryResultsTest.java | 28 ++++++++++++++++++ 6 files changed, 107 insertions(+), 27 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 228298b831..ba197bbb81 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 @@ -374,8 +374,10 @@ protected QueryResults fetchEdgeBatch(Query query) { } Id cacheKey = new QueryId(request); Id batchKey = new QueryId(query); + // An empty label denotes the outer request itself without duplicating its text. + String batchLabel = batchKey.equals(cacheKey) ? "" : batchKey.asString(); CachedEdgeQuery group = new CachedEdgeQuery(this.edgesCache.get(cacheKey)); - Collection cached = group.get(batchKey); + Collection cached = group.get(batchLabel); if (cached != null) { for (HugeEdge edge : cached) { if (edge.expired()) { @@ -390,7 +392,7 @@ protected QueryResults fetchEdgeBatch(Query query) { } QueryResults fetched = super.fetchEdgeBatch(query); if (!fetched.batches().hasNext()) { - this.cacheEdgeBatch(cacheKey, batchKey, Collections.emptyList()); + this.cacheEdgeBatch(cacheKey, batchLabel, Collections.emptyList()); return fetched; } return fetched.mapBatches(batch -> { @@ -401,17 +403,17 @@ protected QueryResults fetchEdgeBatch(Query query) { candidates.add(source.next()); } if (candidates.size() <= MAX_CACHE_EDGES_PER_QUERY) { - this.cacheEdgeBatch(cacheKey, batchKey, candidates); + this.cacheEdgeBatch(cacheKey, batchLabel, candidates); } return new QueryBatch<>( new ExtendableIterator<>(candidates.iterator(), source), batch.context()); }); } - private void cacheEdgeBatch(Id cacheKey, Id batchKey, List candidates) { + private void cacheEdgeBatch(Id cacheKey, String batchLabel, List candidates) { synchronized (this.edgesCache) { - CachedEdgeQuery existing = new CachedEdgeQuery(this.edgesCache.get(cacheKey)); - if (existing.put(batchKey, candidates)) { + CachedEdgeQuery existing = new CachedEdgeQuery(this.edgesCache.get(cacheKey)).copy(); + if (existing.put(batchLabel, candidates)) { this.edgesCache.update(cacheKey, existing.values); } } @@ -426,20 +428,24 @@ private static final class CachedEdgeQuery { @SuppressWarnings("unchecked") private CachedEdgeQuery(Object cached) { this.values = cached == null ? new ArrayList<>() : - new ArrayList<>((List) cached); + (List) cached; + } + + private CachedEdgeQuery copy() { + return new CachedEdgeQuery(new ArrayList<>(this.values)); } @SuppressWarnings("unchecked") - public Collection get(Id batch) { + public Collection get(String batch) { for (int i = 0; i < this.values.size(); i += 2) { - if (this.values.get(i).equals(batch.asString())) { + if (this.values.get(i).equals(batch)) { return (List) this.values.get(i + 1); } } return null; } - public boolean put(Id batch, List candidates) { + public boolean put(String batch, List candidates) { if (this.get(batch) != null) { return false; } @@ -451,7 +457,7 @@ public boolean put(Id batch, List candidates) { this.values.size() / 2 >= MAX_CACHE_EDGES_PER_QUERY) { return false; } - this.values.add(batch.asString()); + this.values.add(batch); this.values.add(new ArrayList<>(candidates)); return true; } diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/page/PageEntryIterator.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/page/PageEntryIterator.java index 84bf174f27..17996bae9e 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/page/PageEntryIterator.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/page/PageEntryIterator.java @@ -20,8 +20,8 @@ import java.util.Iterator; import org.apache.hugegraph.backend.query.Query; -import org.apache.hugegraph.backend.query.QueryBatch.BatchIterator; import org.apache.hugegraph.backend.query.QueryBatch; +import org.apache.hugegraph.backend.query.QueryBatch.BatchIterator; import org.apache.hugegraph.exception.NotSupportException; import org.apache.hugegraph.util.E; @@ -39,8 +39,8 @@ public PageEntryIterator(QueryList queries, long pageSize) { this.pageSize = pageSize; this.pageInfo = PageInfo.fromString(queries.parent().pageWithoutCheck()); E.checkState(this.pageInfo.offset() < queries.total(), - "Invalid page offset '%s' exceeds the size of IdHolderList", - this.pageInfo.offset()); + "Invalid page '%s' with an offset '%s' exceeds the size of IdHolderList", + queries.parent().pageWithoutCheck(), this.pageInfo.offset()); this.remaining = queries.parent().limit(); } diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/QueryBatch.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/QueryBatch.java index e7177b0b6f..f210a503a2 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/QueryBatch.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/QueryBatch.java @@ -18,7 +18,6 @@ package org.apache.hugegraph.backend.query; import java.util.ArrayList; -import java.util.Collections; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -68,11 +67,29 @@ public QueryResultContext context() { } public QueryBatch map(Function mapper) { - return this.flatMap(value -> { - T mapped = mapper.apply(value); - return mapped == null ? Collections.emptyIterator() : - Collections.singleton(mapped).iterator(); - }); + Iterator origin = this.results; + return new QueryBatch<>(new BatchIterator() { + @Override + protected T fetch() { + while (origin.hasNext()) { + T mapped = mapper.apply(origin.next()); + if (mapped != null) { + return mapped; + } + } + return null; + } + + @Override + protected void closeResources() throws Exception { + closeAll(origin); + } + + @Override + public Object metadata(String meta, Object... args) { + return metadataOf(origin, meta, args); + } + }, this.context); } public QueryBatch flatMap(Function> mapper) { diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/tx/GraphTransaction.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/tx/GraphTransaction.java index d011388e8e..3999b3d0c2 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/tx/GraphTransaction.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/tx/GraphTransaction.java @@ -20,7 +20,6 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; -import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; @@ -44,11 +43,11 @@ import org.apache.hugegraph.backend.page.IdHolderList; import org.apache.hugegraph.backend.page.PageInfo; import org.apache.hugegraph.backend.page.QueryList; -import org.apache.hugegraph.backend.query.Aggregate.AggregateFunc; import org.apache.hugegraph.backend.query.Aggregate; +import org.apache.hugegraph.backend.query.Aggregate.AggregateFunc; import org.apache.hugegraph.backend.query.Condition; -import org.apache.hugegraph.backend.query.ConditionQuery.OptimizedType; import org.apache.hugegraph.backend.query.ConditionQuery; +import org.apache.hugegraph.backend.query.ConditionQuery.OptimizedType; import org.apache.hugegraph.backend.query.ConditionQueryFlatten; import org.apache.hugegraph.backend.query.IdQuery; import org.apache.hugegraph.backend.query.Query; @@ -65,9 +64,7 @@ import org.apache.hugegraph.iterator.BatchMapperIterator; import org.apache.hugegraph.iterator.ExtendableIterator; import org.apache.hugegraph.iterator.FilterIterator; -import org.apache.hugegraph.iterator.FlatMapperIterator; import org.apache.hugegraph.iterator.LimitIterator; -import org.apache.hugegraph.iterator.ListIterator; import org.apache.hugegraph.iterator.MapperIterator; import org.apache.hugegraph.job.system.DeleteExpiredJob; import org.apache.hugegraph.perf.PerfUtil.Watched; @@ -102,7 +99,6 @@ import org.apache.tinkerpop.gremlin.util.iterator.IteratorUtils; import com.google.common.collect.ImmutableList; -import com.google.common.collect.Iterators; import jakarta.ws.rs.ForbiddenException; @@ -1110,6 +1106,8 @@ private QueryResults queryEdgeBatchesFromBackend(Query query) { if (flattened.size() == 1) { return fetcher.apply(flattened.get(0)); } + // Each branch is validated when activated, so later branches may fail + // during iteration after earlier branches have already returned results. return QueryResults.flatMap(flattened.iterator(), fetcher); } return this.queryEdgeBatchesFromBackendInternal(query); @@ -1982,8 +1980,11 @@ private boolean rightResultFromIndexQuery(QueryResultContext context, HugeElemen } protected QueryResults filterExpiredBatches(QueryResults batches) { + if (this.storeFeatures().supportsTtl()) { + return batches; + } return batches.filter((context, elem) -> { - if (this.storeFeatures().supportsTtl() || context.showExpired() || !elem.expired()) { + if (context.showExpired() || !elem.expired()) { return true; } DeleteExpiredJob.asyncDeleteExpiredObject(this.graph(), elem); 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 0e7b66f0da..ae0321ee01 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 @@ -33,6 +33,7 @@ import org.apache.hugegraph.HugeGraph; import org.apache.hugegraph.HugeGraphParams; import org.apache.hugegraph.backend.cache.Cache; +import org.apache.hugegraph.backend.cache.CachedBackendStore.QueryId; import org.apache.hugegraph.backend.cache.CachedGraphTransaction; import org.apache.hugegraph.backend.cache.OffheapCache; import org.apache.hugegraph.backend.id.EdgeId; @@ -687,6 +688,33 @@ public void testEdgeBatchCacheRoundTripsThroughOffheap() throws Exception { } } + @Test + public void testRootEdgeBatchUsesCompactLabelAndCopyOnWrite() { + List ids = this.persistEdges(2); + IdQuery root = new IdQuery(HugeType.EDGE, new LinkedHashSet<>(ids.subList(0, 1))); + BackendStore store = Mockito.spy(this.params.loadGraphStore()); + CachedGraphTransaction transaction = new CachedGraphTransaction(this.params, store); + try { + transaction.clearCache(null, false); + this.assertEdgeIds(ids.subList(0, 1), this.fetchEdgeIds(transaction, root)); + Cache cache = Whitebox.getInternalState(transaction, "edgesCache"); + Id key = new QueryId(root); + List original = (List) cache.get(key); + Assert.assertEquals("", original.get(0)); + Assert.assertEquals(2, original.size()); + IdQuery sibling = new IdQuery(root, new LinkedHashSet<>(ids.subList(1, 2))); + this.assertEdgeIds(ids.subList(1, 2), this.fetchEdgeIds(transaction, sibling)); + Assert.assertEquals(2, original.size()); + Assert.assertEquals(4, ((List) cache.get(key)).size()); + Mockito.clearInvocations(store); + this.assertEdgeIds(ids.subList(0, 1), this.fetchEdgeIds(transaction, root)); + this.assertEdgeIds(ids.subList(1, 2), this.fetchEdgeIds(transaction, sibling)); + Mockito.verify(store, Mockito.never()).query(Mockito.any(Query.class)); + } finally { + transaction.close(); + } + } + @Test public void testEdgeCacheKeepsDistinctBatchesWithinTotalCandidateLimit() { List ids = this.persistEdges(101); diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/QueryResultsTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/QueryResultsTest.java index 7521dd447c..e17f5a27b7 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/QueryResultsTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/QueryResultsTest.java @@ -45,6 +45,34 @@ public class QueryResultsTest { + @Test + public void testLaterBranchFailureClosesEarlierBranch() { + CountingIterator source = new CountingIterator(1L); + RuntimeException failure = new IllegalArgumentException("later branch"); + int[] activated = {0}; + QueryResults results = QueryResults.flatMap( + ImmutableList.of(0, 1).iterator(), index -> { + activated[0]++; + if (index == 1) { + throw failure; + } + return new QueryResults<>(source, queryOf(1L)); + }); + Assert.assertEquals(0, activated[0]); + Iterator values = results.iterator(); + Assert.assertEquals(IdGenerator.of(1L), values.next().id()); + Assert.assertEquals(1, activated[0]); + try { + values.hasNext(); + Assert.fail("Expected later branch failure"); + } catch (IllegalArgumentException actual) { + Assert.assertSame(failure, actual); + } + Assert.assertEquals(1, source.closed); + Assert.assertEquals(2, activated[0]); + Assert.assertFalse(values.hasNext()); + } + @Test public void testMaterializationKeepsPageMetadataAfterClosingSource() { PageState page = new PageState(new byte[]{1, 2}, 0, 2);