Merge pull request #5191 from reinhapa/in-memory-rpcfiles-6.x.x

[6.x.x] Adds in memory cache for RPC query results

Author: adamretter

.github/workflows/ci-deploy.yml

@@ -11,7 +11,7 @@ jobs:
         with:
           fetch-depth: 1
       - name: Set up JDK 8
-        uses: actions/setup-java@v3
+        uses: actions/setup-java@v4
         with:
           distribution: temurin
           java-version: '8'
@@ -21,7 +21,7 @@ jobs:
         with:
           install: true
       - name: Cache Maven packages
-        uses: actions/cache@v3
+        uses: actions/cache@v4
         with:
           path: ~/.m2
           key: deploy-${{ runner.os }}-maven-${{ hashFiles('**/pom.xml') }}

.github/workflows/ci-test.yml

@@ -11,7 +11,7 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v4
-      - uses: actions/setup-java@v3
+      - uses: actions/setup-java@v4
         with:
           distribution: temurin
           java-version: ${{ env.DEV_JDK }}
@@ -23,12 +23,15 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v4
-      - uses: actions/setup-java@v3
+      - uses: actions/setup-java@v4
         with:
           distribution: temurin
           java-version: ${{ env.DEV_JDK }}
           cache: 'maven'
-      - run: mvn -V -B dependency-check:check
+      - name: OWASP dependency check
+        env:
+          NVD_API_KEY: ${{ secrets.NVD_API_KEY }}
+        run: mvn -V -B dependency-check:check
         timeout-minutes: 60
   test:
     name: ${{ matrix.os }} Test
@@ -40,7 +43,7 @@ jobs:
     steps:
       - uses: actions/checkout@v4
       - name: Set up JDK
-        uses: actions/setup-java@v3
+        uses: actions/setup-java@v4
         with:
           distribution: temurin
           java-version: ${{ env.DEV_JDK }}
@@ -72,7 +75,7 @@ jobs:
         run: ${{ steps.install-mvnd.outputs.mvnd-dir }}/mvnd -V -B jacoco:report coveralls:report
       - name: Archive build logs
         if: always()
-        uses: actions/upload-artifact@v3
+        uses: actions/upload-artifact@v4
         with:
           name: ${{ runner.os }}-build-logs
           retention-days: 5

.github/workflows/ci-xqts.yml

@@ -10,12 +10,12 @@ jobs:
     steps:
       - uses: actions/checkout@v4
       - name: Set up JDK 8
-        uses: actions/setup-java@v3
+        uses: actions/setup-java@v4
         with:
           distribution: temurin
           java-version: '8'
       - name: Cache Maven packages
-        uses: actions/cache@v3
+        uses: actions/cache@v4
         with:
           path: ~/.m2
           key: xqts-${{ runner.os }}-maven-${{ hashFiles('**/pom.xml') }}
@@ -29,22 +29,22 @@ jobs:
         run: find exist-xqts/target -name exist-xqts-runner.sh -exec {} --xqts-version HEAD --output-dir /tmp/xqts-output --exclude-test-case RangeExpr-411d,RangeExpr-409d,RangeExpr-408d,RangeExpr-409c,RangeExpr-408c,GenCompEq-21 \;
       - name: Check for HeapDump
         id: check_heapdump
-        uses: andstor/file-existence-action@v2
+        uses: andstor/file-existence-action@v3
         with:
           files: "/tmp/*.hprof"
       - name: Compress HeapDump
         if: steps.check_heapdump.outputs.files_exists == 'true'
         run: zstd --rm -9 --progress -T0 /tmp/*.hprof
       - name: Attach HeapDump artifact
         if: steps.check_heapdump.outputs.files_exists == 'true'
-        uses: actions/upload-artifact@v3
+        uses: actions/upload-artifact@v4
         with:
           name: exist-xqts-runner-hprof
           retention-days: 1
           path: /tmp/*.hprof.zst
       - name: Archive XQTS Logs
         if: always()
-        uses: actions/upload-artifact@v3
+        uses: actions/upload-artifact@v4
         with:
           name: xqts-logs
           retention-days: 14

exist-core/pom.xml

@@ -514,6 +514,11 @@
             <artifactId>assertj-core</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.awaitility</groupId>
+            <artifactId>awaitility</artifactId>
+            <scope>test</scope>
+        </dependency>
         <dependency>
             <groupId>org.easymock</groupId>
             <artifactId>easymock</artifactId>

exist-core/src/main/java/org/exist/util/io/ContentFilePool.java

@@ -26,6 +26,8 @@
 import org.apache.commons.pool2.impl.GenericObjectPoolConfig;
 import org.exist.util.Configuration;
 
+import javax.annotation.Nullable;
+
 /**
  * Generic pool for {@link ContentFile} instances used to represent RPC server data up to a defined size in memory first
  * before storing the data in the file system using the {@link TemporaryFileManager}.
@@ -50,23 +52,25 @@ public final class ContentFilePool extends GenericObjectPool<ContentFile> {
      * Creates a new pool using the givem temporary file manager, configuration and maximum idle time.
      *
      * @param tempFileManager the temporary file manager used when need to swap out data bigger than max in memory size
+     * @param brokerId the optional broker id
      * @param config the configuration used to configure the main pool properties
      */
-    public ContentFilePool(final TemporaryFileManager tempFileManager, final Configuration config) {
-        super(new ContentFilePoolObjectFactory(tempFileManager, toInMemorySize(config)), toPoolConfig(config));
+    public ContentFilePool(final TemporaryFileManager tempFileManager, @Nullable final String brokerId, final Configuration config) {
+        super(new ContentFilePoolObjectFactory(tempFileManager, toInMemorySize(config)), toPoolConfig(brokerId, config));
     }
 
     private static int toInMemorySize(final Configuration config) {
         return config.getInteger(PROPERTY_IN_MEMORY_SIZE, VirtualTempPath.DEFAULT_IN_MEMORY_SIZE);
     }
 
-    private static GenericObjectPoolConfig<ContentFile> toPoolConfig(final Configuration config) {
+    private static GenericObjectPoolConfig<ContentFile> toPoolConfig(@Nullable final String brokerId, final Configuration config) {
         final GenericObjectPoolConfig<ContentFile> poolConfig = new GenericObjectPoolConfig<>();
         poolConfig.setBlockWhenExhausted(false);
         poolConfig.setLifo(true);
         poolConfig.setMaxIdle(config.getInteger(PROPERTY_POOL_MAX_IDLE));
         poolConfig.setMaxTotal(config.getInteger(PROPERTY_POOL_SIZE));
-        poolConfig.setJmxNameBase("org.exist.management.exist:type=ContentFilePool");
+        final String poolName = brokerId == null ? "" : "pool." + brokerId;
+        poolConfig.setJmxNameBase("org.exist.management.exist:type=ContentFilePool,name=" + poolName);
         return poolConfig;
     }
 

exist-core/src/main/java/org/exist/util/io/MemoryContentsInputStream.java

@@ -26,7 +26,6 @@
 
 import java.io.IOException;
 import java.io.InputStream;
-import java.io.OutputStream;
 import java.util.concurrent.atomic.AtomicLongFieldUpdater;
 
 /**
@@ -84,7 +83,7 @@ public int read() throws IOException {
     public int read(byte[] b, int off, int len) throws IOException {
         boolean success = false;
         int read = 0;
-        while (!success) {
+        while (!success && len > 0) {
             long positionBefore = POSITION_UPDATER.get(this);
             read = this.memoryContents.read(b, positionBefore, off, len);
             if (read < 1) {
@@ -115,13 +114,4 @@ public long skip(long n) throws IOException {
         }
         return skipped;
     }
-
-    // Java 9 method, has to compile under Java 1.7 so no @Override
-    public long transferTo(OutputStream out) throws IOException {
-        long positionBefore = POSITION_UPDATER.get(this);
-        long written = this.memoryContents.transferTo(out, positionBefore);
-        POSITION_UPDATER.set(this, this.memoryContents.size());
-        return written;
-    }
-
 }

exist-core/src/main/java/org/exist/util/io/VirtualTempPath.java

@@ -52,6 +52,10 @@ public final class VirtualTempPath implements ContentFile {
     @GuardedBy("lock")
     private Path contentFile;
 
+    public VirtualTempPath(TemporaryFileManager tempFileManager) {
+        this(DEFAULT_IN_MEMORY_SIZE, tempFileManager);
+    }
+
     public VirtualTempPath(int inMemorySize, TemporaryFileManager tempFileManager) {
         this.inMemorySize = inMemorySize;
         this.lock = new StampedLock();
@@ -97,6 +101,7 @@ public OutputStream newOutputStream() throws IOException {
         }
     }
 
+    @Override
     public InputStream newInputStream() throws IOException {
         long stamp = lock.readLock();
         try {
@@ -129,6 +134,7 @@ public void close() {
         }
     }
 
+    @Override
     public long size() {
         long stamp = lock.readLock();
         try {
@@ -141,6 +147,7 @@ public long size() {
         }
     }
 
+    @Override
     public byte[] getBytes() {
         long stamp = lock.readLock();
         try {

exist-core/src/main/java/org/exist/xmlrpc/CachedContentFile.java

@@ -0,0 +1,53 @@
+/*
+ * eXist-db Open Source Native XML Database
+ * Copyright (C) 2001 The eXist-db Authors
+ *
+ * [email protected]
+ * http://www.exist-db.org
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+package org.exist.xmlrpc;
+
+import org.exist.util.io.ContentFile;
+
+import java.util.function.Consumer;
+
+/**
+ * @author <a href="mailto:[email protected]">Patrick Reinhart</a>
+ */
+final class CachedContentFile extends AbstractCachedResult {
+    private final ContentFile result;
+    private final Consumer<ContentFile> poolConsumer;
+
+    CachedContentFile(final ContentFile result, final Consumer<ContentFile> poolConsumer) {
+        super(0);
+        this.result = result;
+        this.poolConsumer = poolConsumer;
+    }
+
+    @Override
+    public ContentFile getResult() {
+        return result;
+    }
+
+    @Override
+    protected void doClose() {
+        if (result != null) {
+            poolConsumer.accept(result);
+            result.close();
+        }
+    }
+}

exist-core/src/main/java/org/exist/xmlrpc/QueryResultCache.java

@@ -51,9 +51,9 @@ public QueryResultCache() {
                 .expireAfterAccess(TIMEOUT, TimeUnit.MILLISECONDS)
                 .removalListener((key, value, cause) -> {
                     final AbstractCachedResult qr = (AbstractCachedResult)value;
-                    qr.free();  // must free associated resources
+                    qr.close();  // must close associated resources
                     if(LOG.isDebugEnabled()) {
-                        LOG.debug("Removing cached result set: {}", new Date(qr.getTimestamp()).toString());
+                        LOG.debug("Removing cached result set: {}", new Date(qr.getTimestamp()));
                     }
                 }).build();
     }
@@ -81,6 +81,11 @@ public SerializedResult getSerializedResult(final int cacheId) {
         return (acr != null && acr instanceof SerializedResult) ? (SerializedResult) acr : null;
     }
 
+    public CachedContentFile getCachedContentFile(final int cacheId) {
+        final AbstractCachedResult acr = get(cacheId);
+        return (acr != null && acr instanceof CachedContentFile) ? (CachedContentFile) acr : null;
+    }
+
     public void remove(final int cacheId) {
         if (cacheId < 0 || cacheId >= cacheIdCounter.get()) {
             return; // out of scope

exist-core/src/main/java/org/exist/xmlrpc/RpcAPI.java

@@ -112,7 +112,7 @@ public interface RpcAPI {
     /**
      * Retrieve document by name. XML content is indented if prettyPrint is set
      * to >=0. Use supplied encoding for output.
-     *
+     * <p>
      * This method is provided to retrieve a document with encodings other than
      * UTF-8. Since the data is handled as binary data, character encodings are
      * preserved. byte[]-values are automatically BASE64-encoded by the XMLRPC
@@ -132,7 +132,7 @@ byte[] getDocument(String name, String encoding, int prettyPrint)
      * Retrieve document by name. XML content is indented if prettyPrint is set
      * to &gt;=0. Use supplied encoding for output and apply the specified
      * stylesheet.
-     *
+     * <p>
      * This method is provided to retrieve a document with encodings other than
      * UTF-8. Since the data is handled as binary data, character encodings are
      * preserved. byte[]-values are automatically BASE64-encoded by the XMLRPC
@@ -152,7 +152,7 @@ byte[] getDocument(String name, String encoding, int prettyPrint, String stylesh
     /**
      * Retrieve document by name. All optional output parameters are passed as
      * key/value pairs int the <code>parameters</code>.
-     *
+     * <p>
      * Valid keys may either be taken from
      * {@link javax.xml.transform.OutputKeys} or
      * {@link org.exist.storage.serializers.EXistOutputKeys}. For example, the
@@ -247,10 +247,10 @@ List<String> getCollectionListing(final String collection)
     List<String> getDocumentListing(String collection)
             throws EXistException, PermissionDeniedException, URISyntaxException;
 
-    Map<String, List> listDocumentPermissions(String name)
+    Map<String, List<Object>> listDocumentPermissions(String name)
             throws EXistException, PermissionDeniedException, URISyntaxException;
 
-    Map<XmldbURI, List> listCollectionPermissions(String name)
+    Map<String, List<Object>> listCollectionPermissions(String name)
             throws EXistException, PermissionDeniedException, URISyntaxException;
 
     /**
@@ -497,6 +497,7 @@ byte[] query(
      * @throws PermissionDeniedException If the current user is not allowed to perform this action
      * @deprecated use List query() or int executeQuery() instead
      */
+    @Deprecated
     Map<String, Object> querySummary(String xquery)
             throws EXistException, PermissionDeniedException;
 
@@ -612,7 +613,7 @@ String uploadCompressed(String file, byte[] data, int length)
 
     /**
      * Parse a file previously uploaded with upload.
-     *
+     * <p>
      * The temporary file will be removed.
      *
      * @param localFile temporary file name
@@ -749,6 +750,7 @@ Map<String, Object> execute(String path, Map<String, Object> parameters)
      *
      * @deprecated Use {@link #executeT(String, Map)} instead.
      */
+    @Deprecated
     Map<String, Object> executeT(String path, Map<String, Object> parameters)
             throws EXistException, PermissionDeniedException;
 
@@ -926,7 +928,7 @@ String hasUserLock(String path)
 
     List<String> getGroups() throws EXistException, PermissionDeniedException;
 
-    List<List> getIndexedElements(String collectionName, boolean inclusive)
+    List<List<Object>> getIndexedElements(String collectionName, boolean inclusive)
             throws EXistException, PermissionDeniedException, URISyntaxException;
 
     boolean releaseQueryResult(int handle);