Skip to content

Commit abbbdbc

Browse files
committed
[bugfix] Make sure that the REST Server correctly caches and frees query results in its Session
1 parent 1f3c011 commit abbbdbc

2 files changed

Lines changed: 24 additions & 8 deletions

File tree

exist-core/src/main/java/org/exist/http/RESTServer.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1387,7 +1387,9 @@ protected void search(final DBBroker broker, final Txn transaction, final String
13871387
@Nullable final Item contextItem = extractContextItem(contextItemParam);
13881388
final Sequence contextSequence = contextItem != null ? new ValueSequence(contextItem) : null;
13891389

1390-
try (final XQueryUtil.QueryResult queryResult = XQueryUtil.query(broker, source, true, contextSequence, outputProperties, setupXqueryContextPreCompilation, setupXqueryContextPreExecution, setupXqueryContextPostExecution)) {
1390+
@Nullable XQueryUtil.QueryResult queryResult = null;
1391+
try {
1392+
queryResult = XQueryUtil.query(broker, source, true, contextSequence, outputProperties, setupXqueryContextPreCompilation, setupXqueryContextPreExecution, setupXqueryContextPostExecution);
13911393

13921394
// special header to indicate that the query is not returned from cache
13931395
response.setHeader(XQUERY_CACHED_RESPONSE_HEADER, queryResult.compilationTime == XQueryUtil.CompilationResult.RETRIEVED_CACHED_COMPILED_QUERY ? "true" : "false");
@@ -1405,6 +1407,11 @@ protected void search(final DBBroker broker, final Txn transaction, final String
14051407
}
14061408

14071409
writeResults(response, broker, transaction, queryResult, howmany, start, typed, outputProperties, wrap);
1410+
} finally {
1411+
if (!cache && queryResult != null) {
1412+
// NOTE(AR) we can only close the query result if we are not caching it for reuse in the sessionManager, otherwise it has to be closed when it is later removed from the sessionManager
1413+
queryResult.close();
1414+
}
14081415
}
14091416

14101417
} catch (final IOException e) {

exist-core/src/main/java/org/exist/http/SessionManager.java

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,13 @@
4747

4848
import com.github.benmanes.caffeine.cache.Cache;
4949
import com.github.benmanes.caffeine.cache.Caffeine;
50+
import com.github.benmanes.caffeine.cache.RemovalListener;
5051
import net.jcip.annotations.ThreadSafe;
5152
import org.apache.logging.log4j.LogManager;
5253
import org.apache.logging.log4j.Logger;
5354
import org.exist.xquery.XQueryUtil;
5455

56+
import javax.annotation.Nullable;
5557
import java.util.concurrent.TimeUnit;
5658
import java.util.concurrent.atomic.AtomicInteger;
5759

@@ -64,6 +66,15 @@ public class SessionManager {
6466
private static final Logger LOG = LogManager.getLogger(SessionManager.class);
6567
private static final long TIMEOUT = 120_000; // ms (e.g. 2 minutes)
6668

69+
private static final RemovalListener<Integer, QueryAndResult> REMOVAL_LISTENER = (sessionId, queryAndResult, removalCause) -> {
70+
if (LOG.isDebugEnabled()) {
71+
LOG.debug("Removing cached query result for session: {}", sessionId);
72+
}
73+
74+
// NOTE(AR) make sure to release any resources still held by the query result and potentially send it back to the query pool for reuse
75+
queryAndResult.result.close();
76+
};
77+
6778
private final AtomicInteger sessionIdCounter = new AtomicInteger();
6879
private final Cache<Integer, QueryAndResult> cache;
6980

@@ -78,12 +89,10 @@ private QueryAndResult(final String query, final XQueryUtil.QueryResult result)
7889
}
7990

8091
public SessionManager() {
81-
final Caffeine<Object, Object> cacheBuilder = Caffeine.newBuilder()
82-
.expireAfterAccess(TIMEOUT, TimeUnit.MILLISECONDS);
83-
if(LOG.isDebugEnabled()) {
84-
cacheBuilder.removalListener((key, value, cause) -> LOG.debug("Removing cached query result for session: {}", key));
85-
}
86-
cache = cacheBuilder.build();
92+
this.cache = Caffeine.newBuilder()
93+
.expireAfterAccess(TIMEOUT, TimeUnit.MILLISECONDS)
94+
.removalListener(REMOVAL_LISTENER)
95+
.build();
8796
}
8897

8998
public int add(final String query, final XQueryUtil.QueryResult result) {
@@ -92,7 +101,7 @@ public int add(final String query, final XQueryUtil.QueryResult result) {
92101
return sessionId;
93102
}
94103

95-
public XQueryUtil.QueryResult get(final String query, final int sessionId) {
104+
public @Nullable XQueryUtil.QueryResult get(final String query, final int sessionId) {
96105
if (sessionId < 0 || sessionId >= sessionIdCounter.get()) {
97106
return null; // out of scope
98107
}

0 commit comments

Comments
 (0)