GitHub Issue 1187: Auto-disable PostgreSQL JDBC result set caching#7833
GitHub Issue 1187: Auto-disable PostgreSQL JDBC result set caching#7833labkey-jeckels wants to merge 14 commits into
Conversation
|
Interested in feedback related to connection reuse and the warnings for returning long ArrayLists |
|
|
||
| // Throttles the large-result warning to at most once per day per unique call stack, so a legitimately large (but | ||
| // expected) load doesn't flood the log. Keyed by a signature of the call stack; see getArrayList(). | ||
| private static final Cache<String, Boolean> LARGE_RESULT_WARNING_THROTTLE = CacheManager.getCache(1000, CacheManager.DAY, "SqlSelector large result warnings"); |
There was a problem hiding this comment.
A Throttle is more convenient than a generic Cache
| * (here plus, potentially, in the JDBC driver's buffer) is a common source of OutOfMemoryErrors; callers should | ||
| * generally prefer a streaming method — {@link #forEach(Class, Selector.ForEachBlock)}, {@link #forEachBatch}, or {@link #uncachedStream} — that | ||
| * processes rows without materializing them all at once. {@code getArray}, {@code getCollection}, | ||
| * {@code getMapArray}, and {@code getMapCollection} all delegate here, so they're covered as well. |
There was a problem hiding this comment.
FYI: BaseSelector.stream() calls this as well
There was a problem hiding this comment.
Comment adjusted. Unexpected that stream() isn't actually streaming.
There was a problem hiding this comment.
Well, it is a Java Stream. The trade-off is there's no way for a Stream to close resources (e.g., ResultSet, Connection) after exhausting/terminal operation. So either you need to get the stream via try-with-resources (required for our uncached variants) or stream an in-memory data structure. These methods are well documented. We could reverse the paradigm (cachedStream(), stream()). Or require every stream() caller to close the stream. It's just not super intuitive when folks aren't used to closing streams. Devs have messed this up repeatedly with Files.find(), e.g.
2 participants
Rationale
The Postgres JDBC driver caches ResultSets in memory by default for our configuration. We want to opt-out more aggressively to avoid OutOfMemoryError and performance problems.
Changes
SqlExecutingSelectorto not cache by defaultArrayListof results