Skip to content

Commit 4e41289

Browse files
Adam likes Throttles
1 parent 8ab2604 commit 4e41289

1 file changed

Lines changed: 27 additions & 13 deletions

File tree

api/src/org/labkey/api/data/SqlExecutingSelector.java

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@
1919
import org.apache.logging.log4j.Logger;
2020
import org.jetbrains.annotations.NotNull;
2121
import org.jetbrains.annotations.Nullable;
22-
import org.labkey.api.cache.Cache;
2322
import org.labkey.api.cache.CacheManager;
23+
import org.labkey.api.cache.Throttle;
2424
import org.labkey.api.data.dialect.SqlDialect;
2525
import org.labkey.api.data.dialect.SqlDialect.ExecutionPlanType;
2626
import org.labkey.api.data.dialect.StatementWrapper;
@@ -56,7 +56,9 @@ public abstract class SqlExecutingSelector<FACTORY extends SqlFactory, SELECTOR
5656

5757
// Throttles the large-result warning to at most once per day per unique call stack, so a legitimately large (but
5858
// expected) load doesn't flood the log. Keyed by a signature of the call stack; see getArrayList().
59-
private static final Cache<String, Boolean> LARGE_RESULT_WARNING_THROTTLE = CacheManager.getCache(1000, CacheManager.DAY, "SqlSelector large result warnings");
59+
private static final Throttle<LargeResultWarning> LARGE_RESULT_WARNING_THROTTLE = new Throttle<>("SqlSelector large result warnings", 1000, CacheManager.DAY,
60+
w -> LOGGER.warn("{} rows loaded into a collection via {}. Consider switching to streaming variants to reduce memory usage. SQL: {}",
61+
w.rowCount, w.selectorClass, w.sql, w.stackTrace));
6062

6163
int _maxRows = Table.ALL_ROWS;
6264
protected long _offset = Table.NO_OFFSET;
@@ -172,7 +174,7 @@ public SELECTOR setJdbcCaching(boolean cache)
172174
* (here plus, potentially, in the JDBC driver's buffer) is a common source of OutOfMemoryErrors; callers should
173175
* generally prefer a streaming method — {@link #forEach(Class, Selector.ForEachBlock)}, {@link #forEachBatch}, or {@link #uncachedStream} — that
174176
* processes rows without materializing them all at once. {@code getArray}, {@code getCollection},
175-
* {@code getMapArray}, and {@code getMapCollection} all delegate here, so they're covered as well.
177+
* {@code getMapArray}, {@code stream}, and {@code getMapCollection} all delegate here, so they're covered as well.
176178
*/
177179
@Override
178180
public @NotNull <E> ArrayList<E> getArrayList(Class<E> clazz)
@@ -182,16 +184,10 @@ public SELECTOR setJdbcCaching(boolean cache)
182184
if (result.size() >= LARGE_RESULT_THRESHOLD)
183185
{
184186
Throwable stackTrace = new Throwable("Stack trace for large collection load");
185-
String stackKey = getStackKey(stackTrace);
186-
187-
// Warn at most once per day (tolerating a race condition) per unique call stack to avoid flooding the log.
188-
if (null == LARGE_RESULT_WARNING_THROTTLE.get(stackKey))
189-
{
190-
LARGE_RESULT_WARNING_THROTTLE.put(stackKey, Boolean.TRUE);
191-
// Log the parameterized SQL only (getSQL(), not the SQLFragment) so bound parameter values stay out of the log
192-
LOGGER.warn("{} rows loaded into a collection via {}. Consider switching to forEach(), forEachBatch(), or uncachedStream() to reduce memory usage. SQL: {}",
193-
result.size(), getClass().getSimpleName(), getSqlFactory(false).getSql().getSQL(), stackTrace);
194-
}
187+
// Log the parameterized SQL only so bound parameter values stay out of the log
188+
SQLFragment sql = getSqlFactory(false).getSql();
189+
LARGE_RESULT_WARNING_THROTTLE.execute(new LargeResultWarning(getStackKey(stackTrace), result.size(),
190+
getClass().getSimpleName(), sql == null ? null : sql.getSQL(), stackTrace));
195191
}
196192

197193
return result;
@@ -203,6 +199,24 @@ private static String getStackKey(Throwable t)
203199
return Arrays.stream(t.getStackTrace()).map(StackTraceElement::toString).collect(Collectors.joining("\n"));
204200
}
205201

202+
// Carries the fields needed to build the large-result warning, but hashes/compares only on the call-stack signature
203+
// so the throttle dedupes per unique call stack rather than per (stack + row count + SQL) combination.
204+
private record LargeResultWarning(String stackKey, int rowCount, String selectorClass, String sql,
205+
Throwable stackTrace)
206+
{
207+
@Override
208+
public boolean equals(Object o)
209+
{
210+
return o instanceof LargeResultWarning w && stackKey.equals(w.stackKey);
211+
}
212+
213+
@Override
214+
public int hashCode()
215+
{
216+
return stackKey.hashCode();
217+
}
218+
}
219+
206220
/**
207221
* Set a ResultSet fetch size that differs from the default value (1,000 rows on PostgreSQL). This is normally a
208222
* fine fetch size, but not when dealing with rows containing large TEXT or BYTEA columns.

0 commit comments

Comments
 (0)