introduce CheckUtil to make multiple condition checks clearer in log
Project: http://git-wip-us.apache.org/repos/asf/kylin/repo Commit: http://git-wip-us.apache.org/repos/asf/kylin/commit/2436140e Tree: http://git-wip-us.apache.org/repos/asf/kylin/tree/2436140e Diff: http://git-wip-us.apache.org/repos/asf/kylin/diff/2436140e Branch: refs/heads/master-cdh5.7 Commit: 2436140ef093c8d5b061124a5dbc2f5a1c3e9f3f Parents: 182c565 Author: Hongbin Ma <mahong...@apache.org> Authored: Sat Dec 24 16:52:19 2016 +0800 Committer: Hongbin Ma <mahong...@apache.org> Committed: Tue Dec 27 22:15:44 2016 +0800 ---------------------------------------------------------------------- .../org/apache/kylin/common/util/CheckUtil.java | 33 ++++++++++++++++++++ .../apache/kylin/rest/service/QueryService.java | 17 ++++++---- 2 files changed, 44 insertions(+), 6 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kylin/blob/2436140e/core-common/src/main/java/org/apache/kylin/common/util/CheckUtil.java ---------------------------------------------------------------------- diff --git a/core-common/src/main/java/org/apache/kylin/common/util/CheckUtil.java b/core-common/src/main/java/org/apache/kylin/common/util/CheckUtil.java new file mode 100644 index 0000000..ae189f7 --- /dev/null +++ b/core-common/src/main/java/org/apache/kylin/common/util/CheckUtil.java @@ -0,0 +1,33 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. +*/ +package org.apache.kylin.common.util; + +import org.slf4j.LoggerFactory; + +public class CheckUtil { + public static final org.slf4j.Logger logger = LoggerFactory.getLogger(CheckUtil.class); + + public static boolean checkCondition(boolean condition, String message, Object... args) { + if (condition) { + return true; + } else { + logger.debug(message, args); + return false; + } + } +} http://git-wip-us.apache.org/repos/asf/kylin/blob/2436140e/server-base/src/main/java/org/apache/kylin/rest/service/QueryService.java ---------------------------------------------------------------------- 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 0dd5c5f..bb0342f 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 @@ -18,6 +18,8 @@ package org.apache.kylin.rest.service; +import static org.apache.kylin.common.util.CheckUtil.checkCondition; + import java.io.IOException; import java.sql.Connection; import java.sql.DatabaseMetaData; @@ -341,7 +343,9 @@ public class QueryService extends BasicService { long startTime = System.currentTimeMillis(); SQLResponse sqlResponse = null; - boolean queryCacheEnabled = kylinConfig.isQueryCacheEnabled() && !BackdoorToggles.getDisableCache(); + boolean queryCacheEnabled = checkCondition(kylinConfig.isQueryCacheEnabled(), "query cache disabled in KylinConfig") && // + checkCondition(!BackdoorToggles.getDisableCache(), "query cache disabled in BackdoorToggles"); + if (queryCacheEnabled) { sqlResponse = searchQueryInCache(sqlRequest); } @@ -355,12 +359,13 @@ public class QueryService extends BasicService { sqlResponse.setDuration(System.currentTimeMillis() - startTime); logger.info("Stats of SQL response: isException: {}, duration: {}, total scan count {}", // String.valueOf(sqlResponse.getIsException()), String.valueOf(sqlResponse.getDuration()), String.valueOf(sqlResponse.getTotalScanCount())); - if (queryCacheEnabled && // - !sqlResponse.getIsException() && // - (sqlResponse.getDuration() > durationThreshold || sqlResponse.getTotalScanCount() > scancountThreshold) && // - (sqlResponse.getResults().size() < kylinConfig.getLargeQueryThreshold())) { //don't cache too large response + if (checkCondition(queryCacheEnabled, "query cache is disabled") && // + checkCondition(!sqlResponse.getIsException(), "query has exception") && // + checkCondition(sqlResponse.getDuration() > durationThreshold || sqlResponse.getTotalScanCount() > scancountThreshold, "query is too lightweight with duration: {} ({}), scan count: {} ({})", sqlResponse.getDuration(), durationThreshold, sqlResponse.getTotalScanCount(), scancountThreshold) && // + checkCondition(sqlResponse.getResults().size() < kylinConfig.getLargeQueryThreshold(), "query response is too large: {} ({})", sqlResponse.getResults().size(), kylinConfig.getLargeQueryThreshold())) { cacheManager.getCache(SUCCESS_QUERY_CACHE).put(new Element(sqlRequest, sqlResponse)); } + } else { sqlResponse.setDuration(System.currentTimeMillis() - startTime); } @@ -437,7 +442,7 @@ public class QueryService extends BasicService { String correctedSql = QueryUtil.massageSql(sqlRequest); if (!correctedSql.equals(sqlRequest.getSql())) { logger.info("The corrected query: " + correctedSql); - + //CAUTION: should not change sqlRequest content! //sqlRequest.setSql(correctedSql); }
