#3515 Bug in QueryCache invalidation

Effectively the bug fix is in SqlTreeNodeRoot.dependentTables() to include the baseTable in the set of dependent tables
This commit is contained in:
Rob Bygrave
2025-03-20 23:47:21 +13:00
parent 3ab3f81fa3
commit eb6906ad2b
11 changed files with 41 additions and 29 deletions
@@ -1,5 +1,6 @@
package io.ebean.cache;
import java.time.Instant;
import java.util.Set;
/**
@@ -14,7 +15,7 @@ public class QueryCacheEntry {
private final Object value;
private final Set<String> dependentTables;
private final long timestamp;
private final Instant timestamp;
/**
* Create with dependent tables and timestamp.
@@ -23,7 +24,7 @@ public class QueryCacheEntry {
* @param dependentTables The extra tables the query is dependent on (joins to)
* @param timestamp The timestamp that the query uses to check for modifications
*/
public QueryCacheEntry(Object value, Set<String> dependentTables, long timestamp) {
public QueryCacheEntry(Object value, Set<String> dependentTables, Instant timestamp) {
this.value = value;
this.dependentTables = dependentTables;
this.timestamp = timestamp;
@@ -46,7 +47,7 @@ public class QueryCacheEntry {
/**
* Return the timestamp used to check for modifications on the dependent tables.
*/
public long getTimestamp() {
public Instant getTimestamp() {
return timestamp;
}
}
@@ -14,6 +14,7 @@ import io.ebeanservice.docstore.api.DocStoreTransaction;
import jakarta.persistence.PersistenceException;
import java.sql.Connection;
import java.sql.SQLException;
import java.time.Instant;
/**
* Extends Transaction with additional API required on server.
@@ -95,7 +96,7 @@ public interface SpiTransaction extends Transaction {
/**
* Return the start timestamp for the transaction (JVM side).
*/
long startNanoTime();
Instant startTime();
/**
* Return true if this transaction has updateAllLoadedProperties set.
@@ -14,6 +14,7 @@ import io.ebeanservice.docstore.api.DocStoreTransaction;
import jakarta.persistence.PersistenceException;
import java.sql.Connection;
import java.sql.SQLException;
import java.time.Instant;
/**
* Proxy for an underlying SpiTransaction (most of the API).
@@ -28,8 +29,8 @@ public abstract class SpiTransactionProxy implements SpiTransaction {
}
@Override
public long startNanoTime() {
return transaction.startNanoTime();
public Instant startTime() {
return transaction.startTime();
}
@Override
@@ -685,7 +685,7 @@ public final class OrmQueryRequest<T> extends BeanRequest implements SpiOrmQuery
}
public void putToQueryCache(Object result) {
beanDescriptor.queryCachePut(cacheKey, new QueryCacheEntry(result, dependentTables, transaction.startNanoTime()));
beanDescriptor.queryCachePut(cacheKey, new QueryCacheEntry(result, dependentTables, transaction.startTime()));
}
/**
@@ -76,6 +76,7 @@ final class SqlTreeNodeRoot extends SqlTreeNodeBean {
@Override
public void dependentTables(Set<String> tables) {
tables.add(baseTable);
for (SqlTreeNode child : children) {
child.dependentTables(tables);
}
@@ -14,6 +14,7 @@ import io.ebeanservice.docstore.api.DocStoreTransaction;
import jakarta.persistence.PersistenceException;
import java.sql.Connection;
import java.sql.SQLException;
import java.time.Instant;
import java.util.HashMap;
import java.util.Map;
@@ -58,6 +59,7 @@ final class ImplicitReadOnlyTransaction implements SpiTransaction, TxnProfileEve
private SpiPersistenceContext persistenceContext;
private Object tenantId;
private Map<String, Object> userObjects;
private final Instant startTime = Instant.now();
private final long startNanos;
private ProfileLocation profileLocation;
@@ -89,9 +91,8 @@ final class ImplicitReadOnlyTransaction implements SpiTransaction, TxnProfileEve
}
@Override
public long startNanoTime() {
// not used on read only transaction
return startNanos;
public Instant startTime() {
return startTime;
}
@Override
@@ -17,6 +17,7 @@ import jakarta.persistence.PersistenceException;
import jakarta.persistence.RollbackException;
import java.sql.Connection;
import java.sql.SQLException;
import java.time.Instant;
import java.util.*;
import java.util.function.Consumer;
@@ -91,6 +92,7 @@ class JdbcTransaction implements SpiTransaction, TxnProfileEventCodes {
DocStoreTransaction docStoreTxn;
private ProfileStream profileStream;
private ProfileLocation profileLocation;
private final Instant startTime = Instant.now();
private final long startNanos;
private boolean autoPersistUpdates;
@@ -141,8 +143,8 @@ class JdbcTransaction implements SpiTransaction, TxnProfileEventCodes {
}
@Override
public final long startNanoTime() {
return startNanos;
public final Instant startTime() {
return startTime;
}
@Override
@@ -17,6 +17,7 @@ import io.ebeanservice.docstore.api.DocStoreTransaction;
import jakarta.persistence.PersistenceException;
import java.sql.Connection;
import java.sql.SQLException;
import java.time.Instant;
/**
* Placeholder for use with SUPPORTS and NEVER transactional when there really isn't a transaction.
@@ -56,9 +57,9 @@ final class NoTransaction implements SpiTransaction {
}
@Override
public long startNanoTime() {
public Instant startTime() {
// not used
return System.nanoTime();
return Instant.now();
}
@Override
@@ -6,6 +6,7 @@ import io.ebean.cache.QueryCacheEntryValidate;
import io.ebean.cache.ServerCacheNotification;
import io.ebean.cache.ServerCacheNotify;
import java.time.Instant;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
@@ -23,7 +24,7 @@ public final class TableModState implements QueryCacheEntryValidate, ServerCache
private static final System.Logger log = AppLog.getLogger("io.ebean.cache.TABLEMOD");
private final Map<String, Long> tableModStamp = new ConcurrentHashMap<>();
private final Map<String, Instant> tableModStamp = new ConcurrentHashMap<>();
public TableModState() {
}
@@ -32,22 +33,22 @@ public final class TableModState implements QueryCacheEntryValidate, ServerCache
* Set the modified timestamp on the tables that have been touched.
*/
void touch(Set<String> touchedTables) {
long modNanoTime = System.nanoTime();
final var modTime = Instant.now();
for (String tableName : touchedTables) {
tableModStamp.put(tableName, modNanoTime);
tableModStamp.put(tableName, modTime);
}
if (log.isLoggable(DEBUG)) {
log.log(DEBUG, "TableModState updated - touched:{0} modNanoTime:{1}", touchedTables, modNanoTime);
log.log(DEBUG, "TableModState updated - touched:{0} modTime:{1}", touchedTables, modTime);
}
}
/**
* Return true if all the tables are valid based on timestamp comparison.
*/
boolean isValid(Set<String> tables, long sinceNanoTime) {
boolean isValid(Set<String> tables, Instant sinceTime) {
for (String tableName : tables) {
Long modTime = tableModStamp.get(tableName);
if (modTime != null && modTime >= sinceNanoTime) {
final var modTime = tableModStamp.get(tableName);
if (modTime != null && !modTime.isBefore(sinceTime)) {
if (log.isLoggable(TRACE)) {
log.log(TRACE, "Invalidate on table:{0}", tableName);
}
@@ -2,6 +2,7 @@ package io.ebeaninternal.server.transaction;
import org.junit.jupiter.api.Test;
import java.time.Instant;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;
@@ -9,24 +10,23 @@ import java.util.Set;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
public class TableModStateTest {
class TableModStateTest {
private TableModState tableModState = new TableModState();
private final TableModState tableModState = new TableModState();
@Test
public void isValid() {
long before = System.nanoTime();
void isValid() {
Instant before = Instant.now();
tableModState.touch(setOf("one", "two", "three"));
long after = System.nanoTime();
Instant after = Instant.now();
// empty
assertTrue(tableModState.isValid(Collections.emptySet(), 12L));
assertTrue(tableModState.isValid(Collections.emptySet(), before));
// no entry
assertTrue(tableModState.isValid(setOf("noEntry"), 12L));
assertTrue(tableModState.isValid(setOf("noEntry"), before));
// later timestamp
assertTrue(tableModState.isValid(setOf("one"), after));
@@ -1,7 +1,9 @@
package org.tests.cache;
import io.ebean.DB;
import io.ebean.annotation.Platform;
import io.ebean.xtest.BaseTestCase;
import io.ebean.xtest.ForPlatform;
import org.junit.jupiter.api.Test;
import org.tests.model.cache.EColAB;
@@ -15,6 +17,7 @@ public class TestQueryCacheConcurrent extends BaseTestCase {
private volatile boolean failed;
private volatile int step;
@ForPlatform(Platform.POSTGRES) // H2 isn't good enough for this test
@Test
public void testConcurrent() throws InterruptedException {