This is an automated email from the ASF dual-hosted git repository. maytasm pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/druid.git
The following commit(s) were added to refs/heads/master by this push: new 187d3fa7c4f Wrap populateResults() method to prevent query from failing (#18044) 187d3fa7c4f is described below commit 187d3fa7c4f5a53289c6e1a1887c926b4c7eaba3 Author: jtuglu-netflix <jtu...@netflix.com> AuthorDate: Sat May 31 12:51:44 2025 -0700 Wrap populateResults() method to prevent query from failing (#18044) Fixes the case where the ObjectStream construction is possible, but cache insertion fails. An example of cache insertion failure might be an LZ4 max compression size breach, for example. --- .../druid/query/ResultLevelCachingQueryRunner.java | 33 ++++++++++++++-------- .../query/ResultLevelCachingQueryRunnerTest.java | 22 +++++++++++++++ 2 files changed, 44 insertions(+), 11 deletions(-) diff --git a/server/src/main/java/org/apache/druid/query/ResultLevelCachingQueryRunner.java b/server/src/main/java/org/apache/druid/query/ResultLevelCachingQueryRunner.java index 7e32e098a13..872590b0643 100644 --- a/server/src/main/java/org/apache/druid/query/ResultLevelCachingQueryRunner.java +++ b/server/src/main/java/org/apache/druid/query/ResultLevelCachingQueryRunner.java @@ -144,21 +144,32 @@ public class ResultLevelCachingQueryRunner<T> implements QueryRunner<T> resultLevelCachePopulator, "ResultLevelCachePopulator cannot be null during cache population" ); - if (thrown != null) { + try { + if (thrown != null) { + log.error( + thrown, + "Error while preparing for result level caching for query %s with error %s ", + query.getId(), + thrown.getMessage() + ); + } else if (resultLevelCachePopulator.isShouldPopulate()) { + // The resultset identifier and its length is cached along with the resultset + resultLevelCachePopulator.populateResults(); + log.debug("Cache population complete for query %s", query.getId()); + } else { // thrown == null && !resultLevelCachePopulator.isShouldPopulate() + log.error("Failed (gracefully) to prepare result level cache entry for query %s", query.getId()); + } + } + catch (Exception e) { log.error( - thrown, - "Error while preparing for result level caching for query %s with error %s ", + "Failed to populate result level cache for query %s with error %s", query.getId(), - thrown.getMessage() + e.getMessage() ); - } else if (resultLevelCachePopulator.isShouldPopulate()) { - // The resultset identifier and its length is cached along with the resultset - resultLevelCachePopulator.populateResults(); - log.debug("Cache population complete for query %s", query.getId()); - } else { // thrown == null && !resultLevelCachePopulator.isShouldPopulate() - log.error("Failed (gracefully) to populate result level cache for query %s", query.getId()); } - resultLevelCachePopulator.stopPopulating(); + finally { + resultLevelCachePopulator.stopPopulating(); + } } } ); diff --git a/server/src/test/java/org/apache/druid/query/ResultLevelCachingQueryRunnerTest.java b/server/src/test/java/org/apache/druid/query/ResultLevelCachingQueryRunnerTest.java index a0c14239e49..aef6ed503ad 100644 --- a/server/src/test/java/org/apache/druid/query/ResultLevelCachingQueryRunnerTest.java +++ b/server/src/test/java/org/apache/druid/query/ResultLevelCachingQueryRunnerTest.java @@ -340,6 +340,28 @@ public class ResultLevelCachingQueryRunnerTest extends QueryRunnerBasedOnCluster Assert.assertEquals(1, cache.getStats().getNumMisses()); } + @Test + public void testPopulateCacheThrowsException() + { + cache = Mockito.spy(cache); + Mockito.doThrow(new RuntimeException("some error")).when(cache).put(ArgumentMatchers.any(), ArgumentMatchers.any()); + + prepareCluster(10); + final Query<Result<TimeseriesResultValue>> query = timeseriesQuery(BASE_SCHEMA_INFO.getDataInterval()); + final ResultLevelCachingQueryRunner<Result<TimeseriesResultValue>> queryRunner1 = createQueryRunner( + newCacheConfig(true, true, DEFAULT_CACHE_ENTRY_MAX_SIZE), + query + ); + + queryRunner1.run( + QueryPlus.wrap(query), + responseContext() + ).toList(); + Assert.assertEquals(0, cache.getStats().getNumHits()); + Assert.assertEquals(0, cache.getStats().getNumEntries()); + Assert.assertEquals(1, cache.getStats().getNumMisses()); + } + private <T> ResultLevelCachingQueryRunner<T> createQueryRunner( CacheConfig cacheConfig, Query<T> query --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org