You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2020/08/07 06:47:04 UTC
[GitHub] [incubator-doris] songenjie opened a new pull request #4284: [Apache][Doris][Feature][FE]Support users with high QPS in the same query through result set cache
songenjie opened a new pull request #4284:
URL: https://github.com/apache/incubator-doris/pull/4284
-Support users with high QPS in the same query through result set cache
Fixes #4283
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] songenjie commented on a change in pull request #4284: [Apache][Doris][Feature][FE]Support users with high QPS in the same query through result set cache
Posted by GitBox <gi...@apache.org>.
songenjie commented on a change in pull request #4284:
URL: https://github.com/apache/incubator-doris/pull/4284#discussion_r478016652
##########
File path: fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java
##########
@@ -37,7 +37,7 @@
// System variable
public class SessionVariable implements Serializable, Writable {
-
+
Review comment:
this LOG object no used in SessionVariable classs, could i clear it ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] songenjie commented on pull request #4284: [Apache][Doris][Feature][FE]Support users with high QPS in the same query through result set cache
Posted by GitBox <gi...@apache.org>.
songenjie commented on pull request #4284:
URL: https://github.com/apache/incubator-doris/pull/4284#issuecomment-680682899
> Hi, I think the selfsame OLAP querys in prod env is very rare. Do you test this PR in your prod env? what's the cache hit rate? The Apache Kylin use SQL result cache Like this PR, After Kylin run many years in MEITUAN, we could confirm the SQL result cache is meaningless for OLAP system.
3ks
the result cache has been used in Jingdong for more than half a year, and the cache hit rate is also very high, especially in the e-commerce industry like ours, in special periods and business scenarios, its hit rate is very high
As far as I know, most of the business of meituan is near real-time, and the scope of business use and analysis is slightly different from ours. So maybe the hit rate of cache is not high
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] yangzhg commented on a change in pull request #4284: [Apache][Doris][Feature][FE]Support users with high QPS in the same query through result set cache
Posted by GitBox <gi...@apache.org>.
yangzhg commented on a change in pull request #4284:
URL: https://github.com/apache/incubator-doris/pull/4284#discussion_r472805752
##########
File path: fe/fe-core/src/main/java/org/apache/doris/common/FeMetaVersion.java
##########
@@ -190,6 +190,7 @@
// force drop db, force drop table, force drop partition
// make force drop operation do not recycle meta
public static final int VERSION_89 = 89;
+ // fe result cache
Review comment:
this is unnecessary
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] kangkaisen commented on pull request #4284: [Apache][Doris][Feature][FE]Support users with high QPS in the same query through result set cache
Posted by GitBox <gi...@apache.org>.
kangkaisen commented on pull request #4284:
URL: https://github.com/apache/incubator-doris/pull/4284#issuecomment-679909964
@songenjie Hi, what's your design doc for this PR? I think this PR is repeated with SQL mode cache in https://github.com/apache/incubator-doris/pull/4330 , what do you think it？
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman commented on a change in pull request #4284: [Apache][Doris][Feature][FE]Support users with high QPS in the same query through result set cache
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #4284:
URL: https://github.com/apache/incubator-doris/pull/4284#discussion_r477230172
##########
File path: fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java
##########
@@ -582,31 +592,55 @@ private void handleSetStmt() {
}
// Process a select statement.
- private void handleQueryStmt() throws Exception {
+ private boolean handleQueryStmt() throws Exception {
Review comment:
Add comment to explain the return value.
##########
File path: fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java
##########
@@ -582,31 +592,55 @@ private void handleSetStmt() {
}
// Process a select statement.
- private void handleQueryStmt() throws Exception {
+ private boolean handleQueryStmt() throws Exception {
// Every time set no send flag and clean all data in buffer
context.getMysqlChannel().reset();
QueryStmt queryStmt = (QueryStmt) parsedStmt;
+ // Use connection ID as session identifier
+ NamedKey namedKey = new NamedKey(String.valueOf(context.getConnectionId()),
+ StringUtils.toUtf8(queryStmt.toSql()));
+ LOG.debug("Result Cache NamedKey [" + namedKey + "]");
Review comment:
```suggestion
LOG.debug("Result Cache NamedKey [{}]", namedKey);
```
##########
File path: fe/fe-core/src/main/java/org/apache/doris/common/Config.java
##########
@@ -1218,4 +1300,30 @@
*/
@ConfField(mutable = true, masterOnly = true)
public static boolean recover_with_empty_tablet = false;
-}
+
+ /**
+ * Sql_result_cache
+ * Whether or not the result cache is enabled in Fe level, it can be overwritten with connection/session
+ * level setting in Context.
+ */
+ @ConfField(mutable = true)
+ public static boolean enable_result_cache = false;
Review comment:
Please add docs for the new config in `administrator-guide/config/fe_config.md`
##########
File path: fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java
##########
@@ -582,31 +592,55 @@ private void handleSetStmt() {
}
// Process a select statement.
- private void handleQueryStmt() throws Exception {
+ private boolean handleQueryStmt() throws Exception {
// Every time set no send flag and clean all data in buffer
context.getMysqlChannel().reset();
QueryStmt queryStmt = (QueryStmt) parsedStmt;
+ // Use connection ID as session identifier
+ NamedKey namedKey = new NamedKey(String.valueOf(context.getConnectionId()),
Review comment:
We had a `originStmt.originStmt`, which is the origin sql string. how about use it instead of `queryStmt.toSql()`?
##########
File path: fe/fe-core/src/main/java/org/apache/doris/cache/SimpleLocalCache.java
##########
@@ -0,0 +1,164 @@
+// 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.doris.cache;
+
+import com.github.benmanes.caffeine.cache.Caffeine;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Maps;
+import net.jpountz.lz4.LZ4Compressor;
+import net.jpountz.lz4.LZ4Factory;
+import net.jpountz.lz4.LZ4FastDecompressor;
+import org.apache.doris.common.Config;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+import java.nio.ByteBuffer;
+import java.util.Map;
+import java.util.OptionalLong;
+import java.util.concurrent.Executor;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * A base cache on each FE node, local only
+ */
+public class SimpleLocalCache implements Cache {
+ private static final Logger LOG = LogManager.getLogger(SimpleLocalCache.class);
+ /**
+ * Minimum cost in "weight" per entry;
+ */
+ private static final int FIXED_COST = 8;
+ private static final int MAX_DEFAULT_BYTES = 1024 * 1024 * 1024;
+ private static final LZ4Factory LZ4_FACTORY = LZ4Factory.fastestInstance();
+ private static final LZ4FastDecompressor LZ4_DECOMPRESSOR = LZ4_FACTORY.fastDecompressor();
+ private static final LZ4Compressor LZ4_COMPRESSOR = LZ4_FACTORY.fastCompressor();
+
+ private final com.github.benmanes.caffeine.cache.Cache<NamedKey, byte[]> cache;
+
+ public static Cache create() {
+ return create(CacheExecutorFactory.COMMON_FJP.createExecutor());
+ }
+
+ // Used in testing
+ public static Cache create(final Executor executor) {
+ LOG.info("Instance cache with expiration " + Config.result_cache_expire_after_in_milliseconds
+ + " milliseconds, max size " + Config.result_cache_size_in_bytes + " bytes");
+ Caffeine<Object, Object> builder = Caffeine.newBuilder().recordStats();
+ if (Config.result_cache_expire_after_in_milliseconds >= 0) {
+ builder.expireAfterWrite(Config.result_cache_expire_after_in_milliseconds, TimeUnit.MILLISECONDS);
+ }
+ if (Config.result_cache_size_in_bytes >= 0) {
+ builder.maximumWeight(Config.result_cache_size_in_bytes);
+ } else {
+ builder.maximumWeight(Math.min(MAX_DEFAULT_BYTES, Runtime.getRuntime().maxMemory() / 10));
+ }
+ builder.weigher((NamedKey key, byte[] value) -> value.length
+ + key.key.length
+ + key.namespace.length() * Character.BYTES
+ + FIXED_COST)
+ .executor(executor);
+ return new SimpleLocalCache(builder.build());
+ }
+
+ private SimpleLocalCache(final com.github.benmanes.caffeine.cache.Cache<NamedKey, byte[]> cache) {
+ this.cache = cache;
+ }
+
+ @Override
+ public byte[] get(NamedKey key) {
+ return decompress(cache.getIfPresent(key));
+ }
+
+ @Override
+ public void put(NamedKey key, byte[] value) {
+ final byte[] compresssize = compress(value);
+ if (compresssize.length > Config.result_cache_size_per_query_in_bytes) {
+ LOG.info(" result size more than result_cache_size_per_query_in_bytes: "
+ + Config.result_cache_size_per_query_in_bytes + " so not storage in cache");
+ return;
+ }
+ cache.put(key, compress(value));
Review comment:
```suggestion
cache.put(key, compresssize);
```
##########
File path: fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java
##########
@@ -582,31 +592,55 @@ private void handleSetStmt() {
}
// Process a select statement.
- private void handleQueryStmt() throws Exception {
+ private boolean handleQueryStmt() throws Exception {
// Every time set no send flag and clean all data in buffer
context.getMysqlChannel().reset();
QueryStmt queryStmt = (QueryStmt) parsedStmt;
+ // Use connection ID as session identifier
+ NamedKey namedKey = new NamedKey(String.valueOf(context.getConnectionId()),
+ StringUtils.toUtf8(queryStmt.toSql()));
+ LOG.debug("Result Cache NamedKey [" + namedKey + "]");
+
QueryDetail queryDetail = new QueryDetail(context.getStartTime(),
- DebugUtil.printId(context.queryId()),
- context.getStartTime(), -1, -1,
- QueryDetail.QueryMemState.RUNNING,
- context.getDatabase(),
- originStmt.originStmt);
+ DebugUtil.printId(context.queryId()),
+ context.getStartTime(), -1, -1,
+ QueryDetail.QueryMemState.RUNNING,
+ context.getDatabase(),
+ originStmt.originStmt);
context.setQueryDetail(queryDetail);
QueryDetailQueue.addOrUpdateQueryDetail(queryDetail);
if (queryStmt.isExplain()) {
- String explainString = planner.getExplainString(planner.getFragments(), queryStmt.isVerbose() ? TExplainLevel.VERBOSE: TExplainLevel.NORMAL.NORMAL);
+ String explainString = planner.getExplainString(planner.getFragments(), queryStmt.isVerbose() ? TExplainLevel.VERBOSE : TExplainLevel.NORMAL.NORMAL);
handleExplainStmt(explainString);
- return;
+ return false;
}
coord = new Coordinator(context, analyzer, planner);
- QeProcessorImpl.INSTANCE.registerQuery(context.queryId(),
+ QeProcessorImpl.INSTANCE.registerQuery(context.queryId(),
new QeProcessorImpl.QueryInfo(context, originStmt.originStmt, coord));
- coord.exec();
+ boolean isCacheEnabled = context.getSessionVariable().isEnableResultCache();
+ LOG.debug("Session level cache is " + (isCacheEnabled ? "enabled" : false));
Review comment:
```suggestion
LOG.debug("Session level cache is {}", (isCacheEnabled ? "enabled" : false));
```
##########
File path: fe/fe-core/src/main/java/org/apache/doris/cache/SimpleLocalCache.java
##########
@@ -0,0 +1,164 @@
+// 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.doris.cache;
+
+import com.github.benmanes.caffeine.cache.Caffeine;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Maps;
+import net.jpountz.lz4.LZ4Compressor;
+import net.jpountz.lz4.LZ4Factory;
+import net.jpountz.lz4.LZ4FastDecompressor;
+import org.apache.doris.common.Config;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+import java.nio.ByteBuffer;
+import java.util.Map;
+import java.util.OptionalLong;
+import java.util.concurrent.Executor;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * A base cache on each FE node, local only
+ */
+public class SimpleLocalCache implements Cache {
+ private static final Logger LOG = LogManager.getLogger(SimpleLocalCache.class);
+ /**
+ * Minimum cost in "weight" per entry;
+ */
+ private static final int FIXED_COST = 8;
+ private static final int MAX_DEFAULT_BYTES = 1024 * 1024 * 1024;
+ private static final LZ4Factory LZ4_FACTORY = LZ4Factory.fastestInstance();
+ private static final LZ4FastDecompressor LZ4_DECOMPRESSOR = LZ4_FACTORY.fastDecompressor();
+ private static final LZ4Compressor LZ4_COMPRESSOR = LZ4_FACTORY.fastCompressor();
+
+ private final com.github.benmanes.caffeine.cache.Cache<NamedKey, byte[]> cache;
+
+ public static Cache create() {
+ return create(CacheExecutorFactory.COMMON_FJP.createExecutor());
+ }
+
+ // Used in testing
+ public static Cache create(final Executor executor) {
+ LOG.info("Instance cache with expiration " + Config.result_cache_expire_after_in_milliseconds
+ + " milliseconds, max size " + Config.result_cache_size_in_bytes + " bytes");
+ Caffeine<Object, Object> builder = Caffeine.newBuilder().recordStats();
+ if (Config.result_cache_expire_after_in_milliseconds >= 0) {
+ builder.expireAfterWrite(Config.result_cache_expire_after_in_milliseconds, TimeUnit.MILLISECONDS);
+ }
+ if (Config.result_cache_size_in_bytes >= 0) {
+ builder.maximumWeight(Config.result_cache_size_in_bytes);
+ } else {
+ builder.maximumWeight(Math.min(MAX_DEFAULT_BYTES, Runtime.getRuntime().maxMemory() / 10));
+ }
+ builder.weigher((NamedKey key, byte[] value) -> value.length
+ + key.key.length
+ + key.namespace.length() * Character.BYTES
+ + FIXED_COST)
+ .executor(executor);
+ return new SimpleLocalCache(builder.build());
+ }
+
+ private SimpleLocalCache(final com.github.benmanes.caffeine.cache.Cache<NamedKey, byte[]> cache) {
+ this.cache = cache;
+ }
+
+ @Override
+ public byte[] get(NamedKey key) {
+ return decompress(cache.getIfPresent(key));
+ }
+
+ @Override
+ public void put(NamedKey key, byte[] value) {
+ final byte[] compresssize = compress(value);
+ if (compresssize.length > Config.result_cache_size_per_query_in_bytes) {
+ LOG.info(" result size more than result_cache_size_per_query_in_bytes: "
Review comment:
```suggestion
LOG.debug(" result size more than result_cache_size_per_query_in_bytes: "
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] songenjie commented on pull request #4284: [Apache][Doris][Feature][FE]Support users with high QPS in the same query through result set cache
Posted by GitBox <gi...@apache.org>.
songenjie commented on pull request #4284:
URL: https://github.com/apache/incubator-doris/pull/4284#issuecomment-681312482
> I didn't see any code about cache invalid? Did I miss something?
https://github.com/apache/incubator-doris/blob/6c2ca4690139acdf917193ec4d0acda39b7a148b/fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java#L635
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman commented on pull request #4284: [Apache][Doris][Feature][FE]Support users with high QPS in the same query through result set cache
Posted by GitBox <gi...@apache.org>.
morningman commented on pull request #4284:
URL: https://github.com/apache/incubator-doris/pull/4284#issuecomment-681622060
> > I mean, where is the logic for judging cache invalidation? The cache key (NamedKey) is only composed by connection id and origin stmt? What if the data has been unchanged?
>
> https://github.com/apache/incubator-doris/blob/dd42c6b580afcbb53436dd8b531e03e5fa5d01ed/fe/fe-core/src/main/java/org/apache/doris/cache/SimpleLocalCache.java#L63
>
> use Caffeine.newBuilder().recordStats();
So it only depends on time? If data changed within `Config.result_cache_expire_after_in_milliseconds`, user in same context still read the old data?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] yangzhg commented on a change in pull request #4284: [Apache][Doris][Feature][FE]Support users with high QPS in the same query through result set cache
Posted by GitBox <gi...@apache.org>.
yangzhg commented on a change in pull request #4284:
URL: https://github.com/apache/incubator-doris/pull/4284#discussion_r472801382
##########
File path: fe/fe-core/src/main/java/org/apache/doris/common/FeMetaVersion.java
##########
@@ -190,6 +190,7 @@
// force drop db, force drop table, force drop partition
// make force drop operation do not recycle meta
public static final int VERSION_89 = 89;
+ // fe result cache
Review comment:
you should add a new Version 91 if you change the meta
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman commented on a change in pull request #4284: [Apache][Doris][Feature][FE]Support users with high QPS in the same query through result set cache
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #4284:
URL: https://github.com/apache/incubator-doris/pull/4284#discussion_r477993931
##########
File path: fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java
##########
@@ -37,7 +37,7 @@
// System variable
public class SessionVariable implements Serializable, Writable {
-
+
Review comment:
The LOG created here use wrong class, could you help to modify it?
It should be SessionVariable.class
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] songenjie commented on pull request #4284: [Apache][Doris][Feature][FE]Support users with high QPS in the same query through result set cache
Posted by GitBox <gi...@apache.org>.
songenjie commented on pull request #4284:
URL: https://github.com/apache/incubator-doris/pull/4284#issuecomment-681624639
> > > I mean, where is the logic for judging cache invalidation? The cache key (NamedKey) is only composed by connection id and origin stmt? What if the data has been unchanged?
> >
> >
> > https://github.com/apache/incubator-doris/blob/dd42c6b580afcbb53436dd8b531e03e5fa5d01ed/fe/fe-core/src/main/java/org/apache/doris/cache/SimpleLocalCache.java#L63
> >
> > use Caffeine.newBuilder().recordStats();
>
> So it only depends on time? If data changed within `Config.result_cache_expire_after_in_milliseconds`, user in same context still read the old data?
right
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] songenjie commented on a change in pull request #4284: [Apache][Doris][Feature][FE]Support users with high QPS in the same query through result set cache
Posted by GitBox <gi...@apache.org>.
songenjie commented on a change in pull request #4284:
URL: https://github.com/apache/incubator-doris/pull/4284#discussion_r477998913
##########
File path: fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java
##########
@@ -582,31 +592,55 @@ private void handleSetStmt() {
}
// Process a select statement.
- private void handleQueryStmt() throws Exception {
+ private boolean handleQueryStmt() throws Exception {
Review comment:
3ks,done
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] yangzhg commented on a change in pull request #4284: [Apache][Doris][Feature][FE]Support users with high QPS in the same query through result set cache
Posted by GitBox <gi...@apache.org>.
yangzhg commented on a change in pull request #4284:
URL: https://github.com/apache/incubator-doris/pull/4284#discussion_r472810753
##########
File path: fe/fe-core/src/main/java/org/apache/doris/common/Config.java
##########
@@ -1218,4 +1218,24 @@
*/
@ConfField(mutable = true, masterOnly = true)
public static boolean recover_with_empty_tablet = false;
-}
+
+ /**
+ * Sql_result_cache
+ * Whether or not the result cache is enabled in Fe level, it can be overwritten with connection/session
+ * level setting in Context.
+ */
+ @ConfField(mutable = true)
+ public static boolean enable_result_cache = false;
+
+ /**
+ * Specify how long an entry will be expired in milliseconds, 10000 by default.
+ */
+ @ConfField(mutable = true)
+ public static long result_cache_expire_after_in_milliseconds = 10*1000;
+
+ /**
+ * Specify the overall threshold of local cache in bytes, 1G bytes by default.
+ */
+ @ConfField(mutable = true)
+ public static long result_cache_size_in_bytes = 1024 * 1024 * 1024;
Review comment:
you should also limit ervey cache result entity size to avoid one large result use all caches, we prefer to cache small size and need more time to calculating result,
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman commented on pull request #4284: [Apache][Doris][Feature][FE]Support users with high QPS in the same query through result set cache
Posted by GitBox <gi...@apache.org>.
morningman commented on pull request #4284:
URL: https://github.com/apache/incubator-doris/pull/4284#issuecomment-680826517
I didn't see any code about cache invalid? Did I miss something?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] kangkaisen commented on pull request #4284: [Apache][Doris][Feature][FE]Support users with high QPS in the same query through result set cache
Posted by GitBox <gi...@apache.org>.
kangkaisen commented on pull request #4284:
URL: https://github.com/apache/incubator-doris/pull/4284#issuecomment-680607002
> 1. this PR `Result Cache`
>
> * Solve the same connect ID, the same SQL high QPS problem, the same user, in `result_ cache_ expire_ after_ In_ Within milliseconds` time, executing the same SQL will hit the result cache
Hi, I think the selfsame OLAP querys in prod env is very rare. Do you test this PR in your prod env? what's the cache hit rate?
The Apache Kylin use SQL result cache Like this PR, After Kylin run many years in MEITUAN, we could confirm the SQL result cache is meaningless for OLAP system.
The Apache Druid use segment cache like #4330, After Druid run many years in MEITUAN, we could confirm the segment cache is useful for OLAP system.
So I think If we already have segment query cache, we don't need the query result cache.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] songenjie commented on pull request #4284: [Apache][Doris][Feature][FE]Support users with high QPS in the same query through result set cache
Posted by GitBox <gi...@apache.org>.
songenjie commented on pull request #4284:
URL: https://github.com/apache/incubator-doris/pull/4284#issuecomment-681613359
> I mean, where is the logic for judging cache invalidation? The cache key (NamedKey) is only composed by connection id and origin stmt? What if the data has been unchanged?
https://github.com/apache/incubator-doris/blob/dd42c6b580afcbb53436dd8b531e03e5fa5d01ed/fe/fe-core/src/main/java/org/apache/doris/cache/SimpleLocalCache.java#L63
use Caffeine.newBuilder().recordStats();
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] songenjie commented on pull request #4284: [Apache][Doris][Feature][FE]Support users with high QPS in the same query through result set cache
Posted by GitBox <gi...@apache.org>.
songenjie commented on pull request #4284:
URL: https://github.com/apache/incubator-doris/pull/4284#issuecomment-680461074
> @songenjie Hi, what's your design doc for this PR? I think this PR is repeated with SQL mode cache in #4330 , what do you think it？
@kangkaisen
1. Sql Cache Mode
- If this switch is turned on, the SQL query result set will be cached. If the interval between the last visit version time in all partitions of all tables in the query is greater than cache_last_version_interval_second, and the result set is less than cache_result_max_row_count, the result set will be cached, and the next same SQL will hit the cache.
2. this PR `Result Cache`
- Solve the same connect ID, the same SQL high QPS problem, the same user, in `result_ cache_ expire_ after_ In_ Within milliseconds` time, executing the same SQL will hit the result cache
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] yangzhg commented on a change in pull request #4284: [Apache][Doris][Feature][FE]Support users with high QPS in the same query through result set cache
Posted by GitBox <gi...@apache.org>.
yangzhg commented on a change in pull request #4284:
URL: https://github.com/apache/incubator-doris/pull/4284#discussion_r472805316
##########
File path: fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java
##########
@@ -623,6 +653,9 @@ public void readFields(DataInput in) throws IOException {
if (Catalog.getCurrentCatalogJournalVersion() >= FeMetaVersion.VERSION_62) {
exchangeInstanceParallel = in.readInt();
}
+ if (Catalog.getCurrentCatalogJournalVersion() >= FeMetaVersion.VERSION_90) {
Review comment:
this is unnecessary, we use json for serialization now, this functiuon is deprecated
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org