From 0809e366cfa2c37daaf07a11a61d0328fec20faa Mon Sep 17 00:00:00 2001 From: lvca Date: Fri, 6 Dec 2024 11:08:46 -0500 Subject: [PATCH] perf: supported inverse iteration on the bucket This was a leftover from the beginning of ArcadeDB: the bucket iterator didn't support browsing in reverse order. With ArcadeBrain and a specific query (select from Chat order by @rid desc) was getting very slow, so I dug into it and found out the SQL engine was sorting the records instead of using an inverse iterator at the source. Issue #1854 --- .../main/java/com/arcadedb/engine/Bucket.java | 2 + .../com/arcadedb/engine/BucketIterator.java | 64 ++++++++++++++----- .../java/com/arcadedb/engine/LocalBucket.java | 8 ++- .../FetchFromClusterExecutionStep.java | 30 +++++---- .../executor/FetchFromTypeExecutionStep.java | 2 +- .../sql/executor/SelectExecutionPlanner.java | 41 ++++++------ .../com/arcadedb/remote/RemoteBucket.java | 5 ++ .../com/arcadedb/server/SelectOrderTest.java | 43 +++++++++++++ 8 files changed, 143 insertions(+), 52 deletions(-) diff --git a/engine/src/main/java/com/arcadedb/engine/Bucket.java b/engine/src/main/java/com/arcadedb/engine/Bucket.java index 669e76d54f..e214887e63 100644 --- a/engine/src/main/java/com/arcadedb/engine/Bucket.java +++ b/engine/src/main/java/com/arcadedb/engine/Bucket.java @@ -40,6 +40,8 @@ public interface Bucket { Iterator iterator(); + Iterator inverseIterator(); + long count(); int getFileId(); diff --git a/engine/src/main/java/com/arcadedb/engine/BucketIterator.java b/engine/src/main/java/com/arcadedb/engine/BucketIterator.java index 2951f79a58..b4ebda89de 100644 --- a/engine/src/main/java/com/arcadedb/engine/BucketIterator.java +++ b/engine/src/main/java/com/arcadedb/engine/BucketIterator.java @@ -19,7 +19,6 @@ package com.arcadedb.engine; import com.arcadedb.database.Binary; -import com.arcadedb.database.Database; import com.arcadedb.database.DatabaseInternal; import com.arcadedb.database.RID; import com.arcadedb.database.Record; @@ -39,19 +38,22 @@ public class BucketIterator implements Iterator { final Record[] nextBatch = new Record[PREFETCH_SIZE]; private int prefetchIndex = 0; final long limit; - int nextPageNumber = 0; - BasePage currentPage = null; + int nextPageNumber; + BasePage currentPage = null; short recordCountInCurrentPage; int totalPages; - int currentRecordInPage = 0; - long browsed = 0; - private int writeIndex = 0; + int currentRecordInPage; + long browsed = 0; + private int writeIndex = 0; + private boolean forwardDirection = true; - BucketIterator(final LocalBucket bucket, final Database db) { - ((DatabaseInternal) db).checkPermissionsOnFile(bucket.fileId, SecurityDatabaseUser.ACCESS.READ_RECORD); + BucketIterator(final LocalBucket bucket, final boolean forwardDirection) { + final DatabaseInternal db = bucket.getDatabase(); + db.checkPermissionsOnFile(bucket.fileId, SecurityDatabaseUser.ACCESS.READ_RECORD); - this.database = (DatabaseInternal) db; + this.database = db; this.bucket = bucket; + this.forwardDirection = forwardDirection; this.totalPages = bucket.pageCount.get(); final Integer txPageCounter = database.getTransaction().getPageCounter(bucket.fileId); @@ -59,6 +61,15 @@ public class BucketIterator implements Iterator { this.totalPages = txPageCounter; limit = database.getResultSetLimit(); + + if (forwardDirection) { + currentRecordInPage = 0; + nextPageNumber = 0; + } else { + nextPageNumber = this.totalPages - 1; + currentRecordInPage = Integer.MAX_VALUE; + } + fetchNext(); } @@ -101,15 +112,28 @@ private void fetchNext() { for (writeIndex = 0; writeIndex < nextBatch.length; ) { if (currentPage == null) { - if (nextPageNumber > totalPages) { - return null; + if (forwardDirection) { + // MOVE FORWARD + if (nextPageNumber > totalPages) + return null; + } else { + // MOVE BACKWARDS + if (nextPageNumber < 0) + return null; } + currentPage = database.getTransaction() .getPage(new PageId(database, bucket.file.getFileId(), nextPageNumber), bucket.pageSize); recordCountInCurrentPage = currentPage.readShort(LocalBucket.PAGE_RECORD_COUNT_IN_PAGE_OFFSET); + + if (!forwardDirection && currentRecordInPage == Integer.MAX_VALUE) + currentRecordInPage = recordCountInCurrentPage - 1; } - if (recordCountInCurrentPage > 0 && currentRecordInPage < recordCountInCurrentPage) { + if (recordCountInCurrentPage > 0 && + (forwardDirection && currentRecordInPage < recordCountInCurrentPage) || + (!forwardDirection && currentRecordInPage > -1) + ) { try { final int recordPositionInPage = (int) currentPage.readUnsignedInt( LocalBucket.PAGE_RECORD_TABLE_OFFSET + currentRecordInPage * INT_SERIALIZED_SIZE); @@ -147,15 +171,25 @@ private void fetchNext() { (nextPageNumber * bucket.getMaxRecordsInPage()) + currentRecordInPage, e.getMessage()); LogManager.instance().log(this, Level.SEVERE, msg); } finally { - currentRecordInPage++; + if (forwardDirection) + currentRecordInPage++; + else + currentRecordInPage--; } - } else if (currentRecordInPage == recordCountInCurrentPage) { + } else if (forwardDirection && currentRecordInPage == recordCountInCurrentPage) { currentRecordInPage = 0; currentPage = null; nextPageNumber++; + } else if (!forwardDirection && currentRecordInPage < 0) { + currentRecordInPage = Integer.MAX_VALUE; + currentPage = null; + nextPageNumber--; } else { - currentRecordInPage++; + if (forwardDirection) + currentRecordInPage++; + else + currentRecordInPage--; } } return null; diff --git a/engine/src/main/java/com/arcadedb/engine/LocalBucket.java b/engine/src/main/java/com/arcadedb/engine/LocalBucket.java index 43bf8a4005..cccbbc053d 100644 --- a/engine/src/main/java/com/arcadedb/engine/LocalBucket.java +++ b/engine/src/main/java/com/arcadedb/engine/LocalBucket.java @@ -260,7 +260,13 @@ public void fetchPageInTransaction(final RID rid) throws IOException { @Override public Iterator iterator() { database.checkPermissionsOnFile(fileId, SecurityDatabaseUser.ACCESS.READ_RECORD); - return new BucketIterator(this, database); + return new BucketIterator(this, true); + } + + @Override + public Iterator inverseIterator() { + database.checkPermissionsOnFile(fileId, SecurityDatabaseUser.ACCESS.READ_RECORD); + return new BucketIterator(this, false); } @Override diff --git a/engine/src/main/java/com/arcadedb/query/sql/executor/FetchFromClusterExecutionStep.java b/engine/src/main/java/com/arcadedb/query/sql/executor/FetchFromClusterExecutionStep.java index 7757057436..f671a0122a 100755 --- a/engine/src/main/java/com/arcadedb/query/sql/executor/FetchFromClusterExecutionStep.java +++ b/engine/src/main/java/com/arcadedb/query/sql/executor/FetchFromClusterExecutionStep.java @@ -38,13 +38,15 @@ public class FetchFromClusterExecutionStep extends AbstractExecutionStep { private Iterator iterator; public FetchFromClusterExecutionStep(final int bucketId, final CommandContext context) { - this(bucketId, null, context); + this(bucketId, null, null, context); } - public FetchFromClusterExecutionStep(final int bucketId, final QueryPlanningInfo queryPlanning, final CommandContext context) { + public FetchFromClusterExecutionStep(final int bucketId, final QueryPlanningInfo queryPlanning, final Object order, + final CommandContext context) { super(context); this.bucketId = bucketId; this.queryPlanning = queryPlanning; + this.order = order; } @Override @@ -53,16 +55,16 @@ public ResultSet syncPull(final CommandContext context, final int nRecords) thro final long begin = context.isProfiling() ? System.nanoTime() : 0; try { if (iterator == null) { - iterator = context.getDatabase().getSchema().getBucketById(bucketId).iterator(); + if (order == ORDER_DESC) + iterator = context.getDatabase().getSchema().getBucketById(bucketId).inverseIterator(); + else + iterator = context.getDatabase().getSchema().getBucketById(bucketId).iterator(); - //TODO check how to support ranges and DESC + //TODO check how to support ranges // long minClusterPosition = calculateMinClusterPosition(); // long maxClusterPosition = calculateMaxClusterPosition(); // new ORecordIteratorCluster((ODatabaseDocumentInternal) context.getDatabase(), // (ODatabaseDocumentInternal) context.getDatabase(), bucketId, minClusterPosition, maxClusterPosition); -// if (ORDER_DESC == order) { -// iterator.last(); -// } } return new ResultSet() { int nFetched = 0; @@ -81,7 +83,7 @@ public boolean hasNext() { return iterator.hasNext(); // } } finally { - if( context.isProfiling() ) { + if (context.isProfiling()) { cost += (System.nanoTime() - begin1); } } @@ -116,14 +118,14 @@ record = iterator.next(); return result; } finally { - if( context.isProfiling() ) { + if (context.isProfiling()) { cost += (System.nanoTime() - begin1); } } } }; } finally { - if( context.isProfiling() ) { + if (context.isProfiling()) { cost += (System.nanoTime() - begin); } } @@ -192,9 +194,10 @@ record = iterator.next(); @Override public String prettyPrint(final int depth, final int indent) { String result = - ExecutionStepInternal.getIndent(depth, indent) + "+ FETCH FROM BUCKET " + bucketId + " (" + context.getDatabase().getSchema().getBucketById(bucketId) + ExecutionStepInternal.getIndent(depth, indent) + "+ FETCH FROM BUCKET " + bucketId + " (" + context.getDatabase() + .getSchema().getBucketById(bucketId) .getName() + ") " + (ORDER_DESC.equals(order) ? "DESC" : "ASC" + " = " + totalFetched + " RECORDS"); - if ( context.isProfiling() ) + if (context.isProfiling()) result += " (" + getCostFormatted() + ")"; return result; } @@ -210,6 +213,7 @@ public boolean canBeCached() { @Override public ExecutionStep copy(final CommandContext context) { - return new FetchFromClusterExecutionStep(this.bucketId, this.queryPlanning == null ? null : this.queryPlanning.copy(), context); + return new FetchFromClusterExecutionStep(this.bucketId, this.queryPlanning == null ? null : this.queryPlanning.copy(), + this.order, context); } } diff --git a/engine/src/main/java/com/arcadedb/query/sql/executor/FetchFromTypeExecutionStep.java b/engine/src/main/java/com/arcadedb/query/sql/executor/FetchFromTypeExecutionStep.java index 38aff34fc8..4ff1f80dc1 100755 --- a/engine/src/main/java/com/arcadedb/query/sql/executor/FetchFromTypeExecutionStep.java +++ b/engine/src/main/java/com/arcadedb/query/sql/executor/FetchFromTypeExecutionStep.java @@ -122,7 +122,7 @@ else if (Boolean.FALSE.equals(ridOrder)) sortBuckets(bucketIds); for (final int bucketId : bucketIds) { if (bucketId > 0) { - final FetchFromClusterExecutionStep step = new FetchFromClusterExecutionStep(bucketId, planningInfo, context); + final FetchFromClusterExecutionStep step = new FetchFromClusterExecutionStep(bucketId, planningInfo, null, context); if (orderByRidAsc) step.setOrder(FetchFromClusterExecutionStep.ORDER_ASC); else if (orderByRidDesc) diff --git a/engine/src/main/java/com/arcadedb/query/sql/executor/SelectExecutionPlanner.java b/engine/src/main/java/com/arcadedb/query/sql/executor/SelectExecutionPlanner.java index b79f50bfbd..78856fb27d 100644 --- a/engine/src/main/java/com/arcadedb/query/sql/executor/SelectExecutionPlanner.java +++ b/engine/src/main/java/com/arcadedb/query/sql/executor/SelectExecutionPlanner.java @@ -1273,13 +1273,12 @@ private void handleClassAsTarget(final SelectExecutionPlan plan, final Set iterator() { throw new UnsupportedOperationException(); } + @Override + public Iterator inverseIterator() { + throw new UnsupportedOperationException(); + } + @Override public long count() { throw new UnsupportedOperationException(); diff --git a/server/src/test/java/com/arcadedb/server/SelectOrderTest.java b/server/src/test/java/com/arcadedb/server/SelectOrderTest.java index 9cac4781f8..2cdf244a6b 100644 --- a/server/src/test/java/com/arcadedb/server/SelectOrderTest.java +++ b/server/src/test/java/com/arcadedb/server/SelectOrderTest.java @@ -27,6 +27,8 @@ import com.arcadedb.database.DatabaseFactory; import com.arcadedb.database.RID; import com.arcadedb.engine.Bucket; +import com.arcadedb.graph.MutableVertex; +import com.arcadedb.graph.Vertex; import com.arcadedb.integration.misc.IntegrationUtils; import com.arcadedb.query.sql.executor.Result; import com.arcadedb.query.sql.executor.ResultSet; @@ -282,6 +284,47 @@ public void testLocalDateTimeOrderBy() { } } + @Test + public void testRIDOrderingDesc() { + final ContextConfiguration serverConfiguration = new ContextConfiguration(); + final String rootPath = IntegrationUtils.setRootPath(serverConfiguration); + + try (DatabaseFactory databaseFactory = new DatabaseFactory(rootPath + "/databases/" + DATABASE_NAME)) { + if (databaseFactory.exists()) + databaseFactory.open().drop(); + + try (Database db = databaseFactory.create()) { + db.transaction(() -> { + DocumentType dtProduct = db.getSchema().createVertexType("Product", 1); + dtProduct.createProperty("name", Type.STRING); + }); + + db.transaction(() -> { + for (int i = 0; i < 1_000; i++) { + final MutableVertex v = db.newVertex("Product"); + v.set("id", i); + v.save(); + } + + for (int i = 0; i < 10; i++) { + int last = Integer.MAX_VALUE; + int total = 0; + final ResultSet resultset = db.query("sql", "select from Product order by @rid desc"); + while (resultset.hasNext()) { + final Vertex v = resultset.next().getVertex().get(); + final Integer id = v.getInteger("id"); + assertThat(id).isLessThan(last); + + last = id; + ++total; + } + assertThat(total).isEqualTo(1_000); + } + }); + } + } + } + @BeforeEach public void beginTests() { final ContextConfiguration serverConfiguration = new ContextConfiguration();