You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@carbondata.apache.org by kevinjmh <gi...@git.apache.org> on 2018/03/31 03:38:28 UTC
[GitHub] carbondata pull request #2123: [CARBONDATA-2297] Support SEARCH_MODE for bas...
GitHub user kevinjmh opened a pull request:
https://github.com/apache/carbondata/pull/2123
[CARBONDATA-2297] Support SEARCH_MODE for basic filter query
Be sure to do all of the following checklist to help us incorporate
your contribution quickly and easily:
- [ ] Any interfaces changed?
- [ ] Any backward compatibility impacted?
- [ ] Document update required?
- [ ] Testing done
Please provide details on
- Whether new unit test cases have been added or why no new tests are required?
- How it is tested? Please attach test report.
- Is it a performance related change? Please attach the performance test report.
- Any additional information to help reviewers in testing this change.
- [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/kevinjmh/carbondata searchmode
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/carbondata/pull/2123.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #2123
----
commit bf4c24c3def7b076edfabb9201ed8ae4df08ebb1
Author: Manhua <ke...@...>
Date: 2018-03-29T11:50:39Z
add search mode
----
---
[GitHub] carbondata pull request #2123: [CARBONDATA-2297] Support SEARCH_MODE for bas...
Posted by kevinjmh <gi...@git.apache.org>.
Github user kevinjmh commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2123#discussion_r178568134
--- Diff: core/src/main/java/org/apache/carbondata/core/scan/executor/impl/SearchModeVectorDetailQueryExecutor.java ---
@@ -0,0 +1,67 @@
+/*
+ * 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.carbondata.core.scan.executor.impl;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+
+import org.apache.carbondata.common.CarbonIterator;
+import org.apache.carbondata.common.logging.LogService;
+import org.apache.carbondata.common.logging.LogServiceFactory;
+import org.apache.carbondata.core.constants.CarbonCommonConstants;
+import org.apache.carbondata.core.scan.executor.exception.QueryExecutionException;
+import org.apache.carbondata.core.scan.executor.infos.BlockExecutionInfo;
+import org.apache.carbondata.core.scan.model.QueryModel;
+import org.apache.carbondata.core.scan.result.iterator.SearchModeResultIterator;
+import org.apache.carbondata.core.util.CarbonProperties;
+
+/**
+ * Below class will be used to execute the detail query and returns columnar vectors.
+ */
+public class SearchModeVectorDetailQueryExecutor extends AbstractQueryExecutor<Object> {
+ private static final LogService LOGGER =
+ LogServiceFactory.getLogService(SearchModeVectorDetailQueryExecutor.class.getName());
+ private static ExecutorService executorService;
+
+ static {
+ int nThread;
+ try {
+ nThread = Integer.parseInt(CarbonProperties.getInstance()
+ .getProperty(CarbonCommonConstants.CARBON_SEARCH_MODE_THREAD,
+ CarbonCommonConstants.CARBON_SEARCH_MODE_THREAD_DEFAULT));
+ } catch (NumberFormatException e) {
+ nThread = Integer.parseInt(CarbonCommonConstants.CARBON_SEARCH_MODE_THREAD_DEFAULT);
+ LOGGER.warn("The carbon.search.mode.thread is invalid. Using the default value " + nThread);
+ }
+ executorService = Executors.newFixedThreadPool(nThread);
--- End diff --
Setting number of threads is a way to limit the parallelism in case of dealing huge number of splits in a task. As for static ExecutorService, it can avoid to create a new thread pool for every query and keep the thread pool standby for next query.
---
[GitHub] carbondata pull request #2123: [CARBONDATA-2297] Support SEARCH_MODE for bas...
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2123#discussion_r178431264
--- Diff: core/src/main/java/org/apache/carbondata/core/scan/executor/impl/SearchModeVectorDetailQueryExecutor.java ---
@@ -0,0 +1,45 @@
+/*
+ * 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.carbondata.core.scan.executor.impl;
+
+import java.io.IOException;
+import java.util.List;
+
+import org.apache.carbondata.common.CarbonIterator;
+import org.apache.carbondata.core.scan.executor.exception.QueryExecutionException;
+import org.apache.carbondata.core.scan.executor.infos.BlockExecutionInfo;
+import org.apache.carbondata.core.scan.model.QueryModel;
+import org.apache.carbondata.core.scan.result.iterator.SearchModeResultIterator;
+
+/**
+ * Below class will be used to execute the detail query and returns columnar vectors.
+ */
+public class SearchModeVectorDetailQueryExecutor extends AbstractQueryExecutor<Object> {
+
+ @Override
+ public CarbonIterator<Object> execute(QueryModel queryModel)
+ throws QueryExecutionException, IOException {
+ List<BlockExecutionInfo> blockExecutionInfoList = getBlockExecutionInfos(queryModel);
+ this.queryIterator = new SearchModeResultIterator(
+ blockExecutionInfoList,
+ queryModel,
+ queryProperties.executorService
--- End diff --
I think we should use a static ExecutorService instead of using queryProperties.executorService in AbstractQueryExecutor. Because, we should not create a new thread pool for every query. All queries should share a same thread pool. The thread pool size should be set by user, you can add a CarbonProperty for this size, it can be a global configuration as of now.
---
[GitHub] carbondata issue #2123: [CARBONDATA-2297] Support SEARCH_MODE for basic filt...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2123
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/4755/
---
[GitHub] carbondata issue #2123: [CARBONDATA-2297] Support SEARCH_MODE for basic filt...
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/2123
LGTM
---
[GitHub] carbondata pull request #2123: [CARBONDATA-2297] Support SEARCH_MODE for bas...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/carbondata/pull/2123
---
[GitHub] carbondata issue #2123: [CARBONDATA-2297] Support SEARCH_MODE for basic filt...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2123
Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/3548/
---
[GitHub] carbondata issue #2123: [CARBONDATA-2297] Support SEARCH_MODE for basic filt...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2123
Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/3527/
---
[GitHub] carbondata issue #2123: [CARBONDATA-2297] Support SEARCH_MODE for basic filt...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2123
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/4700/
---
[GitHub] carbondata issue #2123: [CARBONDATA-2297] Support SEARCH_MODE for basic filt...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/2123
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/4278/
---
[GitHub] carbondata issue #2123: [CARBONDATA-2297] Support SEARCH_MODE for basic filt...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/2123
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/4260/
---
[GitHub] carbondata issue #2123: [CARBONDATA-2297] Support SEARCH_MODE for basic filt...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2123
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/4776/
---
[GitHub] carbondata issue #2123: [CARBONDATA-2297] Support SEARCH_MODE for basic filt...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/2123
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/4208/
---
[GitHub] carbondata pull request #2123: [CARBONDATA-2297] Support SEARCH_MODE for bas...
Posted by xuchuanyin <gi...@git.apache.org>.
Github user xuchuanyin commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2123#discussion_r178560655
--- Diff: core/src/main/java/org/apache/carbondata/core/scan/executor/impl/SearchModeVectorDetailQueryExecutor.java ---
@@ -0,0 +1,67 @@
+/*
+ * 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.carbondata.core.scan.executor.impl;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+
+import org.apache.carbondata.common.CarbonIterator;
+import org.apache.carbondata.common.logging.LogService;
+import org.apache.carbondata.common.logging.LogServiceFactory;
+import org.apache.carbondata.core.constants.CarbonCommonConstants;
+import org.apache.carbondata.core.scan.executor.exception.QueryExecutionException;
+import org.apache.carbondata.core.scan.executor.infos.BlockExecutionInfo;
+import org.apache.carbondata.core.scan.model.QueryModel;
+import org.apache.carbondata.core.scan.result.iterator.SearchModeResultIterator;
+import org.apache.carbondata.core.util.CarbonProperties;
+
+/**
+ * Below class will be used to execute the detail query and returns columnar vectors.
+ */
+public class SearchModeVectorDetailQueryExecutor extends AbstractQueryExecutor<Object> {
+ private static final LogService LOGGER =
+ LogServiceFactory.getLogService(SearchModeVectorDetailQueryExecutor.class.getName());
+ private static ExecutorService executorService;
+
+ static {
+ int nThread;
+ try {
+ nThread = Integer.parseInt(CarbonProperties.getInstance()
+ .getProperty(CarbonCommonConstants.CARBON_SEARCH_MODE_THREAD,
+ CarbonCommonConstants.CARBON_SEARCH_MODE_THREAD_DEFAULT));
+ } catch (NumberFormatException e) {
+ nThread = Integer.parseInt(CarbonCommonConstants.CARBON_SEARCH_MODE_THREAD_DEFAULT);
+ LOGGER.warn("The carbon.search.mode.thread is invalid. Using the default value " + nThread);
+ }
+ executorService = Executors.newFixedThreadPool(nThread);
--- End diff --
Why should we specify the #threads ?
Besides, since executorService is static, all SearchModeResultIterator will use the same pool. Is it intended？
---
[GitHub] carbondata issue #2123: [CARBONDATA-2297] Support SEARCH_MODE for basic filt...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2123
Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/3473/
---