[GitHub] drill pull request: DRILL-4192: Fix selection root when there is o...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/307#discussion_r48179942
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/FileSelection.java
---
@@ -201,7 +202,7 @@ public static FileSelection create(final
DrillFileSystem fs, final String parent
*
* @see FileSelection#FileSelection(List, List, String)
*/
- public static FileSelection create(final List statuses,
final List files, final String root) {
+ public static FileSelection create(final List statuses,
final List files, String root) {
--- End diff --
I would recommend avoiding the re-assignment to root.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4192: Fix selection root when there is o...
Github user hnfgns commented on a diff in the pull request: https://github.com/apache/drill/pull/307#discussion_r48180127 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/FileSelection.java --- @@ -213,14 +214,15 @@ public static FileSelection create(final List statuses, final List
[GitHub] drill pull request: DRILL-4192: Fix selection root when there is o...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/307#discussion_r48180355
--- Diff: exec/java-exec/src/test/java/org/apache/drill/TestBugFixes.java
---
@@ -18,12 +18,15 @@
package org.apache.drill;
import org.apache.drill.common.exceptions.UserException;
+import org.apache.drill.common.util.TestTools;
import org.apache.drill.exec.planner.physical.PlannerSettings;
import org.junit.Ignore;
import org.junit.Test;
public class TestBugFixes extends BaseTestQuery {
private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(TestBugFixes.class);
+ static final String WORKING_PATH = TestTools.getWorkingPath();
+ static final String TEST_RES_PATH = WORKING_PATH + "/src/test/resources";
--- End diff --
You may like to use TestTools.getTestResourcesPath() instead, less
fail-prone.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4192: Fix selection root when there is o...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/307#discussion_r48180622
--- Diff: exec/java-exec/src/test/java/org/apache/drill/TestBugFixes.java
---
@@ -144,4 +147,15 @@ public void testDRILL2361_JoinColumnAliasWithDots()
throws Exception {
.build().run();
}
+ @Test
+ public void testDRILL4192() throws Exception {
+String query = (String.format("select dir0, dir1 from
dfs_test.`%s/bugs/DRILL-4192` order by dir1", TEST_RES_PATH));
--- End diff --
I am wondering if this test should also include an HDFS wildcard.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4187: introduce a new query state ENQUEU...
GitHub user hnfgns opened a pull request: https://github.com/apache/drill/pull/310 DRILL-4187: introduce a new query state ENQUEUED and rename the state PENDING to STARTING You can merge this pull request into a Git repository by running: $ git pull https://github.com/hnfgns/incubator-drill DRILL-4187 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/310.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 #310 commit f108fcf65078d3173c0c4433494ca0d3f4f85de2 Author: Hanifi Gunes Date: 2015-12-16T01:24:27Z DRILL-4187: introduce a new query state ENQUEUED and rename the state PENDING to STARTING --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4187: introduce a new query state ENQUEU...
Github user hnfgns commented on the pull request: https://github.com/apache/drill/pull/310#issuecomment-166731599 @sudheeshkatkam please review --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4187: introduce a new query state ENQUEU...
Github user hnfgns commented on the pull request: https://github.com/apache/drill/pull/310#issuecomment-166762090 Cancellation is handled usual way after query is cancelled only after it has been started. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4187: introduce a new query state ENQUEU...
Github user hnfgns commented on the pull request: https://github.com/apache/drill/pull/310#issuecomment-166762114 Addressed review comments. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4275: Refactor e/pstore interfaces and t...
GitHub user hnfgns opened a pull request: https://github.com/apache/drill/pull/325 DRILL-4275: Refactor e/pstore interfaces and their factories to provide a unified mechanism to access stores The main changes are the following: Remove EStore and EStoreProvider interfaces. Rename PStore to Store, PStoreProvider to StoreProvider Extend Store with getMode():StoreMode that returns store persistence level Ensure StoreProvider.getStore(mode) acts like a factory. Ensure Store implementations throw StoreException in case of failures and handle StoreExceptions All other files contain very tiny, local, satellite changes like variable name updates for name consistency, exception handling et cedera. You can merge this pull request into a Git repository by running: $ git pull https://github.com/hnfgns/incubator-drill DRILL-4275 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/325.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 #325 commit 5b5dcd52242261050cfac2dfd2bbfc64e103cf12 Author: Hanifi Gunes Date: 2016-01-15T01:06:21Z DRILL-4275: Refactor e/pstore interfaces and their factories to provide a unified mechanism to access stores --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4275: Refactor e/pstore interfaces and t...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/325#discussion_r49916936
--- Diff:
contrib/storage-mongo/src/main/java/org/apache/drill/exec/store/mongo/MongoGroupScan.java
---
@@ -180,39 +179,39 @@ private void init() throws IOException {
chunksMapping = Maps.newHashMap();
chunksInverseMapping = Maps.newLinkedHashMap();
if (isShardedCluster(client)) {
- MongoDatabase db = client.getDatabase(CONFIG);
- MongoCollection chunksCollection =
db.getCollection(CHUNKS);
+ MongoDatabase db = client.getDatabase(DrillMongoConstants.CONFIG);
--- End diff --
Whole point here was to avoid using an interface for constants and to avoid
doing a static/wildcard import, following [Google guidelines](
https://google.github.io/styleguide/javaguide.html#s3.3.1-wildcard-imports).
However, if this is taken as a show stopper, I can switch to static imports,
disliking myself later ;)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4275: Refactor e/pstore interfaces and t...
Github user hnfgns commented on the pull request: https://github.com/apache/drill/pull/325#issuecomment-172126518 I took the API somewhat Drill internal but you are right about discussing this in the public list first. This patch -as the numbers above check out- is more of a clean-up than an extension. My plan was to discuss possible changes to API going forward once this clean-up patch made its way to master. Here is the rationale: I spent quite a while trying to understand the current design, event flow in the API in an attempt to find a way for subscribing to store events. Especially being notified when an ephemeral node dies just like what Zookeeper watches offer. I got confused multiple times with E/PStore and E/PStoreProvider distinction at interface level as it seems redundant. That's how the idea of this clean-up patch rolled out. As for naming, I am open for non-abbreviated alternates. I would invite developers relying on this interface to read the documentation first. To this end, I will go ahead and extend documentation with possible warning areas. Please let me know if you want me to address specific issues with this patch. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4275: Refactor e/pstore interfaces and t...
Github user hnfgns commented on the pull request: https://github.com/apache/drill/pull/325#issuecomment-172604790 Cool. Let's sync up on tomorrow's weekly hangout. Let me know if that does not work for you. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4275: Refactor e/pstore interfaces and t...
Github user hnfgns commented on the pull request: https://github.com/apache/drill/pull/325#issuecomment-173003718 How about anytime tomorrow from 10am till noon? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4275: Refactor e/pstore interfaces and t...
Github user hnfgns commented on the pull request: https://github.com/apache/drill/pull/325#issuecomment-174113221 I have created the following [doodle](http://doodle.com/poll/zxkxdzfkgfuanw59) to schedule this. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4235: record new state STARTING when sta...
GitHub user hnfgns opened a pull request: https://github.com/apache/drill/pull/358 DRILL-4235: record new state STARTING when state transitions to STARTING from ENQUEUED You can merge this pull request into a Git repository by running: $ git pull https://github.com/hnfgns/incubator-drill DRILL-4235 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/358.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 #358 commit cb1a0236fa10c031a7b3ead0906c0e135a5f1f3f Author: Hanifi Gunes Date: 2016-02-04T20:07:36Z DRILL-4235: record new state STARTING when state transitions to STARTING from ENQUEUED --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4380: Fix performance regression: in cre...
Github user hnfgns commented on a diff in the pull request: https://github.com/apache/drill/pull/369#discussion_r52378537 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetFormatPlugin.java --- @@ -233,7 +233,7 @@ private FileSelection expandSelection(DrillFileSystem fs, FileSelection selectio // /a/b/c.parquet and the format of the selection root must match that of the file names // otherwise downstream operations such as partition pruning can break. final Path metaRootPath = Path.getPathWithoutSchemeAndAuthority(metaRootDir.getPath()); -final FileSelection newSelection = FileSelection.create(null, fileNames, metaRootPath.toString()); +final FileSelection newSelection = new FileSelection(selection.getStatuses(fs), fileNames, metaRootPath.toString()); --- End diff -- Whole point of making this c'tor non public was to centralize creation via FileSelection.create(...). Looks like we need more explicit comments over here. For this patch, a public c'tor seems not required as well. FileSelection.create(selections, null, root) should do the trick. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4380: Fix performance regression: in cre...
Github user hnfgns commented on a diff in the pull request: https://github.com/apache/drill/pull/369#discussion_r52383230 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetFormatPlugin.java --- @@ -233,7 +233,7 @@ private FileSelection expandSelection(DrillFileSystem fs, FileSelection selectio // /a/b/c.parquet and the format of the selection root must match that of the file names // otherwise downstream operations such as partition pruning can break. final Path metaRootPath = Path.getPathWithoutSchemeAndAuthority(metaRootDir.getPath()); -final FileSelection newSelection = FileSelection.create(null, fileNames, metaRootPath.toString()); +final FileSelection newSelection = new FileSelection(selection.getStatuses(fs), fileNames, metaRootPath.toString()); --- End diff -- Filed DRILL-4381. Thanks. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4275: Refactor e/pstore interfaces and t...
Github user hnfgns closed the pull request at: https://github.com/apache/drill/pull/325 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...
GitHub user hnfgns opened a pull request: https://github.com/apache/drill/pull/374 DRILL-4275: create TransientStore for short-lived objects; refactor PersistentStore to introduce pagination mechanism ps: removed PR#395 mistakenly so starting over. collections/ introducing immutable entry coord/ClusterCoordinator add a factory method to create transient store coord/store introduce transient store and other classes around: factory, config, event, event type introduce base transient store implementation coord/zk introducing path utils for zk introducing general purpose zk client, unit tested complete rewrite of ZkPersistentStore complete rewrite of ZkEphemeralStore, unit tested introducing event dispatcher used by ZkEphemeralStore -- externalized for unit testing, unit tested coord/local/MapBackedStore introduces a local, map backed transient store coord/* updates to adapt new subclasses serialization/ (both transient & persistent store uses this package) introducing instance serializer introducing two concrete implementations: proto and jackson serializers all of PersistentStore subclasses implements new pagination logic java-exec/pom.xml adds curator-test dependency for unit tests server/ update so that transient store is acquired, properly closed. */ misc renamings to reflect class name changes, to remove unneeded import misc unit test fixes misc minor clean-ups You can merge this pull request into a Git repository by running: $ git pull https://github.com/hnfgns/incubator-drill DRILL-4275 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/374.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 #374 commit e077a2d6ba59a6abfe526bd8f38259d3959be5a7 Author: Hanifi Gunes Date: 2016-01-15T01:06:21Z DRILL-4275: create TransientStore for short-lived objects; refactor PersistentStore to introduce pagination mechanism --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/374#discussion_r52817125
--- Diff: exec/java-exec/src/main/resources/drill-module.conf ---
@@ -120,7 +120,7 @@ drill.exec: {
affinity.factor: 1.2
},
sys.store.provider: {
-class: "org.apache.drill.exec.store.sys.zk.ZkPStoreProvider",
+class:
"org.apache.drill.exec.store.sys.store.provider.ZookeeperPersistentStoreProvider",
--- End diff --
Sure. Overriding config with what is already a default
value(ZkPStoreProvider) seems odd though. Back to your point, this patch
proposes a major design/API change around old E/PStore so all references to
ZkPStoreProvider will fail following this patch.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...
Github user hnfgns commented on a diff in the pull request: https://github.com/apache/drill/pull/374#discussion_r52950537 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/store/ZookeeperPersistentStore.java --- @@ -0,0 +1,136 @@ +/** + * 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.drill.exec.store.sys.store; + +import java.io.IOException; +import java.util.Iterator; +import java.util.Map; + +import javax.annotation.Nullable; + +import com.google.common.base.Function; +import com.google.common.base.Preconditions; +import com.google.common.collect.Iterators; +import org.apache.curator.framework.CuratorFramework; +import org.apache.drill.common.collections.ImmutableEntry; +import org.apache.drill.common.exceptions.DrillRuntimeException; +import org.apache.drill.exec.coord.zk.PathUtils; +import org.apache.drill.exec.coord.zk.ZookeeperClient; +import org.apache.drill.exec.exception.StoreException; +import org.apache.drill.exec.serialization.InstanceSerializer; +import org.apache.drill.exec.store.sys.BasePersistentStore; +import org.apache.drill.exec.store.sys.PersistentStoreConfig; +import org.apache.drill.exec.store.sys.PersistentStoreMode; +import org.apache.zookeeper.CreateMode; + +/** + * This is the abstract class that is shared by ZkPStore (Persistent store) and ZkEStore (Ephemeral Store) --- End diff -- done --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/374#discussion_r52950576
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/coord/local/MapBackedStore.java
---
@@ -0,0 +1,86 @@
+/**
+ * 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.drill.exec.coord.local;
+
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.Map;
+
+import com.google.common.collect.Maps;
+import org.apache.drill.exec.coord.store.BaseTransientStore;
+import org.apache.drill.exec.coord.store.TransientStoreConfig;
+import org.apache.drill.exec.coord.store.TransientStoreEvent;
+import org.apache.drill.exec.coord.store.TransientStoreEventType;
+
+public class MapBackedStore extends BaseTransientStore {
+ private final HashMap delegate = Maps.newHashMap();
--- End diff --
done.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/374#discussion_r52950533
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKClusterCoordinator.java
---
@@ -206,6 +205,16 @@ public DistributedSemaphore getSemaphore(String name,
int maximumLeases) {
return new ZkDistributedSemaphore(curator, "/semaphore/" + name,
maximumLeases);
}
+ @Override
+ public TransientStore newTransientStore(final
TransientStoreConfig config) {
+final ZkEphemeralStore store = new ZkEphemeralStore<>(config,
curator);
--- End diff --
done.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/374#discussion_r52950554
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/serialization/JacksonSerializer.java
---
@@ -0,0 +1,59 @@
+/**
+ * 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.drill.exec.serialization;
+
+import java.io.IOException;
+import java.util.Objects;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.ObjectReader;
+import com.fasterxml.jackson.databind.ObjectWriter;
+
+public class JacksonSerializer implements InstanceSerializer {
+ private final ObjectReader reader;
+ private final ObjectWriter writer;
+
+ public JacksonSerializer(final ObjectMapper mapper, final Class
klazz) {
+this.reader = mapper.reader(klazz);
+this.writer = mapper.writer();
+ }
+
+ @Override
+ public T deserialize(final byte[] raw) throws IOException {
+return reader.readValue(raw);
+ }
+
+ @Override
+ public byte[] serialize(final T instance) throws IOException {
+return writer.writeValueAsBytes(instance);
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+if (obj instanceof JacksonSerializer &&
obj.getClass().equals(getClass())) {
+ final JacksonSerializer other = (JacksonSerializer)obj;
+ return Objects.equals(reader, other.reader) &&
Objects.equals(writer, other.writer);
+}
+return false;
+ }
+
+ @Override
+ public int hashCode() {
+return super.hashCode();
--- End diff --
nice catch. thanks. done.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/374#discussion_r52950999
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/coord/ClusterCoordinator.java
---
@@ -60,16 +62,23 @@
public abstract DistributedSemaphore getSemaphore(String name, int
maximumLeases);
/**
+ * Returns a new {@link TransientStore store} instance with the given
{@link TransientStoreConfig configuration}.
+ * @param config store configuration
+ * @param value type for this store
+ */
+ public abstract TransientStore
newTransientStore(TransientStoreConfig config);
+
+ /**
* Actions to take when there are a set of new de-active drillbits.
* @param unregisteredBits
*/
- public void drillbitUnregistered(Set unregisteredBits)
{
+ protected void drillbitUnregistered(Set
unregisteredBits) {
--- End diff --
Wrong use of this method outside of subclasses could easily mess the system
up. To my understanding, this method should only be visible to implementor who
is the ultimate authority to fire listeners on topology changes, so protected
seems to make more sense here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/374#discussion_r52951273
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/PathUtils.java ---
@@ -0,0 +1,59 @@
+/**
+ * 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.drill.exec.coord.zk;
+
+import com.google.common.base.Preconditions;
+import org.apache.parquet.Strings;
+
+public final class PathUtils {
--- End diff --
done.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/374#discussion_r52951541
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/coord/store/TransientStoreListener.java
---
@@ -0,0 +1,22 @@
+/**
+ * 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.drill.exec.coord.store;
+
+public interface TransientStoreListener {
+ void onChange(TransientStoreEvent event);
--- End diff --
I typically prefer this way as it allows extending capabilities(esp event
types) via polymorphism without requiring to alter the interface. I am fine
with either way though.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/374#discussion_r52951701
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/serialization/InstanceSerializer.java
---
@@ -6,20 +6,20 @@
* 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.drill.exec.store.sys.serialize;
+package org.apache.drill.exec.serialization;
import java.io.IOException;
-public interface PClassSerializer {
- public byte[] serialize(X val) throws IOException;
- public X deserialize(byte[] bytes) throws IOException;
+public interface InstanceSerializer {
+ byte[] serialize(T instance) throws IOException;
--- End diff --
Sounds good. I can do that on a follow-up issue without messing with RPC on
this patch.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...
Github user hnfgns commented on the pull request: https://github.com/apache/drill/pull/374#issuecomment-184436146 I have retracted unrelated mongo changes. Thanks for the feedback. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/374#discussion_r52953797
--- Diff:
common/src/main/java/org/apache/drill/common/collections/ImmutableEntry.java ---
@@ -0,0 +1,62 @@
+/**
+ * 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.drill.common.collections;
+
+import java.util.Map;
+
+import com.google.common.base.Objects;
+import com.google.common.base.Preconditions;
+
+public class ImmutableEntry implements Map.Entry {
+ private final K key;
+ private final V value;
+
+ public ImmutableEntry(final K key, final V value) {
+this.key = Preconditions.checkNotNull(key, "key is required");
+this.value = Preconditions.checkNotNull(value, "value is required");
+ }
+
+ @Override
+ public K getKey() {
+return key;
+ }
+
+ @Override
+ public V getValue() {
+return value;
+ }
+
+ @Override
+ public V setValue(final V value) {
+throw new UnsupportedOperationException("entry is immutable");
+ }
+
+ @Override
+ public boolean equals(final Object other) {
+if (other instanceof ImmutableEntry && other.getClass() == getClass())
{
+ final ImmutableEntry entry = (ImmutableEntry)other;
+ return Objects.equal(key, entry.key) && Objects.equal(value,
entry.value);
--- End diff --
That sure makes sense. You are probably looking at the cutting edge. We are
on guava-18 that does not deprecate these. Thanks for the reminder. Going
forward we should keep that in mind.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/374#discussion_r52953875
--- Diff:
contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/config/HBasePersistentStore.java
---
@@ -148,13 +154,13 @@ private void delete(byte[] row) {
private Result current = null;
private Result last = null;
private boolean done = false;
-private int rowsRead = 0;
-Iter() {
+Iter(int take) {
try {
Scan scan = new Scan(tableNameStartKey, tableNameStopKey);
scan.addColumn(FAMILY, QUALIFIER);
-scan.setCaching(config.getMaxIteratorSize() > 100 ? 100 :
config.getMaxIteratorSize());
+scan.setCaching(Math.min(take, 100));
--- End diff --
nope. this patch introduces a way to paginate results via #getRange. this
option is no longer needed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/374#discussion_r52953970
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/coord/store/TransientStoreListener.java
---
@@ -0,0 +1,22 @@
+/**
+ * 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.drill.exec.coord.store;
+
+public interface TransientStoreListener {
--- End diff --
done.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/374#discussion_r52954081
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZkEphemeralStore.java
---
@@ -0,0 +1,145 @@
+/**
+ * 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.drill.exec.coord.zk;
+
+import java.io.IOException;
+import java.util.Iterator;
+import java.util.Map;
+
+import javax.annotation.Nullable;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Function;
+import com.google.common.collect.Iterators;
+import com.google.common.collect.Lists;
+import org.apache.curator.framework.CuratorFramework;
+import
org.apache.curator.framework.recipes.cache.PathChildrenCacheListener;
+import org.apache.drill.common.collections.ImmutableEntry;
+import org.apache.drill.common.exceptions.DrillRuntimeException;
+import org.apache.drill.exec.coord.store.BaseTransientStore;
+import org.apache.drill.exec.coord.store.TransientStoreConfig;
+import org.apache.drill.exec.coord.store.TransientStoreEvent;
+import org.apache.drill.exec.serialization.InstanceSerializer;
+import org.apache.zookeeper.CreateMode;
+
+public class ZkEphemeralStore extends BaseTransientStore {
+
+ @VisibleForTesting
+ protected final PathChildrenCacheListener dispatcher = new
EventDispatcher<>(this);
+ private final ZookeeperClient client;
+
+ public ZkEphemeralStore(final TransientStoreConfig config, final
CuratorFramework curator) {
+super(config);
+this.client = new ZookeeperClient(curator, PathUtils.join("/",
config.getName()), CreateMode.EPHEMERAL);
+ }
+
+ public void start() throws Exception {
+getClient().getCache().getListenable().addListener(dispatcher);
+getClient().start();
+ }
+
+ protected ZookeeperClient getClient() {
+return client;
+ }
+
+ @Override
+ public V get(final String key) {
+final byte[] bytes = getClient().get(key);
+if (bytes == null) {
+ return null;
+}
+try {
+ return config.getSerializer().deserialize(bytes);
+} catch (final IOException e) {
+ throw new DrillRuntimeException(String.format("unable to deserialize
value at %s", key), e);
+}
+ }
+
+ @Override
+ public V put(final String key, final V value) {
+final InstanceSerializer serializer = config.getSerializer();
+try {
+ final byte[] old = getClient().get(key);
+ final byte[] bytes = serializer.serialize(value);
+ getClient().put(key, bytes);
+ if (old == null) {
+return null;
+ }
+ return serializer.deserialize(old);
+} catch (final IOException e) {
+ throw new DrillRuntimeException(String.format("unable to
de/serialize value of type %s", value.getClass()), e);
+}
+ }
+
+ @Override
+ public V putIfAbsent(final String key, final V value) {
+final V old = get(key);
+if (old == null) {
+ try {
+final byte[] bytes = config.getSerializer().serialize(value);
+getClient().put(key, bytes);
+ } catch (final IOException e) {
+throw new DrillRuntimeException(String.format("unable to serialize
value of type %s", value.getClass()), e);
+ }
+}
+return old;
+ }
+
+ @Override
+ public V remove(final String key) {
+final V existing = get(key);
+if (existing != null) {
+ getClient().delete(key);
+}
+return existing;
+ }
+
+ @Override
+ public Iterator> entries() {
+return Iterators.transform(getClient().entries(), new
Function, Map.Entry>() {
+ @Nullable
+ @Override
+ public Map.Entry apply(@Nullable Map.Entry input) {
+try {
+
[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/374#discussion_r52954122
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java
---
@@ -146,38 +155,37 @@ public QProfiles(List runningQueries,
List finishedQue
@Path("/profiles.json")
@Produces(MediaType.APPLICATION_JSON)
public QProfiles getProfilesJSON() {
-PStore completed = null;
-PStore running = null;
-try {
- completed = provider().getStore(QueryManager.QUERY_PROFILE);
- running = provider().getStore(QueryManager.RUNNING_QUERY_INFO);
-} catch (IOException e) {
- logger.debug("Failed to get profiles from persistent or ephemeral
store.");
- return new QProfiles(new ArrayList(), new
ArrayList());
-}
-
-List runningQueries = Lists.newArrayList();
-
-for (Map.Entry entry : running) {
- QueryInfo profile = entry.getValue();
- if (principal.canManageProfileOf(profile.getUser())) {
-runningQueries.add(new ProfileInfo(entry.getKey(),
profile.getStart(), profile.getForeman().getAddress(),
-profile.getQuery(), profile.getState().name(),
profile.getUser()));
+try (
+final PersistentStore completed =
getProvider().getStore(QueryManager.QUERY_PROFILE);
+final TransientStore running =
getCoordinator().newTransientStore(QueryManager.RUNNING_QUERY_INFO);
--- End diff --
nope instances are now cached. I will rename the method to reflect that
change.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/374#discussion_r52954225
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/PersistentStore.java
---
@@ -0,0 +1,77 @@
+/**
+ * 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.drill.exec.store.sys;
+
+import java.util.Iterator;
+import java.util.Map;
+
+/**
+ * An abstraction used to store and retrieve instances of given value type.
+ *
+ * @param value type
+ */
+public interface PersistentStore extends AutoCloseable {
--- End diff --
Mainly because of pagination and being more explicit both at interface and
pagination control level.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/374#discussion_r52954240
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/PersistentStoreProvider.java
---
@@ -17,17 +17,26 @@
*/
package org.apache.drill.exec.store.sys;
-import java.util.Map;
-
+import org.apache.drill.exec.exception.StoreException;
/**
- * Interface for reading and writing values to a persistent storage
provider. Iterators are guaranteed to be returned in key order.
- * @param
+ * A factory used to create {@link PersistentStore store} instances.
+ *
*/
-public interface PStore extends Iterable> {
- public V get(String key);
- public void put(String key, V value);
- public boolean putIfAbsent(String key, V value);
- public void delete(String key);
- public void close();
+public interface PersistentStoreProvider extends AutoCloseable {
+ /**
+ * Gets or creates a {@link PersistentStore persistent store} for the
given configuration.
+ *
+ * Note that implementors have liberty to cache previous {@link
PersistentStore store} instances.
+ *
+ * @param config store configuration
+ * @param store value type
+ */
+ PersistentStore getStore(PersistentStoreConfig config) throws
StoreException;
--- End diff --
done.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/374#discussion_r52954382
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryManager.java
---
@@ -237,11 +237,17 @@ void unpauseExecutingFragments(final DrillbitContext
drillbitContext) {
}
}
+ @Override
+ public void close() throws Exception {
+profileStore.close();
--- End diff --
done.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...
Github user hnfgns commented on a diff in the pull request: https://github.com/apache/drill/pull/374#discussion_r52955096 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZookeeperClient.java --- @@ -0,0 +1,238 @@ +/** + * 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.drill.exec.coord.zk; + +import java.util.Iterator; +import java.util.Map; + +import javax.annotation.Nullable; + +import com.google.common.base.Function; +import com.google.common.base.Preconditions; +import com.google.common.base.Strings; +import com.google.common.collect.Iterables; +import org.apache.curator.framework.CuratorFramework; +import org.apache.curator.framework.recipes.cache.ChildData; +import org.apache.curator.framework.recipes.cache.PathChildrenCache; +import org.apache.drill.common.collections.ImmutableEntry; +import org.apache.drill.common.exceptions.DrillRuntimeException; +import org.apache.zookeeper.CreateMode; + +/** + * A namespace aware Zookeper client. --- End diff -- seriously :+1: --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/374#discussion_r52955131
--- Diff:
contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/config/HBasePersistentStoreProvider.java
---
@@ -41,10 +39,8 @@
import org.apache.hadoop.hbase.client.HTableInterface;
import org.apache.hadoop.hbase.util.Bytes;
-import com.google.common.annotations.VisibleForTesting;
-
-public class HBasePStoreProvider implements PStoreProvider {
- static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(HBasePStoreProvider.class);
+public class HBasePersistentStoreProvider extends
BasePersistentStoreProvider {
+ static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(HBasePersistentStoreProvider.class);
--- End diff --
not relevant to my changes but did my best to privatize them all ;)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/374#discussion_r52955476
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/coord/local/LocalClusterCoordinator.java
---
@@ -125,6 +127,11 @@ public DistributedSemaphore getSemaphore(final String
name, final int maximumLea
return semaphores.get(name);
}
+ @Override
+ public TransientStore getOrCreateTransientStore(final
TransientStoreConfig config) {
+return new MapBackedStore<>(config);
--- End diff --
I think it's quite cheap here to do so here. Added caching too.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/374#discussion_r52956885
--- Diff: exec/java-exec/src/main/resources/drill-module.conf ---
@@ -120,7 +120,7 @@ drill.exec: {
affinity.factor: 1.2
},
sys.store.provider: {
-class: "org.apache.drill.exec.store.sys.zk.ZkPStoreProvider",
+class:
"org.apache.drill.exec.store.sys.store.provider.ZookeeperPersistentStoreProvider",
--- End diff --
The new patch includes a deprecated ZkPStoreProvider that is to be removed
in 1.7. Now old configuration should work as is. We should also state this out
in 1.6 release notes. Let me know if you guys have any other concerns as for
compatibility.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/374#discussion_r52957167
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKClusterCoordinator.java
---
@@ -206,6 +209,16 @@ public DistributedSemaphore getSemaphore(String name,
int maximumLeases) {
return new ZkDistributedSemaphore(curator, "/semaphore/" + name,
maximumLeases);
}
+ @Override
+ public TransientStore getOrCreateTransientStore(final
TransientStoreConfig config) {
+final ZkEphemeralStore store =
(ZkEphemeralStore)factory.getOrCreateStore(config);
+try {
+ store.start();
--- End diff --
That logic now is in factory which calls start once.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/374#discussion_r52957306
--- Diff:
common/src/main/java/org/apache/drill/common/collections/ImmutableEntry.java ---
@@ -0,0 +1,62 @@
+/**
+ * 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.drill.common.collections;
+
+import java.util.Map;
+
+import com.google.common.base.Objects;
+import com.google.common.base.Preconditions;
+
+public class ImmutableEntry implements Map.Entry {
+ private final K key;
+ private final V value;
+
+ public ImmutableEntry(final K key, final V value) {
+this.key = Preconditions.checkNotNull(key, "key is required");
+this.value = Preconditions.checkNotNull(value, "value is required");
+ }
+
+ @Override
+ public K getKey() {
+return key;
+ }
+
+ @Override
+ public V getValue() {
+return value;
+ }
+
+ @Override
+ public V setValue(final V value) {
+throw new UnsupportedOperationException("entry is immutable");
+ }
+
+ @Override
+ public boolean equals(final Object other) {
+if (other instanceof ImmutableEntry && other.getClass() == getClass())
{
+ final ImmutableEntry entry = (ImmutableEntry)other;
+ return Objects.equal(key, entry.key) && Objects.equal(value,
entry.value);
--- End diff --
Cool. I think we should consider replacing all uses on a separate patch
once this is deprecated.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/374#discussion_r52958072
--- Diff: exec/java-exec/src/main/resources/drill-module.conf ---
@@ -120,7 +120,7 @@ drill.exec: {
affinity.factor: 1.2
},
sys.store.provider: {
-class: "org.apache.drill.exec.store.sys.zk.ZkPStoreProvider",
+class:
"org.apache.drill.exec.store.sys.store.provider.ZookeeperPersistentStoreProvider",
--- End diff --
All included.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...
Github user hnfgns commented on the pull request: https://github.com/apache/drill/pull/374#issuecomment-184463952 Please let me know of further comments and remember to +1 if you are comfy with the patch. If nothing severe is seen, let's take all comments on follow-up PRs. This patch is long waiting I'd like to check this in. Thanks for all feedback. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/374#discussion_r52962000
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java
---
@@ -146,38 +154,37 @@ public QProfiles(List runningQueries,
List finishedQue
@Path("/profiles.json")
@Produces(MediaType.APPLICATION_JSON)
public QProfiles getProfilesJSON() {
-PStore completed = null;
-PStore running = null;
-try {
- completed = provider().getStore(QueryManager.QUERY_PROFILE);
- running = provider().getStore(QueryManager.RUNNING_QUERY_INFO);
-} catch (IOException e) {
- logger.debug("Failed to get profiles from persistent or ephemeral
store.");
- return new QProfiles(new ArrayList(), new
ArrayList());
-}
-
-List runningQueries = Lists.newArrayList();
-
-for (Map.Entry entry : running) {
- QueryInfo profile = entry.getValue();
- if (principal.canManageProfileOf(profile.getUser())) {
-runningQueries.add(new ProfileInfo(entry.getKey(),
profile.getStart(), profile.getForeman().getAddress(),
-profile.getQuery(), profile.getState().name(),
profile.getUser()));
+try (
+final PersistentStore completed =
getProvider().getOrCreateStore(QueryManager.QUERY_PROFILE);
--- End diff --
Fixed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/374#discussion_r52962203
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/store/provider/CachingPersistentStoreProvider.java
---
@@ -0,0 +1,72 @@
+/**
+ * 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.drill.exec.store.sys.store.provider;
+
+import java.util.concurrent.ConcurrentMap;
+
+import com.google.common.collect.Maps;
+import org.apache.drill.exec.exception.StoreException;
+import org.apache.drill.exec.store.sys.PersistentStore;
+import org.apache.drill.exec.store.sys.PersistentStoreConfig;
+import org.apache.drill.exec.store.sys.PersistentStoreProvider;
+
+public class CachingPersistentStoreProvider extends
BasePersistentStoreProvider {
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(CachingPersistentStoreProvider.class);
+
+ private final ConcurrentMap,
PersistentStore> storeCache = Maps.newConcurrentMap();
+ private final PersistentStoreProvider provider;
+
+ public CachingPersistentStoreProvider(PersistentStoreProvider provider) {
+this.provider = provider;
+ }
+
+ @SuppressWarnings("unchecked")
+ public PersistentStore getOrCreateStore(final
PersistentStoreConfig config) throws StoreException {
+final PersistentStore store = storeCache.get(config);
+if (store == null) {
+ final PersistentStore newStore =
provider.getOrCreateStore(config);
+ final PersistentStore finalStore = storeCache.putIfAbsent(config,
newStore);
+ if (finalStore == null) {
+return (PersistentStore)newStore;
+ }
+ try {
+newStore.close();
+ } catch (Exception ex) {
+throw new StoreException(ex);
+ }
+}
+
+return (PersistentStore) store;
+
+ }
+
+ @Override
+ public void start() throws Exception {
+provider.start();
+ }
+
+ @Override
+ public void close() throws Exception {
+for(final PersistentStore store : storeCache.values()){
+ store.close();
--- End diff --
AutoCloseables.close(storecache.values() + provider). Done.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/374#discussion_r52962256
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/store/provider/ZookeeperPersistentStoreProvider.java
---
@@ -0,0 +1,85 @@
+/**
+ * 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.drill.exec.store.sys.store.provider;
+
+import java.io.IOException;
+
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.curator.framework.CuratorFramework;
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.exec.coord.zk.ZKClusterCoordinator;
+import org.apache.drill.exec.exception.StoreException;
+import org.apache.drill.exec.store.dfs.DrillFileSystem;
+import org.apache.drill.exec.store.sys.PersistentStore;
+import org.apache.drill.exec.store.sys.PersistentStoreRegistry;
+import org.apache.drill.exec.store.sys.PersistentStoreConfig;
+import org.apache.drill.exec.store.sys.store.LocalPersistentStore;
+import org.apache.drill.exec.store.sys.store.ZookeeperPersistentStore;
+import org.apache.hadoop.fs.Path;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ZookeeperPersistentStoreProvider extends
BasePersistentStoreProvider {
+ private static final Logger logger =
LoggerFactory.getLogger(ZookeeperPersistentStoreProvider.class);
+
+ private static final String DRILL_EXEC_SYS_STORE_PROVIDER_ZK_BLOBROOT =
"drill.exec.sys.store.provider.zk.blobroot";
+
+ private final CuratorFramework curator;
+ private final DrillFileSystem fs;
+ private final Path blobRoot;
+
+ public ZookeeperPersistentStoreProvider(final
PersistentStoreRegistry registry) throws StoreException {
+this(registry.getConfig(), registry.getCoordinator().getCurator());
+ }
+
+ @VisibleForTesting
+ public ZookeeperPersistentStoreProvider(final DrillConfig config, final
CuratorFramework curator) throws StoreException {
+this.curator = curator;
+
+if (config.hasPath(DRILL_EXEC_SYS_STORE_PROVIDER_ZK_BLOBROOT)) {
+ blobRoot = new
Path(config.getString(DRILL_EXEC_SYS_STORE_PROVIDER_ZK_BLOBROOT));
+}else{
+ blobRoot = LocalPersistentStore.getLogDir();
+}
+
+try {
+ this.fs = LocalPersistentStore.getFileSystem(config, blobRoot);
+} catch (IOException ex) {
+ throw new StoreException("unable to get filesystem", ex);
+}
+ }
+
+ @Override
+ public PersistentStore getOrCreateStore(final
PersistentStoreConfig config) throws StoreException {
+switch(config.getMode()){
+case BLOB_PERSISTENT:
+ return new LocalPersistentStore<>(fs, blobRoot, config);
--- End diff --
zk relays blob persistency to fs.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/374#discussion_r52962267
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/store/provider/LocalPersistentStoreProvider.java
---
@@ -0,0 +1,74 @@
+/**
+ * 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.drill.exec.store.sys.store.provider;
+
+import java.io.IOException;
+
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.exec.ExecConstants;
+import org.apache.drill.exec.exception.StoreException;
+import org.apache.drill.exec.store.dfs.DrillFileSystem;
+import org.apache.drill.exec.store.sys.PersistentStoreRegistry;
+import org.apache.drill.exec.store.sys.PersistentStore;
+import org.apache.drill.exec.store.sys.PersistentStoreConfig;
+import org.apache.drill.exec.store.sys.store.LocalPersistentStore;
+import org.apache.drill.exec.testing.store.NoWriteLocalStore;
+import org.apache.hadoop.fs.Path;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * A really simple provider that stores data in the local file system, one
value per file.
+ */
+public class LocalPersistentStoreProvider extends
BasePersistentStoreProvider {
+ private static final Logger logger =
LoggerFactory.getLogger(LocalPersistentStoreProvider.class);
+
+ private final Path path;
+ private final DrillFileSystem fs;
--- End diff --
done.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 4372 for review
Github user hnfgns commented on the pull request: https://github.com/apache/drill/pull/377#issuecomment-186424169 As a general reminder, we should use a more descriptive commit message. It is useful to understand what this patch is about just by looking at the commit message. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4416: quote path separator for cross pla...
GitHub user hnfgns opened a pull request: https://github.com/apache/drill/pull/385 DRILL-4416: quote path separator for cross platform compatibility You can merge this pull request into a Git repository by running: $ git pull https://github.com/hnfgns/incubator-drill DRILL-4416 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/385.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 #385 commit 955cdc80413d7f66cfb50352974c13e346bad9e1 Author: Hanifi Gunes Date: 2016-02-20T00:23:08Z DRILL-4416: quote path separator for cross platform compatibility --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3488: Allow Java 1.8
Github user hnfgns commented on the pull request: https://github.com/apache/drill/pull/388#issuecomment-191448090 +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4416: quote path separator for cross pla...
Github user hnfgns commented on the pull request: https://github.com/apache/drill/pull/385#issuecomment-192074959 This patch causes a random leak. I am backing it off for a while. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4313: Improve method of picking a random...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/396#discussion_r55286410
--- Diff: contrib/native/client/example/querySubmitter.cpp ---
@@ -317,16 +320,20 @@ int main(int argc, char* argv[]) {
std::vector::iterator queryHandleIter;
Drill::DrillClient client;
-// To log to file
-//DrillClient::initLogging("/var/log/drill/", l);
+#if defined _WIN32 || defined _WIN64
+ const char* logpathPrefix =
"C:\\Users\\Administrator\\Documents\\temp\\drillclient";
--- End diff --
minor: the use of GetTempPath in seems more appropriate here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4313: Improve method of picking a random...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/396#discussion_r55290473
--- Diff: contrib/native/client/src/clientlib/drillClientImpl.cpp ---
@@ -1392,6 +1390,206 @@ void DrillClientQueryResult::clearAndDestroy(){
}
}
+
+connectionStatus_t PooledDrillClientImpl::connect(const char* connStr){
+connectionStatus_t stat = CONN_SUCCESS;
+std::string pathToDrill, protocol, hostPortStr;
+std::string host;
+std::string port;
+m_connectStr=connStr;
+Utils::parseConnectStr(connStr, pathToDrill, protocol, hostPortStr);
+if(!strcmp(protocol.c_str(), "zk")){
+// Get a list of drillbits
+ZookeeperImpl zook;
+std::vector drillbits;
+int err = zook.getAllDrillbits(hostPortStr.c_str(),
pathToDrill.c_str(), drillbits);
+if(!err){
+Utils::shuffle(drillbits);
+// The original shuffled order is maintained if we shuffle
first and then add any missing elements
+Utils::add(m_drillbits, drillbits);
+exec::DrillbitEndpoint e;
+size_t nextIndex=0;
+{
+boost::lock_guard cLock(m_cMutex);
+m_lastConnection++;
+nextIndex = (m_lastConnection)%(getDrillbitCount());
+}
+DRILL_MT_LOG(DRILL_LOG(LOG_TRACE) << "Pooled Connection"
+<< "(" << (void*)this << ")"
+<< ": Current counter is: "
+<< m_lastConnection << std::endl;)
+err=zook.getEndPoint(m_drillbits, nextIndex, e);
+if(!err){
+host=boost::lexical_cast(e.address());
+port=boost::lexical_cast(e.user_port());
+}
+}
+if(err){
+return handleConnError(CONN_ZOOKEEPER_ERROR,
getMessage(ERR_CONN_ZOOKEEPER, zook.getError().c_str()));
+}
+zook.close();
+m_bIsDirectConnection=false;
+}else if(!strcmp(protocol.c_str(), "local")){
+char tempStr[MAX_CONNECT_STR+1];
+strncpy(tempStr, hostPortStr.c_str(), MAX_CONNECT_STR);
tempStr[MAX_CONNECT_STR]=0;
+host=strtok(tempStr, ":");
+port=strtok(NULL, "");
+m_bIsDirectConnection=true;
+}else{
+return handleConnError(CONN_INVALID_INPUT,
getMessage(ERR_CONN_UNKPROTO, protocol.c_str()));
+}
+DRILL_MT_LOG(DRILL_LOG(LOG_TRACE) << "Connecting to endpoint: (Pooled)
" << host << ":" << port << std::endl;)
+DrillClientImpl* pDrillClientImpl = new DrillClientImpl();
+stat = pDrillClientImpl->connect(host.c_str(), port.c_str());
+if(stat == CONN_SUCCESS){
+boost::lock_guard lock(m_poolMutex);
+m_clientConnections.push_back(pDrillClientImpl);
+}else{
+DrillClientError* pErr = pDrillClientImpl->getError();
+handleConnError((connectionStatus_t)pErr->status, pErr->msg);
+delete pDrillClientImpl;
+}
+return stat;
+}
+
+connectionStatus_t
PooledDrillClientImpl::validateHandshake(DrillUserProperties* props){
+// Assume there is one valid connection to at least one drillbit
+connectionStatus_t stat=CONN_FAILURE;
+// Keep a copy of the user properties
+if(props!=NULL){
+m_pUserProperties = new DrillUserProperties;
+for(size_t i=0; isize(); i++){
+m_pUserProperties->setProperty(
+props->keyAt(i),
+props->valueAt(i)
+);
+}
+}
+DrillClientImpl* pDrillClientImpl = getOneConnection();
+if(pDrillClientImpl != NULL){
+DRILL_MT_LOG(DRILL_LOG(LOG_TRACE) << "Validating handshake:
(Pooled) " << pDrillClientImpl->m_connectedHost << std::endl;)
+stat=pDrillClientImpl->validateHandshake(m_pUserProperties);
+}
+else{
+stat = handleConnError(CONN_NOTCONNECTED,
getMessage(ERR_CONN_NOCONN));
+}
+return stat;
+}
+
+DrillClientQueryResult*
PooledDrillClientImpl::SubmitQuery(::exec::shared::QueryType t, const
std::string& plan, pfnQueryResultsListener listener, void* listenerCtx){
+DrillClientQueryResult* pDrillClientQueryResult = NULL;
+DrillClientImpl* pDrillClientImpl = NULL;
+pDrillClientImpl = getOneConnection();
+if(pDrillClientImpl != NULL){
+
pDrillClientQueryResult=pDrillClientImp
[GitHub] drill pull request: DRILL-4313: Improve method of picking a random...
Github user hnfgns commented on the pull request: https://github.com/apache/drill/pull/396#issuecomment-193499159 +1 once comments above are addressed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4443: MIN/MAX on VARCHAR throw a NullPoi...
Github user hnfgns commented on the pull request: https://github.com/apache/drill/pull/409#issuecomment-193500314 +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4479: Use varchar for default column whe...
Github user hnfgns commented on the pull request: https://github.com/apache/drill/pull/420#issuecomment-196464641 +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4504: Create an event loop for each of [...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/429#discussion_r56058208
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java ---
@@ -74,73 +74,148 @@
/**
* Thin wrapper around a UserClient that handles connect/close and
transforms
* String into ByteBuf.
+ *
+ * Use the builder class ({@link DrillClient.Builder}) to build objects of
this class.
+ * E.g.
+ *
+ * DrillClient client = DrillClient.newBuilder()
+ * .setConfig(...)
+ * .setIsDirectConnection(true)
+ * .build();
+ *
*/
public class DrillClient implements Closeable, ConnectionThrottle {
private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(DrillClient.class);
private final DrillConfig config;
- private UserClient client;
- private UserProperties props = null;
- private volatile ClusterCoordinator clusterCoordinator;
- private volatile boolean connected = false;
private final BufferAllocator allocator;
- private int reconnectTimes;
- private int reconnectDelay;
- private boolean supportComplexTypes;
- private final boolean ownsZkConnection;
+ private final boolean isDirectConnection;
+ private final int reconnectTimes;
+ private final int reconnectDelay;
+
+ // checks if this client owns these resources (used when closing)
private final boolean ownsAllocator;
- private final boolean isDirectConnection; // true if the connection
bypasses zookeeper and connects directly to a drillbit
+ private final boolean ownsZkConnection;
+ private final boolean ownsEventLoopGroup;
+ private final boolean ownsExecutor;
+
+ // if the following variables are set during construction, they are not
overridden during or after #connect call
+ // otherwise, they are set to defaults during #connect call
private EventLoopGroup eventLoopGroup;
private ExecutorService executor;
+ private boolean supportComplexTypes;
+
+ // the following variables are set during connection, and must not be
overridden later
+ private UserClient client;
+ private UserProperties props;
+ private volatile ClusterCoordinator clusterCoordinator;
--- End diff --
-0.
Why volatile here? Is DrillClient meant to be thread safe? If so, we seem
to have more work to do: #close for instance. Otherwise volatile seems totally
irrelevant.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4504: Create an event loop for each of [...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/429#discussion_r56060382
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java ---
@@ -74,73 +74,148 @@
/**
* Thin wrapper around a UserClient that handles connect/close and
transforms
* String into ByteBuf.
+ *
+ * Use the builder class ({@link DrillClient.Builder}) to build objects of
this class.
+ * E.g.
+ *
+ * DrillClient client = DrillClient.newBuilder()
+ * .setConfig(...)
+ * .setIsDirectConnection(true)
+ * .build();
+ *
*/
public class DrillClient implements Closeable, ConnectionThrottle {
private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(DrillClient.class);
private final DrillConfig config;
- private UserClient client;
- private UserProperties props = null;
- private volatile ClusterCoordinator clusterCoordinator;
- private volatile boolean connected = false;
private final BufferAllocator allocator;
- private int reconnectTimes;
- private int reconnectDelay;
- private boolean supportComplexTypes;
- private final boolean ownsZkConnection;
+ private final boolean isDirectConnection;
+ private final int reconnectTimes;
+ private final int reconnectDelay;
+
+ // checks if this client owns these resources (used when closing)
private final boolean ownsAllocator;
- private final boolean isDirectConnection; // true if the connection
bypasses zookeeper and connects directly to a drillbit
+ private final boolean ownsZkConnection;
+ private final boolean ownsEventLoopGroup;
+ private final boolean ownsExecutor;
+
+ // if the following variables are set during construction, they are not
overridden during or after #connect call
+ // otherwise, they are set to defaults during #connect call
private EventLoopGroup eventLoopGroup;
private ExecutorService executor;
+ private boolean supportComplexTypes;
+
+ // the following variables are set during connection, and must not be
overridden later
+ private UserClient client;
+ private UserProperties props;
+ private volatile ClusterCoordinator clusterCoordinator;
+ private volatile boolean connected; // = false
- public DrillClient() throws OutOfMemoryException {
+ /**
+ * @deprecated Create a DrillClient using {@link DrillClient.Builder}.
+ */
+ @Deprecated
+ public DrillClient() {
this(DrillConfig.create(), false);
}
- public DrillClient(boolean isDirect) throws OutOfMemoryException {
+ /**
+ * @deprecated Create a DrillClient using {@link DrillClient.Builder}.
+ */
+ @Deprecated
+ public DrillClient(boolean isDirect) {
this(DrillConfig.create(), isDirect);
}
- public DrillClient(String fileName) throws OutOfMemoryException {
+ /**
+ * @deprecated Create a DrillClient using {@link DrillClient.Builder}.
+ */
+ @Deprecated
+ public DrillClient(String fileName) {
this(DrillConfig.create(fileName), false);
}
- public DrillClient(DrillConfig config) throws OutOfMemoryException {
+ /**
+ * @deprecated Create a DrillClient using {@link DrillClient.Builder}.
+ */
+ @Deprecated
+ public DrillClient(DrillConfig config) {
this(config, null, false);
}
- public DrillClient(DrillConfig config, boolean isDirect)
- throws OutOfMemoryException {
+ /**
+ * @deprecated Create a DrillClient using {@link DrillClient.Builder}.
+ */
+ @Deprecated
+ public DrillClient(DrillConfig config, boolean isDirect) {
this(config, null, isDirect);
}
- public DrillClient(DrillConfig config, ClusterCoordinator coordinator)
-throws OutOfMemoryException {
+ /**
+ * @deprecated Create a DrillClient using {@link DrillClient.Builder}.
+ */
+ @Deprecated
+ public DrillClient(DrillConfig config, ClusterCoordinator coordinator) {
this(config, coordinator, null, false);
}
- public DrillClient(DrillConfig config, ClusterCoordinator coordinator,
boolean isDirect)
-throws OutOfMemoryException {
+ /**
+ * @deprecated Create a DrillClient using {@link DrillClient.Builder}.
+ */
+ @Deprecated
+ public DrillClient(DrillConfig config, ClusterCoordinator coordinator,
boolean isDirect) {
this(config, coordinator, null, isDirect);
}
- public DrillClient(DrillConfig config, ClusterCoordinator coordinator,
BufferAllocator allocator)
- throws OutOfMemoryException {
+ /**
+ * @deprecated Create a DrillClient using {@link DrillClient.Builder
[GitHub] drill pull request: DRILL-4514 : Add describe schema ...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/436#discussion_r56891737
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DescribeSchemaCommandResult.java
---
@@ -0,0 +1,30 @@
+/**
+ * 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.drill.exec.planner.sql.handlers;
+
+public class DescribeSchemaCommandResult {
+
+ public String name;
--- End diff --
You may want to make these final.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4514 : Add describe schema ...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/436#discussion_r56891914
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlDescribeSchema.java
---
@@ -0,0 +1,82 @@
+/**
+ * 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.drill.exec.planner.sql.parser;
+
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlIdentifier;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlLiteral;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.SqlSpecialOperator;
+import org.apache.calcite.sql.SqlWriter;
+import org.apache.calcite.sql.parser.SqlParserPos;
+import org.apache.drill.exec.planner.sql.handlers.AbstractSqlHandler;
+import org.apache.drill.exec.planner.sql.handlers.DescribeSchemaHandler;
+import org.apache.drill.exec.planner.sql.handlers.SqlHandlerConfig;
+
+import java.util.Collections;
+import java.util.List;
+
+/**
+ * Sql parse tree node to represent statement:
+ * SHOW FILES [{FROM | IN} db_name] [LIKE 'pattern' | WHERE expr]
+ */
+public class SqlDescribeSchema extends DrillSqlCall {
+
+ private final SqlIdentifier schema;
+
+ public static final SqlSpecialOperator OPERATOR =
+ new SqlSpecialOperator("DESCRIBE_SCHEMA", SqlKind.OTHER) {
+@Override
+public SqlCall createCall(SqlLiteral functionQualifier,
SqlParserPos pos, SqlNode... operands) {
+ return new SqlDescribeSchema(pos, (SqlIdentifier) operands[0]);
+}
+ };
+
+ public SqlDescribeSchema(SqlParserPos pos, SqlIdentifier schema) {
+super(pos);
+this.schema = schema;
+assert schema != null;
--- End diff --
What about a precondition here?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4514 : Add describe schema ...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/436#discussion_r56915385
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlDescribeSchema.java
---
@@ -0,0 +1,82 @@
+/**
+ * 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.drill.exec.planner.sql.parser;
+
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlIdentifier;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlLiteral;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.SqlSpecialOperator;
+import org.apache.calcite.sql.SqlWriter;
+import org.apache.calcite.sql.parser.SqlParserPos;
+import org.apache.drill.exec.planner.sql.handlers.AbstractSqlHandler;
+import org.apache.drill.exec.planner.sql.handlers.DescribeSchemaHandler;
+import org.apache.drill.exec.planner.sql.handlers.SqlHandlerConfig;
+
+import java.util.Collections;
+import java.util.List;
+
+/**
+ * Sql parse tree node to represent statement:
+ * SHOW FILES [{FROM | IN} db_name] [LIKE 'pattern' | WHERE expr]
+ */
+public class SqlDescribeSchema extends DrillSqlCall {
+
+ private final SqlIdentifier schema;
+
+ public static final SqlSpecialOperator OPERATOR =
+ new SqlSpecialOperator("DESCRIBE_SCHEMA", SqlKind.OTHER) {
+@Override
+public SqlCall createCall(SqlLiteral functionQualifier,
SqlParserPos pos, SqlNode... operands) {
+ return new SqlDescribeSchema(pos, (SqlIdentifier) operands[0]);
+}
+ };
+
+ public SqlDescribeSchema(SqlParserPos pos, SqlIdentifier schema) {
+super(pos);
+this.schema = schema;
+assert schema != null;
--- End diff --
Sure. I meant we should likely use an explicit null-checking pattern like
`this.schema = Preconditions.checkNotNull(schema, "msg")` instead of an
assertion here.
ps: Guava's Preconditions.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4317: Exceptions on SELECT and CTAS with...
Github user hnfgns commented on the pull request: https://github.com/apache/drill/pull/432#issuecomment-200498974 +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4132 Ability to submit simple type of ph...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/368#discussion_r61345892
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java ---
@@ -321,6 +329,38 @@ public void close() {
return listener.getResults();
}
+ public DrillRpcFuture planQuery(QueryType type,
String query, boolean isSplitPlan) {
+GetQueryPlanFragments runQuery =
GetQueryPlanFragments.newBuilder().setQuery(query).setType(type).setSplitPlan(isSplitPlan).build();
+return client.submitPlanQuery(runQuery);
+ }
+
+ public void runQuery(QueryType type, List planFragments,
UserResultsListener resultsListener)
+ throws RpcException {
+// QueryType can be only executional
+checkArgument((QueryType.EXECUTION == type), "Only EXECUTIONAL type
query is supported with PlanFragments");
+// setting Plan on RunQuery will be used for logging purposes and
therefore can not be null
+// since there is no Plan string provided we will create a JsonArray
out of individual fragment Plans
+ArrayNode jsonArray = objectMapper.createArrayNode();
+for (PlanFragment fragment : planFragments) {
+ try {
+jsonArray.add(objectMapper.readTree(fragment.getFragmentJson()));
+ } catch (IOException e) {
+logger.error("Exception while trying to read PlanFragment JSON for
%s", fragment.getHandle().getQueryId(), e);
+throw new RpcException(e);
+ }
+}
+final String fragmentsToJsonString;
+try {
+ fragmentsToJsonString = objectMapper.writeValueAsString(jsonArray);
--- End diff --
why don't we create & serialize List instead of relying on
ArrayNode?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4132 Ability to submit simple type of ph...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/368#discussion_r61346266
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/contrib/ExchangeRemoverMaterializer.java
---
@@ -0,0 +1,96 @@
+/**
+ * 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.drill.exec.planner.fragment.contrib;
+
+import java.util.List;
+
+import org.apache.drill.common.exceptions.ExecutionSetupException;
+import org.apache.drill.exec.exception.FragmentSetupException;
+import org.apache.drill.exec.physical.PhysicalOperatorSetupException;
+import org.apache.drill.exec.physical.base.AbstractPhysicalVisitor;
+import org.apache.drill.exec.physical.base.Exchange;
+import org.apache.drill.exec.physical.base.GroupScan;
+import org.apache.drill.exec.physical.base.PhysicalOperator;
+import org.apache.drill.exec.physical.base.Store;
+import org.apache.drill.exec.physical.base.SubScan;
+import org.apache.drill.exec.planner.fragment.Materializer;
+import
org.apache.drill.exec.planner.fragment.Materializer.IndexedFragmentNode;
+
+import com.google.common.collect.Lists;
+
+/**
+ * Materializer visitor to remove exchange(s)
+ * NOTE: this Visitor does NOT set OperatorId, as after Exchange removal
all operators need renumbering
+ * Use OperatorIdVisitor on top to set correct OperatorId
+ */
+public class ExchangeRemoverMaterializer extends
AbstractPhysicalVisitor {
+
+ static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(ExchangeRemoverMaterializer.class);
--- End diff --
private
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4132 Ability to submit simple type of ph...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/368#discussion_r61347199
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserClient.java ---
@@ -137,4 +142,9 @@ protected void finalizeConnection(BitToUserHandshake
handshake, BasicClientWithC
public ProtobufLengthDecoder getDecoder(BufferAllocator allocator) {
return new UserProtobufLengthDecoder(allocator,
OutOfMemoryHandler.DEFAULT_INSTANCE);
}
+
+ public DrillRpcFuture submitPlanQuery(
--- End diff --
Should we consider renaming this to planQuery(...) and add a small comment?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4132 Ability to submit simple type of ph...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/368#discussion_r61347587
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/util/Utilities.java ---
@@ -17,13 +17,27 @@
*/
package org.apache.drill.exec.util;
+import java.util.LinkedList;
+import java.util.List;
+
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.exec.ExecConstants;
import org.apache.drill.exec.expr.fn.impl.DateUtility;
+import org.apache.drill.exec.memory.RootAllocatorFactory;
import org.apache.drill.exec.ops.FragmentContext;
+import org.apache.drill.exec.ops.QueryContext;
+import org.apache.drill.exec.physical.PhysicalPlan;
+import org.apache.drill.exec.physical.base.PhysicalOperator;
+import org.apache.drill.exec.physical.config.ExternalSort;
import org.apache.drill.exec.proto.BitControl.QueryContextInformation;
import org.apache.drill.exec.proto.ExecProtos;
import org.apache.drill.exec.proto.helper.QueryIdHelper;
+import org.apache.drill.exec.server.options.OptionManager;
public class Utilities {
--- End diff --
I am against the anti-pattern of a generic Utilities class. It won't take
too long for us to turn into a junk yard of code :) I would propose that we
externalize the functionality here to its relevant utility class like SortUtils
or similar.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4132 Ability to submit simple type of ph...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/368#discussion_r61349687
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java ---
@@ -419,6 +425,64 @@ private void runPhysicalPlan(final PhysicalPlan plan)
throws ExecutionSetupExcep
logger.debug("Fragments running.");
}
+ /**
+ * This is a helper method to run query based on the list of
PlanFragment that were planned
+ * at some point of time
+ * @param fragmentsList
+ * @throws ExecutionSetupException
+ */
+ private void runFragment(List fragmentsList) throws
ExecutionSetupException {
+// need to set QueryId, MinorFragment for incoming Fragments
+PlanFragment rootFragment = null;
+boolean isFirst = true;
+final List planFragments = Lists.newArrayList();
+final int fragmentsCount = fragmentsList.size();
+for (PlanFragment myFragment : fragmentsList) {
+ final FragmentHandle handle = myFragment.getHandle();
+ // for split plan number of minor fragments will be always one,
+ // but minor fragmentId may not be 0, as it is one of the minor
fragments from split plan
+ final int minorFragment = (fragmentsCount == 1) ? 0 :
handle.getMinorFragmentId();
--- End diff --
Even though I understand that a split plan consists of a single minor
fragment, I wonder if there is a better way to handle minor fragment numbering.
Why do not we just re-number minor fragments during planning/splitting phase at
the first place instead of relying on fragmentsCount == 1 check?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4132 Ability to submit simple type of ph...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/368#discussion_r61349858
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/work/user/QueryPlanFragmentsSplitHelper.java
---
@@ -0,0 +1,141 @@
+/**
+ * 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.drill.exec.work.user;
+
+import java.util.List;
+
+import org.apache.drill.exec.ops.QueryContext;
+import org.apache.drill.exec.physical.PhysicalPlan;
+import org.apache.drill.exec.physical.base.PhysicalOperator;
+import org.apache.drill.exec.planner.fragment.Fragment;
+import org.apache.drill.exec.planner.fragment.MakeFragmentsVisitor;
+import org.apache.drill.exec.planner.fragment.SimpleParallelizer;
+import
org.apache.drill.exec.planner.fragment.contrib.SimpleParallelizerMultiPlans;
+import org.apache.drill.exec.planner.sql.DrillSqlWorker;
+import org.apache.drill.exec.proto.BitControl.PlanFragment;
+import org.apache.drill.exec.proto.UserBitShared.DrillPBError;
+import org.apache.drill.exec.proto.UserBitShared.QueryId;
+import org.apache.drill.exec.proto.UserBitShared.QueryResult.QueryState;
+import org.apache.drill.exec.proto.UserProtos.GetQueryPlanFragments;
+import org.apache.drill.exec.proto.UserProtos.QueryPlanFragments;
+import org.apache.drill.exec.rpc.user.UserServer.UserClientConnection;
+import org.apache.drill.exec.server.DrillbitContext;
+import org.apache.drill.exec.util.Pointer;
+import org.apache.drill.exec.util.Utilities;
+import org.apache.drill.exec.work.QueryWorkUnit;
+
+import com.google.common.collect.Lists;
+
+/**
+ * Helper class to return PlanFragments based on the query plan
+ * or based on split query plan
+ *
+ */
+public class QueryPlanFragmentsSplitHelper {
--- End diff --
rename to QueryPlanSplit[ter or Helper]?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4132 Ability to submit simple type of ph...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/368#discussion_r61349902
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/work/user/QueryPlanFragmentsSplitHelper.java
---
@@ -0,0 +1,141 @@
+/**
+ * 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.drill.exec.work.user;
+
+import java.util.List;
+
+import org.apache.drill.exec.ops.QueryContext;
+import org.apache.drill.exec.physical.PhysicalPlan;
+import org.apache.drill.exec.physical.base.PhysicalOperator;
+import org.apache.drill.exec.planner.fragment.Fragment;
+import org.apache.drill.exec.planner.fragment.MakeFragmentsVisitor;
+import org.apache.drill.exec.planner.fragment.SimpleParallelizer;
+import
org.apache.drill.exec.planner.fragment.contrib.SimpleParallelizerMultiPlans;
+import org.apache.drill.exec.planner.sql.DrillSqlWorker;
+import org.apache.drill.exec.proto.BitControl.PlanFragment;
+import org.apache.drill.exec.proto.UserBitShared.DrillPBError;
+import org.apache.drill.exec.proto.UserBitShared.QueryId;
+import org.apache.drill.exec.proto.UserBitShared.QueryResult.QueryState;
+import org.apache.drill.exec.proto.UserProtos.GetQueryPlanFragments;
+import org.apache.drill.exec.proto.UserProtos.QueryPlanFragments;
+import org.apache.drill.exec.rpc.user.UserServer.UserClientConnection;
+import org.apache.drill.exec.server.DrillbitContext;
+import org.apache.drill.exec.util.Pointer;
+import org.apache.drill.exec.util.Utilities;
+import org.apache.drill.exec.work.QueryWorkUnit;
+
+import com.google.common.collect.Lists;
+
+/**
+ * Helper class to return PlanFragments based on the query plan
+ * or based on split query plan
+ *
+ */
+public class QueryPlanFragmentsSplitHelper {
--- End diff --
or even PlanSplitter.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4132 Ability to submit simple type of ph...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/368#discussion_r61490732
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java ---
@@ -321,6 +329,38 @@ public void close() {
return listener.getResults();
}
+ public DrillRpcFuture planQuery(QueryType type,
String query, boolean isSplitPlan) {
+GetQueryPlanFragments runQuery =
GetQueryPlanFragments.newBuilder().setQuery(query).setType(type).setSplitPlan(isSplitPlan).build();
+return client.submitPlanQuery(runQuery);
+ }
+
+ public void runQuery(QueryType type, List planFragments,
UserResultsListener resultsListener)
+ throws RpcException {
+// QueryType can be only executional
+checkArgument((QueryType.EXECUTION == type), "Only EXECUTIONAL type
query is supported with PlanFragments");
+// setting Plan on RunQuery will be used for logging purposes and
therefore can not be null
+// since there is no Plan string provided we will create a JsonArray
out of individual fragment Plans
+ArrayNode jsonArray = objectMapper.createArrayNode();
+for (PlanFragment fragment : planFragments) {
+ try {
+jsonArray.add(objectMapper.readTree(fragment.getFragmentJson()));
+ } catch (IOException e) {
+logger.error("Exception while trying to read PlanFragment JSON for
%s", fragment.getHandle().getQueryId(), e);
+throw new RpcException(e);
+ }
+}
+final String fragmentsToJsonString;
+try {
+ fragmentsToJsonString = objectMapper.writeValueAsString(jsonArray);
--- End diff --
That's because we are not telling Jackson how to handle cyclicity. Can you
try using DrillbitContext#getPlanReader instead?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4132 Ability to submit simple type of ph...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/368#discussion_r61491072
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/contrib/SimpleParallelizerMultiPlans.java
---
@@ -0,0 +1,228 @@
+/**
+ * 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.drill.exec.planner.fragment.contrib;
+
+import java.util.Collection;
+import java.util.Iterator;
+import java.util.List;
+
+import org.apache.drill.common.exceptions.ExecutionSetupException;
+import org.apache.drill.common.util.DrillStringUtils;
+import org.apache.drill.exec.ops.QueryContext;
+import org.apache.drill.exec.physical.base.Exchange;
+import org.apache.drill.exec.physical.base.FragmentRoot;
+import org.apache.drill.exec.physical.base.PhysicalOperator;
+import org.apache.drill.exec.planner.PhysicalPlanReader;
+import org.apache.drill.exec.planner.fragment.Fragment;
+import org.apache.drill.exec.planner.fragment.PlanningSet;
+import org.apache.drill.exec.planner.fragment.SimpleParallelizer;
+import org.apache.drill.exec.planner.fragment.Wrapper;
+import
org.apache.drill.exec.planner.fragment.Materializer.IndexedFragmentNode;
+import org.apache.drill.exec.proto.BitControl.PlanFragment;
+import org.apache.drill.exec.proto.BitControl.QueryContextInformation;
+import org.apache.drill.exec.proto.CoordinationProtos.DrillbitEndpoint;
+import org.apache.drill.exec.proto.ExecProtos.FragmentHandle;
+import org.apache.drill.exec.proto.UserBitShared.QueryId;
+import org.apache.drill.exec.rpc.user.UserSession;
+import org.apache.drill.exec.server.options.OptionList;
+import org.apache.drill.exec.work.QueryWorkUnit;
+import org.apache.drill.exec.work.foreman.ForemanSetupException;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.google.common.base.Preconditions;
+import com.google.common.collect.Lists;
+
+/**
+ * SimpleParallelizerMultiPlans class is an extension to SimpleParallelizer
+ * to help with getting PlanFragments for split plan.
+ * Split plan is essentially ability to create multiple Physical Operator
plans from original Physical Operator plan
+ * to be able to run plans separately.
+ * Moving functionality specific to splitting the plan to this class
+ * allows not to pollute parent class with non-authentic functionality
+ *
+ */
+public class SimpleParallelizerMultiPlans extends SimpleParallelizer {
--- End diff --
Name advise: SplittingParallelizer + documentation seems pretty good here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4132 Ability to submit simple type of ph...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/368#discussion_r61491214
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/contrib/SimpleParallelizerMultiPlans.java
---
@@ -0,0 +1,228 @@
+/**
+ * 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.drill.exec.planner.fragment.contrib;
+
+import java.util.Collection;
+import java.util.Iterator;
+import java.util.List;
+
+import org.apache.drill.common.exceptions.ExecutionSetupException;
+import org.apache.drill.common.util.DrillStringUtils;
+import org.apache.drill.exec.ops.QueryContext;
+import org.apache.drill.exec.physical.base.Exchange;
+import org.apache.drill.exec.physical.base.FragmentRoot;
+import org.apache.drill.exec.physical.base.PhysicalOperator;
+import org.apache.drill.exec.planner.PhysicalPlanReader;
+import org.apache.drill.exec.planner.fragment.Fragment;
+import org.apache.drill.exec.planner.fragment.PlanningSet;
+import org.apache.drill.exec.planner.fragment.SimpleParallelizer;
+import org.apache.drill.exec.planner.fragment.Wrapper;
+import
org.apache.drill.exec.planner.fragment.Materializer.IndexedFragmentNode;
+import org.apache.drill.exec.proto.BitControl.PlanFragment;
+import org.apache.drill.exec.proto.BitControl.QueryContextInformation;
+import org.apache.drill.exec.proto.CoordinationProtos.DrillbitEndpoint;
+import org.apache.drill.exec.proto.ExecProtos.FragmentHandle;
+import org.apache.drill.exec.proto.UserBitShared.QueryId;
+import org.apache.drill.exec.rpc.user.UserSession;
+import org.apache.drill.exec.server.options.OptionList;
+import org.apache.drill.exec.work.QueryWorkUnit;
+import org.apache.drill.exec.work.foreman.ForemanSetupException;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.google.common.base.Preconditions;
+import com.google.common.collect.Lists;
+
+/**
+ * SimpleParallelizerMultiPlans class is an extension to SimpleParallelizer
+ * to help with getting PlanFragments for split plan.
+ * Split plan is essentially ability to create multiple Physical Operator
plans from original Physical Operator plan
+ * to be able to run plans separately.
+ * Moving functionality specific to splitting the plan to this class
+ * allows not to pollute parent class with non-authentic functionality
+ *
+ */
+public class SimpleParallelizerMultiPlans extends SimpleParallelizer {
+
+ static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(SimpleParallelizerMultiPlans.class);
+
+ public SimpleParallelizerMultiPlans(QueryContext context) {
+super(context);
+ }
+
+ /**
+ * Create multiple physical plans from original query planning, it will
allow execute them eventually independently
+ * @param options
+ * @param foremanNode
+ * @param queryId
+ * @param activeEndpoints
+ * @param reader
+ * @param rootFragment
+ * @param session
+ * @param queryContextInfo
+ * @return
+ * @throws ExecutionSetupException
+ */
+ public List getSplitFragments(OptionList options,
DrillbitEndpoint foremanNode, QueryId queryId,
--- End diff --
Name advice: splitFragments/split.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4132 Ability to submit simple type of ph...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/368#discussion_r61491435
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/contrib/SimpleParallelizerMultiPlans.java
---
@@ -0,0 +1,228 @@
+/**
+ * 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.drill.exec.planner.fragment.contrib;
+
+import java.util.Collection;
+import java.util.Iterator;
+import java.util.List;
+
+import org.apache.drill.common.exceptions.ExecutionSetupException;
+import org.apache.drill.common.util.DrillStringUtils;
+import org.apache.drill.exec.ops.QueryContext;
+import org.apache.drill.exec.physical.base.Exchange;
+import org.apache.drill.exec.physical.base.FragmentRoot;
+import org.apache.drill.exec.physical.base.PhysicalOperator;
+import org.apache.drill.exec.planner.PhysicalPlanReader;
+import org.apache.drill.exec.planner.fragment.Fragment;
+import org.apache.drill.exec.planner.fragment.PlanningSet;
+import org.apache.drill.exec.planner.fragment.SimpleParallelizer;
+import org.apache.drill.exec.planner.fragment.Wrapper;
+import
org.apache.drill.exec.planner.fragment.Materializer.IndexedFragmentNode;
+import org.apache.drill.exec.proto.BitControl.PlanFragment;
+import org.apache.drill.exec.proto.BitControl.QueryContextInformation;
+import org.apache.drill.exec.proto.CoordinationProtos.DrillbitEndpoint;
+import org.apache.drill.exec.proto.ExecProtos.FragmentHandle;
+import org.apache.drill.exec.proto.UserBitShared.QueryId;
+import org.apache.drill.exec.rpc.user.UserSession;
+import org.apache.drill.exec.server.options.OptionList;
+import org.apache.drill.exec.work.QueryWorkUnit;
+import org.apache.drill.exec.work.foreman.ForemanSetupException;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.google.common.base.Preconditions;
+import com.google.common.collect.Lists;
+
+/**
+ * SimpleParallelizerMultiPlans class is an extension to SimpleParallelizer
+ * to help with getting PlanFragments for split plan.
+ * Split plan is essentially ability to create multiple Physical Operator
plans from original Physical Operator plan
+ * to be able to run plans separately.
+ * Moving functionality specific to splitting the plan to this class
+ * allows not to pollute parent class with non-authentic functionality
+ *
+ */
+public class SimpleParallelizerMultiPlans extends SimpleParallelizer {
+
+ static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(SimpleParallelizerMultiPlans.class);
+
+ public SimpleParallelizerMultiPlans(QueryContext context) {
+super(context);
+ }
+
+ /**
+ * Create multiple physical plans from original query planning, it will
allow execute them eventually independently
+ * @param options
+ * @param foremanNode
+ * @param queryId
+ * @param activeEndpoints
+ * @param reader
+ * @param rootFragment
+ * @param session
+ * @param queryContextInfo
+ * @return
+ * @throws ExecutionSetupException
+ */
+ public List getSplitFragments(OptionList options,
DrillbitEndpoint foremanNode, QueryId queryId,
+ Collection activeEndpoints, PhysicalPlanReader
reader, Fragment rootFragment,
+ UserSession session, QueryContextInformation queryContextInfo)
throws ExecutionSetupException {
+
+final PlanningSet planningSet = getFragmentsHelper(activeEndpoints,
rootFragment);
+
+return generateWorkUnits(
+options, foremanNode, queryId, reader, rootFragment, planningSet,
session, queryContextInfo);
+ }
+
+ /**
+ * Split plan into multiple plans based on parallelization
+ * Ideally it is applicable only to plans with two major fragments:
Screen and UnionExchange
+ * But there could be cases where we can remove even multiple exchanges
like in case of "order by"
--- End diff --
[GitHub] drill pull request: DRILL-4132 Ability to submit simple type of ph...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/368#discussion_r61491885
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/util/Utilities.java ---
@@ -17,13 +17,27 @@
*/
package org.apache.drill.exec.util;
+import java.util.LinkedList;
+import java.util.List;
+
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.exec.ExecConstants;
import org.apache.drill.exec.expr.fn.impl.DateUtility;
+import org.apache.drill.exec.memory.RootAllocatorFactory;
import org.apache.drill.exec.ops.FragmentContext;
+import org.apache.drill.exec.ops.QueryContext;
+import org.apache.drill.exec.physical.PhysicalPlan;
+import org.apache.drill.exec.physical.base.PhysicalOperator;
+import org.apache.drill.exec.physical.config.ExternalSort;
import org.apache.drill.exec.proto.BitControl.QueryContextInformation;
import org.apache.drill.exec.proto.ExecProtos;
import org.apache.drill.exec.proto.helper.QueryIdHelper;
+import org.apache.drill.exec.server.options.OptionManager;
public class Utilities {
+
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(Utilities.class);
--- End diff --
Please revert the changes in this class after migration of the real logic.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4132 Ability to submit simple type of ph...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/368#discussion_r61492206
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/work/user/PlanSplitter.java
---
@@ -0,0 +1,142 @@
+/**
+ * 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.drill.exec.work.user;
+
+import java.util.List;
+
+import org.apache.drill.exec.ops.QueryContext;
+import org.apache.drill.exec.physical.PhysicalPlan;
+import org.apache.drill.exec.physical.base.PhysicalOperator;
+import org.apache.drill.exec.planner.fragment.Fragment;
+import org.apache.drill.exec.planner.fragment.MakeFragmentsVisitor;
+import org.apache.drill.exec.planner.fragment.SimpleParallelizer;
+import
org.apache.drill.exec.planner.fragment.contrib.SimpleParallelizerMultiPlans;
+import org.apache.drill.exec.planner.sql.DrillSqlWorker;
+import org.apache.drill.exec.proto.BitControl.PlanFragment;
+import org.apache.drill.exec.proto.UserBitShared.DrillPBError;
+import org.apache.drill.exec.proto.UserBitShared.QueryId;
+import org.apache.drill.exec.proto.UserBitShared.QueryResult.QueryState;
+import org.apache.drill.exec.proto.UserProtos.GetQueryPlanFragments;
+import org.apache.drill.exec.proto.UserProtos.QueryPlanFragments;
+import org.apache.drill.exec.rpc.user.UserServer.UserClientConnection;
+import org.apache.drill.exec.server.DrillbitContext;
+import org.apache.drill.exec.util.MemoryAllocationUtilities;
+import org.apache.drill.exec.util.Pointer;
+import org.apache.drill.exec.util.Utilities;
+import org.apache.drill.exec.work.QueryWorkUnit;
+
+import com.google.common.collect.Lists;
+
+/**
+ * Helper class to return PlanFragments based on the query plan
+ * or based on split query plan
+ *
+ */
+public class PlanSplitter {
+ static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(PlanSplitter.class);
+
+ private static PlanSplitter s_instance = new PlanSplitter();
+
+ private PlanSplitter() {
+
+ }
+
+ public static PlanSplitter getInstance() {
+return s_instance;
--- End diff --
This seems a very cheap object used once per query. Why to make it
singleton? We should create it in the stack/heap as we need.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4132 Ability to submit simple type of ph...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/368#discussion_r61494244
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/work/user/PlanSplitter.java
---
@@ -0,0 +1,142 @@
+/**
+ * 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.drill.exec.work.user;
+
+import java.util.List;
+
+import org.apache.drill.exec.ops.QueryContext;
+import org.apache.drill.exec.physical.PhysicalPlan;
+import org.apache.drill.exec.physical.base.PhysicalOperator;
+import org.apache.drill.exec.planner.fragment.Fragment;
+import org.apache.drill.exec.planner.fragment.MakeFragmentsVisitor;
+import org.apache.drill.exec.planner.fragment.SimpleParallelizer;
+import
org.apache.drill.exec.planner.fragment.contrib.SimpleParallelizerMultiPlans;
+import org.apache.drill.exec.planner.sql.DrillSqlWorker;
+import org.apache.drill.exec.proto.BitControl.PlanFragment;
+import org.apache.drill.exec.proto.UserBitShared.DrillPBError;
+import org.apache.drill.exec.proto.UserBitShared.QueryId;
+import org.apache.drill.exec.proto.UserBitShared.QueryResult.QueryState;
+import org.apache.drill.exec.proto.UserProtos.GetQueryPlanFragments;
+import org.apache.drill.exec.proto.UserProtos.QueryPlanFragments;
+import org.apache.drill.exec.rpc.user.UserServer.UserClientConnection;
+import org.apache.drill.exec.server.DrillbitContext;
+import org.apache.drill.exec.util.MemoryAllocationUtilities;
+import org.apache.drill.exec.util.Pointer;
+import org.apache.drill.exec.util.Utilities;
+import org.apache.drill.exec.work.QueryWorkUnit;
+
+import com.google.common.collect.Lists;
+
+/**
+ * Helper class to return PlanFragments based on the query plan
+ * or based on split query plan
+ *
+ */
+public class PlanSplitter {
+ static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(PlanSplitter.class);
+
+ private static PlanSplitter s_instance = new PlanSplitter();
+
+ private PlanSplitter() {
+
+ }
+
+ public static PlanSplitter getInstance() {
+return s_instance;
+ }
+
+ /**
+ * Method to plan the query and return list of fragments
+ * it will return query plan "as is" or split plans based on the req
setting: split_plan
+ * @param dContext
+ * @param queryId
+ * @param req
+ * @param connection
+ * @return
+ */
+ public QueryPlanFragments planFragments(DrillbitContext dContext,
QueryId queryId,
+ GetQueryPlanFragments req, UserClientConnection connection) {
+QueryPlanFragments.Builder responseBuilder =
QueryPlanFragments.newBuilder();
+QueryContext queryContext = new QueryContext(connection.getSession(),
dContext, queryId);
+
+responseBuilder.setQueryId(queryId);
+
+try {
+ responseBuilder.addAllFragments(getFragments(dContext, req,
queryContext, queryId));
+ responseBuilder.setStatus(QueryState.COMPLETED);
+} catch (Exception e) {
+ final String errorMessage = String.format("Failed to produce
PlanFragments for query id \"%s\" with "
+ + "request to %s plan", queryId, (req.getSplitPlan() ? "split" :
"no split"));
+ DrillPBError error =
DrillPBError.newBuilder().setMessage(errorMessage).setErrorType(DrillPBError.ErrorType.PLAN).build();
+
+ responseBuilder.setStatus(QueryState.FAILED);
+ responseBuilder.setError(error);
+}
+return responseBuilder.build();
+ }
+
+ private List getFragments(final DrillbitContext dContext,
final GetQueryPlanFragments req,
+ final QueryContext queryContext, final QueryId queryId) throws
Exception {
+final PhysicalPlan plan;
+final String query = req.getQuery();
+switch(req.getType()) {
+case SQL:
+ final Pointer textPlan = new Pointer<>();
+ plan = DrillSqlW
[GitHub] drill pull request: DRILL-4132 Ability to submit simple type of ph...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/368#discussion_r61494824
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/DrillSeparatePlanningTest.java
---
@@ -0,0 +1,350 @@
+/**
+ * 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.drill.exec;
+
+import static org.junit.Assert.*;
+import io.netty.buffer.DrillBuf;
+
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Properties;
+import java.util.concurrent.ExecutionException;
+
+import org.apache.drill.BaseTestQuery;
+import org.apache.drill.common.DrillAutoCloseables;
+import org.apache.drill.common.exceptions.UserException;
+import org.apache.drill.common.util.TestTools;
+import org.apache.drill.exec.client.DrillClient;
+import org.apache.drill.exec.client.PrintingResultsListener;
+import org.apache.drill.exec.client.QuerySubmitter.Format;
+import org.apache.drill.exec.exception.SchemaChangeException;
+import org.apache.drill.exec.memory.BufferAllocator;
+import org.apache.drill.exec.proto.BitControl.PlanFragment;
+import org.apache.drill.exec.proto.CoordinationProtos.DrillbitEndpoint;
+import org.apache.drill.exec.proto.UserBitShared.QueryData;
+import org.apache.drill.exec.proto.UserBitShared.QueryId;
+import org.apache.drill.exec.proto.UserBitShared.QueryResult.QueryState;
+import org.apache.drill.exec.proto.UserBitShared.QueryType;
+import org.apache.drill.exec.proto.UserProtos.QueryPlanFragments;
+import org.apache.drill.exec.record.RecordBatchLoader;
+import org.apache.drill.exec.record.VectorWrapper;
+import org.apache.drill.exec.rpc.ConnectionThrottle;
+import org.apache.drill.exec.rpc.DrillRpcFuture;
+import org.apache.drill.exec.rpc.RpcException;
+import org.apache.drill.exec.rpc.user.AwaitableUserResultsListener;
+import org.apache.drill.exec.rpc.user.QueryDataBatch;
+import org.apache.drill.exec.rpc.user.UserResultsListener;
+import org.apache.drill.exec.util.VectorUtil;
+import org.apache.drill.exec.vector.ValueVector;
+import org.junit.Test;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
+
+/**
+ * Class to test different planning use cases (separate form query
execution)
+ *
+ */
+public class DrillSeparatePlanningTest extends BaseTestQuery {
+
+ static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(DrillSeparatePlanningTest.class);
+
+ static final String WORKING_PATH = TestTools.getWorkingPath();
+ static final String TEST_RES_PATH = WORKING_PATH + "/src/test/resources";
+
+ //final String query = "SELECT sales_city, COUNT(*) cnt FROM
cp.`region.json` GROUP BY sales_city";
+ //final String query = "SELECT * FROM cp.`employee.json` where
employee_id > 1 and employee_id < 1000";
+ //final String query = "SELECT o_orderkey, o_custkey FROM
dfs.tmp.`multilevel` where dir0 = 1995 and o_orderkey > 100 and o_orderkey <
1000 limit 5";
+ //final String query = "SELECT sum(o_totalprice) FROM
dfs.tmp.`multilevel` where dir0 = 1995 and o_orderkey > 100 and o_orderkey <
1000";
+ //final String query = "SELECT o_orderkey FROM dfs.tmp.`multilevel`
order by o_orderkey";
+ //final String query = "SELECT dir1, sum(o_totalprice) FROM
dfs.tmp.`multilevel` where dir0 = 1995 group by dir1 order by dir1";
+ //final String query = String.format("SELECT dir0, sum(o_totalprice)
FROM dfs_test.`%s/multilevel/json` group by dir0 order by dir0", TEST_RES_PATH);
+
+
+ @Test(timeout=3)
+ public void testSingleFragmentQuery() throws Exception {
+final String query = "SELECT * FROM cp.`employee.json` where
employee_id > 1 and employee_id < 1000";
+
+QueryPlanFragments planFragments =
[GitHub] drill pull request: DRILL-4132 Ability to submit simple type of ph...
Github user hnfgns commented on the pull request: https://github.com/apache/drill/pull/368#issuecomment-215548496 Overall looks good ð Thanks. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4132 Ability to submit simple type of ph...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/368#discussion_r61521260
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/DrillSeparatePlanningTest.java
---
@@ -0,0 +1,350 @@
+/**
+ * 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.drill.exec;
+
+import static org.junit.Assert.*;
+import io.netty.buffer.DrillBuf;
+
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Properties;
+import java.util.concurrent.ExecutionException;
+
+import org.apache.drill.BaseTestQuery;
+import org.apache.drill.common.DrillAutoCloseables;
+import org.apache.drill.common.exceptions.UserException;
+import org.apache.drill.common.util.TestTools;
+import org.apache.drill.exec.client.DrillClient;
+import org.apache.drill.exec.client.PrintingResultsListener;
+import org.apache.drill.exec.client.QuerySubmitter.Format;
+import org.apache.drill.exec.exception.SchemaChangeException;
+import org.apache.drill.exec.memory.BufferAllocator;
+import org.apache.drill.exec.proto.BitControl.PlanFragment;
+import org.apache.drill.exec.proto.CoordinationProtos.DrillbitEndpoint;
+import org.apache.drill.exec.proto.UserBitShared.QueryData;
+import org.apache.drill.exec.proto.UserBitShared.QueryId;
+import org.apache.drill.exec.proto.UserBitShared.QueryResult.QueryState;
+import org.apache.drill.exec.proto.UserBitShared.QueryType;
+import org.apache.drill.exec.proto.UserProtos.QueryPlanFragments;
+import org.apache.drill.exec.record.RecordBatchLoader;
+import org.apache.drill.exec.record.VectorWrapper;
+import org.apache.drill.exec.rpc.ConnectionThrottle;
+import org.apache.drill.exec.rpc.DrillRpcFuture;
+import org.apache.drill.exec.rpc.RpcException;
+import org.apache.drill.exec.rpc.user.AwaitableUserResultsListener;
+import org.apache.drill.exec.rpc.user.QueryDataBatch;
+import org.apache.drill.exec.rpc.user.UserResultsListener;
+import org.apache.drill.exec.util.VectorUtil;
+import org.apache.drill.exec.vector.ValueVector;
+import org.junit.Test;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
+
+/**
+ * Class to test different planning use cases (separate form query
execution)
+ *
+ */
+public class DrillSeparatePlanningTest extends BaseTestQuery {
+
+ static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(DrillSeparatePlanningTest.class);
+
+ static final String WORKING_PATH = TestTools.getWorkingPath();
+ static final String TEST_RES_PATH = WORKING_PATH + "/src/test/resources";
+
+ //final String query = "SELECT sales_city, COUNT(*) cnt FROM
cp.`region.json` GROUP BY sales_city";
+ //final String query = "SELECT * FROM cp.`employee.json` where
employee_id > 1 and employee_id < 1000";
+ //final String query = "SELECT o_orderkey, o_custkey FROM
dfs.tmp.`multilevel` where dir0 = 1995 and o_orderkey > 100 and o_orderkey <
1000 limit 5";
+ //final String query = "SELECT sum(o_totalprice) FROM
dfs.tmp.`multilevel` where dir0 = 1995 and o_orderkey > 100 and o_orderkey <
1000";
+ //final String query = "SELECT o_orderkey FROM dfs.tmp.`multilevel`
order by o_orderkey";
+ //final String query = "SELECT dir1, sum(o_totalprice) FROM
dfs.tmp.`multilevel` where dir0 = 1995 group by dir1 order by dir1";
+ //final String query = String.format("SELECT dir0, sum(o_totalprice)
FROM dfs_test.`%s/multilevel/json` group by dir0 order by dir0", TEST_RES_PATH);
+
+
+ @Test(timeout=3)
+ public void testSingleFragmentQuery() throws Exception {
+final String query = "SELECT * FROM cp.`employee.json` where
employee_id > 1 and employee_id < 1000";
+
+QueryPlanFragments planFragments =
[GitHub] drill pull request: DRILL-4132 Ability to submit simple type of ph...
Github user hnfgns commented on the pull request: https://github.com/apache/drill/pull/368#issuecomment-216724026 +1 if made sure all tests are green. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4676: Foreman.moveToState can block fore...
Github user hnfgns commented on a diff in the pull request: https://github.com/apache/drill/pull/503#discussion_r63305469 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java --- @@ -115,6 +115,8 @@ private static final ObjectMapper MAPPER = new ObjectMapper(); private static final long RPC_WAIT_IN_MSECS_PER_FRAGMENT = 5000; + private Thread myThreadRef; // Thread that is currently executing the Foreman. --- End diff -- I believe volatile is unneeded here. In my opinion keeping myThreadRef in heap is also unneeded or seems wrong even. I will detail down. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4676: Foreman.moveToState can block fore...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/503#discussion_r63305602
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java ---
@@ -1221,6 +1223,12 @@ public void interrupted(final InterruptedException
e) {
* to the user
*/
public void moveToState(final QueryState newState, final Exception ex)
{
+ // if the current thread is the foreman thread, throw an exception
+ // otherwise the foreman will be blocked forever on
acceptExternalEvents
+ if (myThreadRef == Thread.currentThread()) {
--- End diff --
So now that we are throwing an exception on foreman thread how will we move
to failed state in case foreman fails?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4676: Foreman.moveToState can block fore...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/503#discussion_r63428428
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java ---
@@ -1221,6 +1223,12 @@ public void interrupted(final InterruptedException
e) {
* to the user
*/
public void moveToState(final QueryState newState, final Exception ex)
{
+ // if the current thread is the foreman thread, throw an exception
+ // otherwise the foreman will be blocked forever on
acceptExternalEvents
+ if (myThreadRef == Thread.currentThread()) {
--- End diff --
Well, Foreman does not call moveToState in common path but when it fails
then it could. My point is instead of hacking the method to throw an exception
if the thread is foreman we should release the latch and handle the exception
in foreman#run. What I mean is
Foreman#run() {
```
try {
do sth
} catch(ex) {
release latch
handleException(ex)
} finally {
...
}
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4676: Foreman.moveToState can block fore...
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/503#discussion_r63430594
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java ---
@@ -1221,6 +1223,12 @@ public void interrupted(final InterruptedException
e) {
* to the user
*/
public void moveToState(final QueryState newState, final Exception ex)
{
+ // if the current thread is the foreman thread, throw an exception
+ // otherwise the foreman will be blocked forever on
acceptExternalEvents
+ if (myThreadRef == Thread.currentThread()) {
--- End diff --
Let me ask you this: what was the primary cause of deadlock came out of
your investigation?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4676: Foreman.moveToState can block fore...
Github user hnfgns commented on the pull request: https://github.com/apache/drill/pull/503#issuecomment-219573757 I see that you are enabling task hand-off for the first task in the serializer as well. I am not convinced that this approach will for sure fix the deadlock case for RPCs and foreman failure case. I do not have time now but I will think about this issue. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4690: CORS in REST API
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/507#discussion_r64627240
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java
---
@@ -159,6 +163,14 @@ public void start() throws Exception {
servletContextHandler.setSessionHandler(createSessionHandler(servletContextHandler.getSecurityHandler()));
}
+if (config.getBoolean(ExecConstants.HTTP_ENABLE_CORS)) {
+ FilterHolder cors =
servletContextHandler.addFilter(CrossOriginFilter.class, "/*",
EnumSet.of(DispatcherType.REQUEST));
--- End diff --
Please apply cors on API endpoints only. The static path should be excluded.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4690: CORS in REST API
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/507#discussion_r64627653
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java
---
@@ -159,6 +163,14 @@ public void start() throws Exception {
servletContextHandler.setSessionHandler(createSessionHandler(servletContextHandler.getSecurityHandler()));
}
+if (config.getBoolean(ExecConstants.HTTP_ENABLE_CORS)) {
+ FilterHolder cors =
servletContextHandler.addFilter(CrossOriginFilter.class, "/*",
EnumSet.of(DispatcherType.REQUEST));
+ cors.setInitParameter(CrossOriginFilter.ALLOWED_ORIGINS_PARAM, "*");
--- End diff --
+1 that may pose a security risk. We should extend the configuration with
allowed-origins. Use of * is too relaxed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4690: CORS in REST API
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/507#discussion_r64629211
--- Diff: exec/java-exec/src/main/resources/drill-module.conf ---
@@ -110,6 +110,7 @@ drill.exec: {
http: {
enabled: true,
ssl_enabled: false,
+cors_enabled: true,
--- End diff --
perhaps we should consider extending cors configuration with
```
http: {
cors: {
enabled: true,
allowedOrigins: ['*.mydomain.com', '*.someother.net'], --> configures
Access-Control-Allow-Origin
allowedMethods: ['option', 'get', 'post', 'some-other'], --> configures
Access-Control-Allow-Methods
allowedHeaders: ['some-allowed-header'], --> configures
Access-Control-Expose-Headers
credentials: true | false -- > configures
Access-Control-Allow-Credentials
}
}
```
I am probably missing few in the list but I find these options essential.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4690: CORS in REST API
Github user hnfgns commented on a diff in the pull request: https://github.com/apache/drill/pull/507#discussion_r64629460 --- Diff: exec/jdbc-all/pom.xml --- @@ -441,7 +441,7 @@ This is likely due to you adding new dependencies to a java-exec and not updating the excludes in this module. This is important as it minimizes the size of the dependency of Drill application users. - 2000 + 2100 --- End diff -- Is this really needed? Looks as though you tried to add a new dependency but failed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4690: CORS in REST API
Github user hnfgns commented on the pull request: https://github.com/apache/drill/pull/507#issuecomment-221669367 Looks pretty good. I will make another pass once reviews are addressed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-4690: CORS in REST API
Github user hnfgns commented on a diff in the pull request: https://github.com/apache/drill/pull/507#discussion_r64632355 --- Diff: exec/jdbc-all/pom.xml --- @@ -441,7 +441,7 @@ This is likely due to you adding new dependencies to a java-exec and not updating the excludes in this module. This is important as it minimizes the size of the dependency of Drill application users. - 2000 + 2100 --- End diff -- I would not worry about this too much at this time. Laurent made a good point above. We should match the jetty-servlets version with our jetty-server version. So you might want to use 9.1.5.v20140505 instead. [9.1.5.v20140505](http://mvnrepository.com/artifact/org.eclipse.jetty/jetty-servlets/9.1.5.v20140505) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill issue #507: DRILL-4690: CORS in REST API
Github user hnfgns commented on the issue: https://github.com/apache/drill/pull/507 +1 this looks good to me but can one of you guys take a look, test and commit? @adeneche @sudheeshkatkam --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill issue #516: DRILL-4664: ScanBatch.isNewSchema() returns wrong result f...
Github user hnfgns commented on the issue: https://github.com/apache/drill/pull/516 looks good to me +1 @parthchandra can we run regression and merge? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3313: Eliminate redundant #load methods ...
GitHub user hnfgns opened a pull request: https://github.com/apache/drill/pull/81 DRILL-3313: Eliminate redundant #load methods and unit-test loading & exporting of vectors You can merge this pull request into a Git repository by running: $ git pull https://github.com/hnfgns/incubator-drill DRILL-3313 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/81.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 #81 commit ef79c18d408582dc07b83c8f62075d2329216bd1 Author: Hanifi Gunes Date: 2015-06-23T19:05:45Z DRILL-3313: Eliminate redundant #load methods and unit-test loading & exporting of vectors --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3441: fix indefinite loop problem in Com...
GitHub user hnfgns opened a pull request: https://github.com/apache/drill/pull/82 DRILL-3441: fix indefinite loop problem in CompliantTextRecordReader#isStarQuery You can merge this pull request into a Git repository by running: $ git pull https://github.com/hnfgns/incubator-drill DRILL-3441 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/82.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 #82 commit 51655617e048b0156058793d57505ac7ffb0267a Author: Hanifi Gunes Date: 2015-07-08T20:18:43Z DRILL-3441: fix indefinite loop problem in CompliantTextRecordReader#isStarQuery --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3476: Merge paths in FieldSelection
Github user hnfgns commented on the pull request: https://github.com/apache/drill/pull/83#issuecomment-124561596 +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3353: Fix dropping nested fields
Github user hnfgns commented on the pull request: https://github.com/apache/drill/pull/86#issuecomment-124561688 +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3153: Fix JDBC's getIdentifierQuoteStrin...
Github user hnfgns commented on the pull request: https://github.com/apache/drill/pull/99#issuecomment-125719344 +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
