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

Reply via email to