[kylin] branch master updated: KYLIN-4432 duplicated queries with sytax error take unexpect long time when lazy query enabled (#1167)

[shaofengshi]

This is an automated email from the ASF dual-hosted git repository.

shaofengshi pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kylin.git


The following commit(s) were added to refs/heads/master by this push:
     new 6342bc5  KYLIN-4432 duplicated queries with sytax error take unexpect 
long time when lazy query enabled (#1167)
6342bc5 is described below

commit 6342bc5f191f88b9856f15c23b15d29548e0ecba
Author: Congling Xia <xiacongl...@xiaomi.com>
AuthorDate: Fri May 22 23:07:55 2020 +0800

    KYLIN-4432 duplicated queries with sytax error take unexpect long time when 
lazy query enabled (#1167)
    
    * KYLIN-4432 duplicated queries with sytax error take unexpect long time 
when lazy query enabled
---
 .../apache/kylin/rest/service/QueryService.java    | 10 ++++++---
 .../kylin/rest/service/QueryServiceTest.java       | 26 ++++++++++++++++++++--
 2 files changed, 31 insertions(+), 5 deletions(-)

diff --git 
a/server-base/src/main/java/org/apache/kylin/rest/service/QueryService.java 
b/server-base/src/main/java/org/apache/kylin/rest/service/QueryService.java
index 4f19e50..8559fcd 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/service/QueryService.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/service/QueryService.java
@@ -492,11 +492,11 @@ public class QueryService extends BasicService {
         Message msg = MsgPicker.getMsg();
         final QueryContext queryContext = QueryContextFacade.current();
 
-        boolean isDummpyResponseEnabled = queryCacheEnabled && 
kylinConfig.isLazyQueryEnabled();
+        boolean isDummyResponseEnabled = queryCacheEnabled && 
kylinConfig.isLazyQueryEnabled();
         SQLResponse sqlResponse = null;
         try {
             // Add dummy response which will be updated or evicted when query 
finishes
-            if (isDummpyResponseEnabled) {
+            if (isDummyResponseEnabled) {
                 SQLResponse dummyResponse = new SQLResponse();
                 
dummyResponse.setLazyQueryStartTime(System.currentTimeMillis());
                 
cacheManager.getCache(QUERY_CACHE).put(sqlRequest.getCacheKey(), dummyResponse);
@@ -559,7 +559,7 @@ public class QueryService extends BasicService {
                 if (!realtimeQuery) {
                     
cacheManager.getCache(QUERY_CACHE).put(sqlRequest.getCacheKey(), sqlResponse);
                 }
-            } else if (isDummpyResponseEnabled) {
+            } else if (isDummyResponseEnabled) {
                 
cacheManager.getCache(QUERY_CACHE).evict(sqlRequest.getCacheKey());
             }
 
@@ -576,6 +576,10 @@ public class QueryService extends BasicService {
                     && ExceptionUtils.getRootCause(e) instanceof 
ResourceLimitExceededException) {
                 Cache exceptionCache = cacheManager.getCache(QUERY_CACHE);
                 exceptionCache.put(sqlRequest.getCacheKey(), sqlResponse);
+            } else if (isDummyResponseEnabled) {
+                // evict dummy response to avoid caching too many bad queries
+                Cache exceptionCache = cacheManager.getCache(QUERY_CACHE);
+                exceptionCache.evict(sqlRequest.getCacheKey());
             }
         }
         return sqlResponse;
diff --git 
a/server/src/test/java/org/apache/kylin/rest/service/QueryServiceTest.java 
b/server/src/test/java/org/apache/kylin/rest/service/QueryServiceTest.java
index f3dc6b1..acf9e19 100644
--- a/server/src/test/java/org/apache/kylin/rest/service/QueryServiceTest.java
+++ b/server/src/test/java/org/apache/kylin/rest/service/QueryServiceTest.java
@@ -34,6 +34,8 @@ import org.junit.Assert;
 import org.junit.Test;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Qualifier;
+import org.springframework.cache.Cache;
+import org.springframework.cache.CacheManager;
 
 /**
  * @author xduo
@@ -45,8 +47,7 @@ public class QueryServiceTest extends ServiceTestBase {
     QueryService queryService;
 
     @Autowired
-    @Qualifier("cacheService")
-    private CacheService cacheService;
+    CacheManager cacheManager;
 
     @Test
     public void testBasics() throws JobException, IOException, SQLException {
@@ -98,4 +99,25 @@ public class QueryServiceTest extends ServiceTestBase {
                     response.getExceptionMessage());
         }
     }
+
+    @Test
+    public void testSyntaxError() {
+        KylinConfig config = KylinConfig.getInstanceFromEnv();
+        config.setProperty("kylin.query.cache-enabled", "true");
+        config.setProperty("kylin.query.lazy-query-enabled", "true");
+
+        String badSql = "select with syntax error";
+
+        SQLRequest request = new SQLRequest();
+        request.setProject("default");
+        request.setSql(badSql);
+
+        try (SetAndUnsetThreadLocalConfig autoUnset = 
KylinConfig.setAndUnsetThreadLocalConfig(config)) {
+            queryService.doQueryWithCache(request, false);
+        } catch (Exception e) {
+            // expected error
+            Cache.ValueWrapper wrapper = 
cacheManager.getCache(QueryService.QUERY_CACHE).get(request.getCacheKey());
+            Assert.assertTrue(wrapper == null || wrapper.get() == null);
+        }
+    }
 }