[GitHub] drill pull request #860: DRILL-5601: Rollup of external sort fixes and impro...

[Ben-Zvi]

Github user Ben-Zvi commented on a diff in the pull request:

https://github.com/apache/drill/pull/860#discussion_r127537900
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java
 ---
@@ -265,7 +277,7 @@ private StreamingAggregator createAggregatorInternal() 
throws SchemaChangeExcept
 context.getFunctionRegistry(), context.getOptions());
 cg.getCodeGenerator().plainJavaCapable(true);
 // Uncomment out this line to debug the generated code.
-//cg.getCodeGenerator().saveCodeForDebugging(true);
+cg.getCodeGenerator().saveCodeForDebugging(true);
--- End diff --

Comment out  ...


---
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 #860: DRILL-5601: Rollup of external sort fixes and impro...

[Ben-Zvi]

Github user Ben-Zvi commented on a diff in the pull request:

https://github.com/apache/drill/pull/860#discussion_r127560655
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggTemplate.java
 ---
@@ -36,6 +36,9 @@
   private int previousIndex = -1;
   private int underlyingIndex = 0;
   private int currentIndex;
+  /**
+   * Number of records added to the current aggregation group. (?)
--- End diff --

Yes, this is correct. No need for the "(?)"


---
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 #860: DRILL-5601: Rollup of external sort fixes and impro...

[Ben-Zvi]

Github user Ben-Zvi commented on a diff in the pull request:

https://github.com/apache/drill/pull/860#discussion_r127536602
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java
 ---
@@ -111,6 +120,8 @@ public void buildSchema() throws SchemaChangeException {
   case STOP:
 state = BatchState.STOP;
 return;
+  default:
+break;
--- End diff --

Not getting into a "religious" debate, but "default+break" is just a 
useless no-op


---
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 #876: DRILL-5659: Fix error code checking in reading from socket

[parthchandra]

Github user parthchandra commented on the issue:

https://github.com/apache/drill/pull/876
  
Still haven't been able to confirm the SASL encryption case, but there is 
no reason why this fix would affect that. Merging it in.


---
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 #875: DRILL-5671 Set secure ACLs (Access Control List) fo...

[bitblender]

Github user bitblender commented on a diff in the pull request:

https://github.com/apache/drill/pull/875#discussion_r127540793
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKACLProviderFactory.java
 ---
@@ -0,0 +1,44 @@
+/**
+ * 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 org.apache.curator.framework.api.ACLProvider;
+import org.apache.curator.framework.imps.DefaultACLProvider;
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.exec.ExecConstants;
+
+
+public class ZKACLProviderFactory {
+
+static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(ZKACLProviderFactory.class);
+
+public static ACLProvider getACLProvider(DrillConfig config, String 
clusterId, String zkRoot) {
--- End diff --

Will make 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 #875: DRILL-5671 Set secure ACLs (Access Control List) fo...

[bitblender]

Github user bitblender commented on a diff in the pull request:

https://github.com/apache/drill/pull/875#discussion_r127540711
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java ---
@@ -39,6 +39,7 @@
   String ZK_TIMEOUT = "drill.exec.zk.timeout";
   String ZK_ROOT = "drill.exec.zk.root";
   String ZK_REFRESH = "drill.exec.zk.refresh";
+  String ZK_SECURE_ACL = "drill.exec.zk.use.secure_acl";
--- End diff --

I was going to make this "zk.use_secure_acl" but when I was looking for an 
example I found "bit.use.ip", so I modeled it on that. I will change this to 
"zk.apply_secure_acl"


---
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 #874: DRILL-5663: Drillbit fails to start when only keyst...

[paul-rogers]

Github user paul-rogers commented on a diff in the pull request:

https://github.com/apache/drill/pull/874#discussion_r127529165
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java 
---
@@ -264,19 +265,26 @@ private ServerConnector createHttpsConnector() throws 
Exception {
 
 final SslContextFactory sslContextFactory = new SslContextFactory();
 
-if (config.hasPath(ExecConstants.HTTP_KEYSTORE_PATH) &&
-
!Strings.isNullOrEmpty(config.getString(ExecConstants.HTTP_KEYSTORE_PATH))) {
-  logger.info("Using configured SSL settings for web server");
-  
sslContextFactory.setKeyStorePath(config.getString(ExecConstants.HTTP_KEYSTORE_PATH));
-  
sslContextFactory.setKeyStorePassword(config.getString(ExecConstants.HTTP_KEYSTORE_PASSWORD));
-
-  // TrustStore and TrustStore password are optional
-  if (config.hasPath(ExecConstants.HTTP_TRUSTSTORE_PATH)) {
-
sslContextFactory.setTrustStorePath(config.getString(ExecConstants.HTTP_TRUSTSTORE_PATH));
-if (config.hasPath(ExecConstants.HTTP_TRUSTSTORE_PASSWORD)) {
-  
sslContextFactory.setTrustStorePassword(config.getString(ExecConstants.HTTP_TRUSTSTORE_PASSWORD));
-}
+final boolean hasPath = 
config.hasPath(ExecConstants.HTTP_KEYSTORE_PATH);
--- End diff --

To avoid this double-checking, standard practice is:

* Use a blank value to indicate the value is unset.
* Provide a default (blank) value in `drill-module.conf`

Then, the code can just be:

```
String path = config.getString(ExecConstants.HTTP_KEYSTORE_PATH).trim();
if (! path.isEmpty()) {
  // do stuff
```

Also, if the user is expected to set this key, please provide an example in 
`drill-override-example.conf`.


---
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 #874: DRILL-5663: Drillbit fails to start when only keyst...

[paul-rogers]

Github user paul-rogers commented on a diff in the pull request:

https://github.com/apache/drill/pull/874#discussion_r127530311
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java 
---
@@ -289,30 +297,19 @@ private ServerConnector createHttpsConnector() throws 
Exception {
   final DateTime now = DateTime.now();
 
   // Create builder for certificate attributes
-  final X500NameBuilder nameBuilder =
-  new X500NameBuilder(BCStyle.INSTANCE)
-  .addRDN(BCStyle.OU, "Apache Drill (auth-generated)")
-  .addRDN(BCStyle.O, "Apache Software Foundation 
(auto-generated)")
-  .addRDN(BCStyle.CN, 
workManager.getContext().getEndpoint().getAddress());
+  final X500NameBuilder nameBuilder = new 
X500NameBuilder(BCStyle.INSTANCE).addRDN(BCStyle.OU, "Apache Drill 
(auth-generated)").addRDN(BCStyle.O, "Apache Software Foundation 
(auto-generated)").addRDN(BCStyle.CN, 
workManager.getContext().getEndpoint().getAddress());
--- End diff --

Can we keep the previous one-argument-per-line, indented style? Much easier 
to read.


---
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 #874: DRILL-5663: Drillbit fails to start when only keyst...

[paul-rogers]

Github user paul-rogers commented on a diff in the pull request:

https://github.com/apache/drill/pull/874#discussion_r127530151
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java 
---
@@ -264,19 +265,26 @@ private ServerConnector createHttpsConnector() throws 
Exception {
 
 final SslContextFactory sslContextFactory = new SslContextFactory();
 
-if (config.hasPath(ExecConstants.HTTP_KEYSTORE_PATH) &&
-
!Strings.isNullOrEmpty(config.getString(ExecConstants.HTTP_KEYSTORE_PATH))) {
-  logger.info("Using configured SSL settings for web server");
-  
sslContextFactory.setKeyStorePath(config.getString(ExecConstants.HTTP_KEYSTORE_PATH));
-  
sslContextFactory.setKeyStorePassword(config.getString(ExecConstants.HTTP_KEYSTORE_PASSWORD));
-
-  // TrustStore and TrustStore password are optional
-  if (config.hasPath(ExecConstants.HTTP_TRUSTSTORE_PATH)) {
-
sslContextFactory.setTrustStorePath(config.getString(ExecConstants.HTTP_TRUSTSTORE_PATH));
-if (config.hasPath(ExecConstants.HTTP_TRUSTSTORE_PASSWORD)) {
-  
sslContextFactory.setTrustStorePassword(config.getString(ExecConstants.HTTP_TRUSTSTORE_PASSWORD));
-}
+final boolean hasPath = 
config.hasPath(ExecConstants.HTTP_KEYSTORE_PATH);
+final boolean hasPassword = 
config.hasPath(ExecConstants.HTTP_KEYSTORE_PASSWORD);
+
+// Check if both keypath and password are present or not
+if (hasPath && hasPassword) {
+  final String pathValue = 
config.getString(ExecConstants.HTTP_KEYSTORE_PATH);
+  final String passwordValue = 
config.getString(ExecConstants.HTTP_KEYSTORE_PASSWORD);
+
+  // checking if any one of them is null or empty
+  if (!Strings.isNullOrEmpty(pathValue) && 
!Strings.isNullOrEmpty(passwordValue)) {
+sslContextFactory.setKeyStorePath(pathValue);
+sslContextFactory.setKeyStorePassword(passwordValue);
+  }
+
+  // Throwing an exception if anyone of them is null or empty
+  else {
+throw new DrillbitStartupException("keystorepath and/or 
keystorepassword can't be empty.");
--- End diff --

Please provide a bit of a clearer message. Also, provide the actual path 
name. This will be the critical message for folks that made mistake, the 
Drillbit won't start, and they have to figure out what's what.

Maybe,

```
"To enable web UI security, both " + ExecConstants.HTTP_KEYSTORE_PATH + " 
and " + ... + " are required.";
```

Also, this is just for the Web UI? In that case, maybe just disable the web 
UI, but don't fail the Drillbit.

```
"Web UI misconfiguration: web UI is disabled."
```


---
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 #874: DRILL-5663: Drillbit fails to start when only keyst...

[paul-rogers]

Github user paul-rogers commented on a diff in the pull request:

https://github.com/apache/drill/pull/874#discussion_r127529372
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java 
---
@@ -264,19 +265,26 @@ private ServerConnector createHttpsConnector() throws 
Exception {
 
 final SslContextFactory sslContextFactory = new SslContextFactory();
 
-if (config.hasPath(ExecConstants.HTTP_KEYSTORE_PATH) &&
-
!Strings.isNullOrEmpty(config.getString(ExecConstants.HTTP_KEYSTORE_PATH))) {
-  logger.info("Using configured SSL settings for web server");
-  
sslContextFactory.setKeyStorePath(config.getString(ExecConstants.HTTP_KEYSTORE_PATH));
-  
sslContextFactory.setKeyStorePassword(config.getString(ExecConstants.HTTP_KEYSTORE_PASSWORD));
-
-  // TrustStore and TrustStore password are optional
-  if (config.hasPath(ExecConstants.HTTP_TRUSTSTORE_PATH)) {
-
sslContextFactory.setTrustStorePath(config.getString(ExecConstants.HTTP_TRUSTSTORE_PATH));
-if (config.hasPath(ExecConstants.HTTP_TRUSTSTORE_PASSWORD)) {
-  
sslContextFactory.setTrustStorePassword(config.getString(ExecConstants.HTTP_TRUSTSTORE_PASSWORD));
-}
+final boolean hasPath = 
config.hasPath(ExecConstants.HTTP_KEYSTORE_PATH);
+final boolean hasPassword = 
config.hasPath(ExecConstants.HTTP_KEYSTORE_PASSWORD);
+
+// Check if both keypath and password are present or not
+if (hasPath && hasPassword) {
+  final String pathValue = 
config.getString(ExecConstants.HTTP_KEYSTORE_PATH);
+  final String passwordValue = 
config.getString(ExecConstants.HTTP_KEYSTORE_PASSWORD);
+
+  // checking if any one of them is null or empty
+  if (!Strings.isNullOrEmpty(pathValue) && 
!Strings.isNullOrEmpty(passwordValue)) {
--- End diff --

Path can't be null here, so `! pathValue.isEmpty()` works also.


---
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 #875: DRILL-5671 Set secure ACLs (Access Control List) fo...

[paul-rogers]

Github user paul-rogers commented on a diff in the pull request:

https://github.com/apache/drill/pull/875#discussion_r127525673
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKACLProviderFactory.java
 ---
@@ -0,0 +1,44 @@
+/**
+ * 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 org.apache.curator.framework.api.ACLProvider;
+import org.apache.curator.framework.imps.DefaultACLProvider;
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.exec.ExecConstants;
+
+
+public class ZKACLProviderFactory {
+
+static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(ZKACLProviderFactory.class);
+
+public static ACLProvider getACLProvider(DrillConfig config, String 
clusterId, String zkRoot) {
--- End diff --

See comment below: probably want to pass a root path here rather than the 
components.


---
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 #875: DRILL-5671 Set secure ACLs (Access Control List) fo...

[paul-rogers]

Github user paul-rogers commented on a diff in the pull request:

https://github.com/apache/drill/pull/875#discussion_r127528429
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKSecureACLProvider.java
 ---
@@ -0,0 +1,80 @@
+/**
+ * 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.collect.ImmutableList;
+import org.apache.curator.framework.api.ACLProvider;
+import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.data.ACL;
+
+import java.util.List;
+
+/**
+ * ZKSecureACLProvider restricts access to znodes created by Drill in a 
secure installation.
+ *
+ * The cluster discovery znode i.e. the znode containing the list of 
Drillbits is
+ * readable by anyone.
+ *
+ * For all other znodes, only the creator of the znode, i.e the Drillbit 
user, has full access.
+ *
+ */
+
+public class ZKSecureACLProvider implements ACLProvider {
+
+static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(ZKSecureACLProvider.class);
+
+/**
+ * DEFAULT_ACL gives the creator of a znode full-access to it
+ */
+static ImmutableList DEFAULT_ACL = new 
ImmutableList.Builder()
+  
.addAll(Ids.CREATOR_ALL_ACL.iterator())
+  .build();
+/**
+ * DRILL_CLUSTER_ACL gives the creator full access and everyone else 
only read access.
+ * Used on the Drillbit discovery znode (znode path 
//)
+ * i.e. the node that contains the list of Drillbits in the cluster.
+ */
+ static ImmutableList DRILL_CLUSTER_ACL = new 
ImmutableList.Builder()
+
.addAll(Ids.READ_ACL_UNSAFE.iterator())
+
.addAll(Ids.CREATOR_ALL_ACL.iterator())
+.build();
+final String clusterName;
+final String drillZkRoot;
+final String drillClusterPath;
+
+public ZKSecureACLProvider(String clusterName, String drillZKRoot) {
+this.clusterName = clusterName;
+this.drillZkRoot = drillZKRoot;
+this.drillClusterPath = "/" + this.drillZkRoot + "/" +  
this.clusterName ;
+}
+
+public List getDefaultAcl() {
+return DEFAULT_ACL;
+}
+
+public List getAclForPath(String path) {
--- End diff --

This approach encodes the meaning of paths in just the path name. Seems 
fragile.

Alternatives:

* Register the secure (or insecure) paths so that the ZK cluster 
coordinator (not this class) decides on security.
* Get ACL base on type: `getSecureACL()` or `getPublicACL()`, and let the 
cluster coordinator define which is which.
* As a refinement of the first idea, provide some kind of config option to 
externalize the set of paths and their security.

For example, in Drill-on-YARN, the app master creates ZK entries to ensure 
that only one app master starts per cluster. We would not want to encode that 
path information 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 #875: DRILL-5671 Set secure ACLs (Access Control List) fo...

[paul-rogers]

Github user paul-rogers commented on a diff in the pull request:

https://github.com/apache/drill/pull/875#discussion_r127525830
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKACLProviderFactory.java
 ---
@@ -0,0 +1,44 @@
+/**
+ * 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 org.apache.curator.framework.api.ACLProvider;
+import org.apache.curator.framework.imps.DefaultACLProvider;
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.exec.ExecConstants;
+
+
+public class ZKACLProviderFactory {
+
+static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(ZKACLProviderFactory.class);
+
+public static ACLProvider getACLProvider(DrillConfig config, String 
clusterId, String zkRoot) {
+if (config.getBoolean(ExecConstants.ZK_SECURE_ACL)) {
+if 
(config.getBoolean(ExecConstants.USER_AUTHENTICATION_ENABLED)){
+logger.trace("ZKACLProviderFactory: Using secure ZK ACL");
--- End diff --

The logger inserts the class name for you; no need to repeat it 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 #875: DRILL-5671 Set secure ACLs (Access Control List) fo...

[paul-rogers]

Github user paul-rogers commented on a diff in the pull request:

https://github.com/apache/drill/pull/875#discussion_r127526692
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKSecureACLProvider.java
 ---
@@ -0,0 +1,80 @@
+/**
+ * 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.collect.ImmutableList;
+import org.apache.curator.framework.api.ACLProvider;
+import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.data.ACL;
+
+import java.util.List;
+
+/**
+ * ZKSecureACLProvider restricts access to znodes created by Drill in a 
secure installation.
+ *
+ * The cluster discovery znode i.e. the znode containing the list of 
Drillbits is
+ * readable by anyone.
+ *
+ * For all other znodes, only the creator of the znode, i.e the Drillbit 
user, has full access.
+ *
+ */
+
+public class ZKSecureACLProvider implements ACLProvider {
+
+static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(ZKSecureACLProvider.class);
+
+/**
+ * DEFAULT_ACL gives the creator of a znode full-access to it
+ */
+static ImmutableList DEFAULT_ACL = new 
ImmutableList.Builder()
+  
.addAll(Ids.CREATOR_ALL_ACL.iterator())
+  .build();
+/**
+ * DRILL_CLUSTER_ACL gives the creator full access and everyone else 
only read access.
+ * Used on the Drillbit discovery znode (znode path 
//)
+ * i.e. the node that contains the list of Drillbits in the cluster.
+ */
+ static ImmutableList DRILL_CLUSTER_ACL = new 
ImmutableList.Builder()
+
.addAll(Ids.READ_ACL_UNSAFE.iterator())
+
.addAll(Ids.CREATOR_ALL_ACL.iterator())
+.build();
+final String clusterName;
+final String drillZkRoot;
+final String drillClusterPath;
+
+public ZKSecureACLProvider(String clusterName, String drillZKRoot) {
+this.clusterName = clusterName;
+this.drillZkRoot = drillZKRoot;
+this.drillClusterPath = "/" + this.drillZkRoot + "/" +  
this.clusterName ;
--- End diff --

Encoding path building here seems fragile. The ZK cluster coordinator 
should build the path, then pass that along to the factory, which passes it 
here. Simplifies things if we ever have to change how we construct the path if 
we create it in one place rather than two (or more).


---
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 #875: DRILL-5671 Set secure ACLs (Access Control List) fo...

[paul-rogers]

Github user paul-rogers commented on a diff in the pull request:

https://github.com/apache/drill/pull/875#discussion_r127524590
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java ---
@@ -39,6 +39,7 @@
   String ZK_TIMEOUT = "drill.exec.zk.timeout";
   String ZK_ROOT = "drill.exec.zk.root";
   String ZK_REFRESH = "drill.exec.zk.refresh";
+  String ZK_SECURE_ACL = "drill.exec.zk.use.secure_acl";
--- End diff --

`use.secure_acl` --> `secure_acl` as `use` is not a group. This provides:

```
  zk: {
connect: "localhost:2181",
root: "drill",
secure_acl: true,
```

Are we using ACL's for security? Then, we are really "applying" ACLs, so 
maybe `apply_acl` (apply an access control list...)


---
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 #860: DRILL-5601: Rollup of external sort fixes and impro...

[Ben-Zvi]

Github user Ben-Zvi commented on a diff in the pull request:

https://github.com/apache/drill/pull/860#discussion_r127363488
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/spill/RecordBatchSizer.java
 ---
@@ -70,52 +72,90 @@
  */
 
 public final int estSize;
+
+/**
+ * Number of times the value here (possibly repeated) appears in
+ * the record batch.
+ */
+
 public final int valueCount;
-public final int entryCount;
-public final int dataSize;
-public final int estElementCount;
+
+/**
+ * Number of times the entries of this column appears. If this is a
+ * scalar, the entry count is the same as the value count. If this
--- End diff --

If a scalar, entryCount would be 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.
---

Re: Debugging Drill in Eclipse

[Paul Rogers]

Thanks for the hint about configuring Eclipse to launch Drill. That would be 
handy for when we need an actual Drillbit process. The technique I mentioned 
involves running Drill in embedded mode, often as part of test case. The 
“cluster test” framework makes that very simple and fast. Combined, we have 
tools to be able to run in whatever mode is easiest for a given test.

I gathered some existing notes into a more complete description of the test 
framework: [1]

This is a starter set. Please let me know if there are particular areas that 
need more detail. I’m hoping that once the Wiki page gets you started, that the 
detailed Javadoc in the various test classes will help you tackle the more 
complex cases. But, do let me know if more description is helpful.

Thanks,

- Paul

[1] https://github.com/paul-rogers/drill/wiki/Cluster-Fixture-Framework

> On Jul 14, 2017, at 8:23 AM, Muhammad Gelbana  wrote:
> 
> When you said I'm rebuilding Drill, do you mean I'm running "*mvn clean
> install*" after every change ? Actually I'm configuring eclipse to run the
> same java command that "*drillbit.sh start*" command would run.
> 
> But I have to say that your method is much more efficient and practical. I
> should've used test cases for development from the start.
> 
> I recently started exploring the test cases classes but I found it very
> hard to comprehend so If there is some sort of a guide to the testing
> classes architecture, packages structure\categorization..etc, it would be
> very helpful if you share it. I looked into your github wiki
>  but I didn't find
> what I need.
> 
> 
> 
> - Gelbana
> 
> On Thu, Jul 13, 2017 at 6:53 PM, Paul Rogers  wrote:
> 
>> Hi Muhammad,
>> 
>> There are several issues here.
>> 
>> First, the problem you describe is not related to Eclipse. Second, several
>> of us do use Eclipse and it works fine. Third, there are far easier ways to
>> debug Drill in Eclipse then building Drill and doing remote debugging.
>> 
>> First the error. The problem is that some bit of code is referring to a
>> config parameter that does not appear in the config files. This kind of key
>> would normally appear in some drill-module.conf file. (The file
>> drill-override.conf is only for the very few config setting that you must
>> modify for your site.)
>> 
>> My source is a bit different than your, but the line in question seems to
>> be this one:
>> 
>>this.loop = TransportCheck.createEventLoopGroup(config.
>> getInt(ExecConstants.BIT_SERVER_RPC_THREADS), "BitServer-“);
>> 
>> The config property here is defined in java-exec/src/main/resources
>> drill-module.conf:
>> 
>> drill.exec: {
>>  ...
>>  rpc: {
>>user: {
>>  ...
>>  server: {
>>   ...
>>threads: 1,
>> 
>> The only way you will get the error in your stack is if, somehow, your
>> build omits the drill-module.conf file. You can inspect your jar files to
>> see if this file is present.
>> 
>> Second, Eclipse works fine. The only trick is if you try to run certain
>> unit tests that use Mockito. “Modern” Eclipse Neon is based on JDK 8. But
>> the Mockito tests only run on JDK 7. There is no way to get the test runner
>> in Eclipse to use JDK 7. So, I end up building Drill for JDK 8 when using
>> Eclipse (just change the Java version in the root pom.xml file from 1.7 to
>> 1.8 — in two places.) Then run the Mockito-based tests outside of Eclipse,
>> after rebuilding back on JDK 7. Yes, a hassle, but this is just the way
>> that Drill works today.
>> 
>> Further, what works for me is:
>> 
>> 1. Check out Drill
>> 2. Change the pom.xml Java version number as noted above
>> 3. Build all of Drill without tests: “mvn clean install -DskipTests”
>> 4. Open Drill or refresh Drill in Eclipse. Eclipse does its own build.
>> 5. Run Drill directly from Eclipse.
>> 
>> Item 5 above is the third item. For many purposes, it is far more
>> convenient to run Drill directly from Eclipse. I use unit tests based on a
>> newer test framework. Find ExampleTest.java for example. For example, if a
>> particular query fails, and I can copy the data locally, then I just use
>> something like “fourthTest” to set up a storage plugin, set required
>> session, system and config options, run the query, and either display the
>> results or as summary. You can set breakpoints in the lines in question and
>> debug. Entire edit/compile/debug cycle is maybe 30 seconds vs. the five
>> minutes if you do a full external build.
>> 
>> I hope some of this helps you resolve your issue.
>> 
>> The most practical solution:
>> 
>> 1. Rebuild Drill
>> 2. Retry the run without the debugger.
>> 
>> If that works, review your Eclipse settings that might affect class path.
>> 
>> Thanks,
>> 
>> - Paul
>> 
>> 
>>> On Jul 13, 2017, at 8:20 AM, Muhammad Gelbana 
>> wrote:
>>> 
>>> To debug Drill in Eclipse, I ran *./drillbit.sh debug* and copied the 

[jira] [Created] (DRILL-5672) Move session option setting from cluster fixture to client fixture

[Paul Rogers (JIRA)]

Paul Rogers created DRILL-5672:
--

 Summary: Move session option setting from cluster fixture to 
client fixture
 Key: DRILL-5672
 URL: https://issues.apache.org/jira/browse/DRILL-5672
 Project: Apache Drill
  Issue Type: Improvement
Affects Versions: 1.10.0
Reporter: Paul Rogers
Assignee: Paul Rogers
Priority: Minor
 Fix For: 1.12.0


The recently-added "cluster fixture" allows setting session and system options. 
At present, both are set on the cluster fixture. However, it makes more sense 
for session options to be set on the client. This way, we can create two 
clients, each with different session options, for tests that need that 
flexibility.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Re: Debugging Drill in Eclipse

[Muhammad Gelbana]

When you said I'm rebuilding Drill, do you mean I'm running "*mvn clean
install*" after every change ? Actually I'm configuring eclipse to run the
same java command that "*drillbit.sh start*" command would run.

But I have to say that your method is much more efficient and practical. I
should've used test cases for development from the start.

I recently started exploring the test cases classes but I found it very
hard to comprehend so If there is some sort of a guide to the testing
classes architecture, packages structure\categorization..etc, it would be
very helpful if you share it. I looked into your github wiki
 but I didn't find
what I need.



- Gelbana

On Thu, Jul 13, 2017 at 6:53 PM, Paul Rogers  wrote:

> Hi Muhammad,
>
> There are several issues here.
>
> First, the problem you describe is not related to Eclipse. Second, several
> of us do use Eclipse and it works fine. Third, there are far easier ways to
> debug Drill in Eclipse then building Drill and doing remote debugging.
>
> First the error. The problem is that some bit of code is referring to a
> config parameter that does not appear in the config files. This kind of key
> would normally appear in some drill-module.conf file. (The file
> drill-override.conf is only for the very few config setting that you must
> modify for your site.)
>
> My source is a bit different than your, but the line in question seems to
> be this one:
>
> this.loop = TransportCheck.createEventLoopGroup(config.
> getInt(ExecConstants.BIT_SERVER_RPC_THREADS), "BitServer-“);
>
> The config property here is defined in java-exec/src/main/resources
> drill-module.conf:
>
> drill.exec: {
>   ...
>   rpc: {
> user: {
>   ...
>   server: {
>...
> threads: 1,
>
> The only way you will get the error in your stack is if, somehow, your
> build omits the drill-module.conf file. You can inspect your jar files to
> see if this file is present.
>
> Second, Eclipse works fine. The only trick is if you try to run certain
> unit tests that use Mockito. “Modern” Eclipse Neon is based on JDK 8. But
> the Mockito tests only run on JDK 7. There is no way to get the test runner
> in Eclipse to use JDK 7. So, I end up building Drill for JDK 8 when using
> Eclipse (just change the Java version in the root pom.xml file from 1.7 to
> 1.8 — in two places.) Then run the Mockito-based tests outside of Eclipse,
> after rebuilding back on JDK 7. Yes, a hassle, but this is just the way
> that Drill works today.
>
> Further, what works for me is:
>
> 1. Check out Drill
> 2. Change the pom.xml Java version number as noted above
> 3. Build all of Drill without tests: “mvn clean install -DskipTests”
> 4. Open Drill or refresh Drill in Eclipse. Eclipse does its own build.
> 5. Run Drill directly from Eclipse.
>
> Item 5 above is the third item. For many purposes, it is far more
> convenient to run Drill directly from Eclipse. I use unit tests based on a
> newer test framework. Find ExampleTest.java for example. For example, if a
> particular query fails, and I can copy the data locally, then I just use
> something like “fourthTest” to set up a storage plugin, set required
> session, system and config options, run the query, and either display the
> results or as summary. You can set breakpoints in the lines in question and
> debug. Entire edit/compile/debug cycle is maybe 30 seconds vs. the five
> minutes if you do a full external build.
>
> I hope some of this helps you resolve your issue.
>
> The most practical solution:
>
> 1. Rebuild Drill
> 2. Retry the run without the debugger.
>
> If that works, review your Eclipse settings that might affect class path.
>
> Thanks,
>
> - Paul
>
>
> > On Jul 13, 2017, at 8:20 AM, Muhammad Gelbana 
> wrote:
> >
> > To debug Drill in Eclipse, I ran *./drillbit.sh debug* and copied the VM
> > args and environment variables into a launcher. This worked fine for
> months.
> >
> > Obviously now I messed things up in a way and I can't debug Drill in
> > Eclipse anymore. I'm facing the following error:
> >
> > Exception in thread "main"
> > org.apache.drill.exec.exception.DrillbitStartupException: Failure while
> > initializing values in Drillbit.
> > at org.apache.drill.exec.server.Drillbit.start(Drillbit.java:288)
> > at org.apache.drill.exec.server.Drillbit.start(Drillbit.java:272)
> > at org.apache.drill.exec.server.Drillbit.main(Drillbit.java:268)
> > Caused by: com.typesafe.config.ConfigException$Missing: *No
> configuration
> > setting found for key 'drill.exec.rpc'*
> > at com.typesafe.config.impl.SimpleConfig.findKey(SimpleConfig.java:115)
> > at com.typesafe.config.impl.SimpleConfig.find(SimpleConfig.java:138)
> > at com.typesafe.config.impl.SimpleConfig.find(SimpleConfig.java:142)
> > at com.typesafe.config.impl.SimpleConfig.find(SimpleConfig.java:142)
> > at com.typesafe.config.impl.SimpleConfig.find(SimpleConfig.java:150)
> > at 

Re: [DISCUSS] Drill 1.11.0 release

[Paul Rogers]

I looked into one NPE on CSV. Turns out it is already fixed by previous 1.11 
PRs, so we’re good.

There is another reported by a user on the user list, but I’ve not heard back 
about getting sample data. Let’s not hold the release for that.

- Paul

> On Jul 14, 2017, at 2:38 AM, Arina Yelchiyeva  
> wrote:
> 
> *Blockers*
> DRILL-5659 - Parth (done)
> DRILL-5669 - Boaz Ben-Zvi (fix will be ready in a couple of days)
> 
> *Possible blockers*
> NPE on CSV - Paul can you please provide an update?
> 
> Kind regards
> Arina
> 
> On Thu, Jul 13, 2017 at 2:46 PM, Arina Yelchiyeva <
> arina.yelchiy...@gmail.com> wrote:
> 
>> *Blockers*
>> DRILL-5659 - problem is identified, Parth is working on the fix.
>> 
>> *Not blockers*
>> DRILL-5660 - Vitalii
>> DRILL-5468 - Jinfeng
>> 
>> *Possible blockers*
>> DRILL-5669 - Boaz Ben-Zvi / Paul
>> NPE on CSV - Paul
>> 
>> Kind regards
>> Arina
>> 
>> 
>> On Thu, Jul 13, 2017 at 1:28 AM, Paul Rogers  wrote:
>> 
>>> I withdraw my suggestion to fix DRILL-5660. Was a big hassle for us to
>>> track down the incompatibility the issue internally, but production users
>>> probably don’t attempt to use one build of Drill with metadata files
>>> created from another. We can worry about the issue later only if a user
>>> actually runs into it.
>>> 
>>> As suggested earlier, let’s just document this obscure case in a release
>>> note.
>>> 
>>> - Paul
>>> 
 On Jul 12, 2017, at 3:04 PM, Jinfeng Ni  wrote:
 
 Upgrade from 1.10 to 1.11 is irreversible only when parquet metadata
>>> cache
 file is involved.
 
 I might be wrong, but I thought that if someone is running on 1.10.0,
>>> and
 he wants to go back to 1.0.0, where the first version of parquet
>>> meatadata
 cache was introduced, can we guarantee that Drillbit would work
>>> correctly?
 I'm trying to understand what DRILL-5660 asks for is a new requirement,
>>> or
 an requirement being there starting from Drill 1.0.0?
 
 Regression failures are normally regarded as release blocker.
 
 
 
 On Wed, Jul 12, 2017 at 2:54 PM, Paul Rogers  wrote:
 
> If we don’t do DRILL-5660 for 1.11 then we don’t need to do it at all,
>>> so
> that is a time savings. We could handle this via a release note item
>>> that
> simply states that an upgrade from 1.10 to 1.11 is irreversible: we do
>>> not
> support the ability to fall back to the old version. Everyone OK with
>>> that?
> 
> We are seeing some stress test regression failures that, I’m told, will
> show up as JIRA entries shortly.
> 
> - Paul
> 
>> On Jul 12, 2017, at 11:05 AM, Jinfeng Ni  wrote:
>> 
>> I put some analysis in DRILL-5468.
>> 
>> Also, IMHO, DRILL-5660 is not a release blocker.
>> 
>> 
>> 
>> On Tue, Jul 11, 2017 at 11:27 AM, Parth Chandra 
> wrote:
>> 
>>> Re DRILL-5634:
>>> From the Apache commons web page [1] -
>>> 
>>> Please note that Apache Commons Crypto doesn't implement the
> cryptographic
>>> algorithm such as AES directly.* It wraps to Openssl or JCE which
> implement
>>> the algorithms.*
>>> 
>>> 
>>> Since we are cleared for using Openssl and JCE, we are OK to include
>>> the
>>> AES code in the UDFs.
>>> 
>>> 
>>> Parth
>>> 
>>> [1] https://commons.apache.org/proper/commons-crypto/
>>> 
>>> On Tue, Jul 11, 2017 at 10:56 AM, Arina Yelchiyeva <
>>> arina.yelchiy...@gmail.com> wrote:
>>> 
 During today hangouts we agreed on approximate cut-off date - 14
>>> July,
>>> 2017
 (date may be shifted due to the blockers).
 I'll start preparing test RC on Thursday to solve any release
> preparation
 issues beforehand.
 
 *Blockers*
 DRILL-5660 - Vitalii
 
 *Possible blockers*
 DRILL-5468 - Jinfeng
 NPE on CSV source - Paul
 DRILL-5659 - Parth
 
 *Will be included if done before the cut-off date*
 DRILL-5616 / DRILL-5665 - Boaz
 DRILL-5634 - Parth will take a look at Apache commons implementation
> and
 update.
 
 Kind regards
 Arina
 
 On Tue, Jul 11, 2017 at 3:14 PM, Charles Givre 
> wrote:
 
> Hi Arina,
> I can’t make the hangout, but we have a few options.  Basically,
>>> since
 all
> the hashing algorithms have no export restrictions, we could:
> 1.  Remove the AES encrypt/decrypt until I can figure out what we
>>> have
>>> to
> do (and do it)
> 2.  I can try to research it today or tomorrow to figure out
>>> exactly
>>> what
> we have to do to include AES.
> 3.  Remove the entire package
> 
> I’d like to see the 

[GitHub] drill issue #877: DRILL-5660: Drill 1.10 queries fail due to Parquet Metadat...

[paul-rogers]

Github user paul-rogers commented on the issue:

https://github.com/apache/drill/pull/877
  
I wonder if you are very close to solving another long-running problem in 
Drill: Parquet metadata file corruption (or, at least, the corruption causing 
queries to fail.)

We have an issue that two Drillbits can write to metadata file concurrently 
and corrupt it. We also have a case where Drillbit A writes the file while 
Drillbit B reads it. In both cases, the reading Drillbit can't deserialize the 
file and "bad things happen."

We need code that can try to deserialize a file, notice that the JSON in 
the file does not fit our schema, and handle the deserialization exception. If 
we do that, it is a small step to ignore all forms of read errors: 
file-not-found, file truncation, etc.

In this case, we give an explicit version error if we can read the file and 
it has the wrong version. If we can't deserialize the file, we log an error 
about "file is of a newer version or corrupt."

After this, the corruption can still occur, but the user view is that 
queries run slow instead of just failing.


---
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 #865: DRILL-5634 - Add Crypto and Hash Functions

[arina-ielchiieva]

Github user arina-ielchiieva commented on the issue:

https://github.com/apache/drill/pull/865
  
+1, checked the latest changes and ran unit tests.


---
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.
---

Re: [DISCUSS] Drill 1.11.0 release

[Arina Yelchiyeva]

*Blockers*
DRILL-5659 - Parth (done)
DRILL-5669 - Boaz Ben-Zvi (fix will be ready in a couple of days)

*Possible blockers*
NPE on CSV - Paul can you please provide an update?

Kind regards
Arina

On Thu, Jul 13, 2017 at 2:46 PM, Arina Yelchiyeva <
arina.yelchiy...@gmail.com> wrote:

> *Blockers*
> DRILL-5659 - problem is identified, Parth is working on the fix.
>
> *Not blockers*
> DRILL-5660 - Vitalii
> DRILL-5468 - Jinfeng
>
> *Possible blockers*
> DRILL-5669 - Boaz Ben-Zvi / Paul
> NPE on CSV - Paul
>
> Kind regards
> Arina
>
>
> On Thu, Jul 13, 2017 at 1:28 AM, Paul Rogers  wrote:
>
>> I withdraw my suggestion to fix DRILL-5660. Was a big hassle for us to
>> track down the incompatibility the issue internally, but production users
>> probably don’t attempt to use one build of Drill with metadata files
>> created from another. We can worry about the issue later only if a user
>> actually runs into it.
>>
>> As suggested earlier, let’s just document this obscure case in a release
>> note.
>>
>> - Paul
>>
>> > On Jul 12, 2017, at 3:04 PM, Jinfeng Ni  wrote:
>> >
>> > Upgrade from 1.10 to 1.11 is irreversible only when parquet metadata
>> cache
>> > file is involved.
>> >
>> > I might be wrong, but I thought that if someone is running on 1.10.0,
>> and
>> > he wants to go back to 1.0.0, where the first version of parquet
>> meatadata
>> > cache was introduced, can we guarantee that Drillbit would work
>> correctly?
>> > I'm trying to understand what DRILL-5660 asks for is a new requirement,
>> or
>> > an requirement being there starting from Drill 1.0.0?
>> >
>> > Regression failures are normally regarded as release blocker.
>> >
>> >
>> >
>> > On Wed, Jul 12, 2017 at 2:54 PM, Paul Rogers  wrote:
>> >
>> >> If we don’t do DRILL-5660 for 1.11 then we don’t need to do it at all,
>> so
>> >> that is a time savings. We could handle this via a release note item
>> that
>> >> simply states that an upgrade from 1.10 to 1.11 is irreversible: we do
>> not
>> >> support the ability to fall back to the old version. Everyone OK with
>> that?
>> >>
>> >> We are seeing some stress test regression failures that, I’m told, will
>> >> show up as JIRA entries shortly.
>> >>
>> >> - Paul
>> >>
>> >>> On Jul 12, 2017, at 11:05 AM, Jinfeng Ni  wrote:
>> >>>
>> >>> I put some analysis in DRILL-5468.
>> >>>
>> >>> Also, IMHO, DRILL-5660 is not a release blocker.
>> >>>
>> >>>
>> >>>
>> >>> On Tue, Jul 11, 2017 at 11:27 AM, Parth Chandra 
>> >> wrote:
>> >>>
>>  Re DRILL-5634:
>>  From the Apache commons web page [1] -
>> 
>>  Please note that Apache Commons Crypto doesn't implement the
>> >> cryptographic
>>  algorithm such as AES directly.* It wraps to Openssl or JCE which
>> >> implement
>>  the algorithms.*
>> 
>> 
>>  Since we are cleared for using Openssl and JCE, we are OK to include
>> the
>>  AES code in the UDFs.
>> 
>> 
>>  Parth
>> 
>>  [1] https://commons.apache.org/proper/commons-crypto/
>> 
>>  On Tue, Jul 11, 2017 at 10:56 AM, Arina Yelchiyeva <
>>  arina.yelchiy...@gmail.com> wrote:
>> 
>> > During today hangouts we agreed on approximate cut-off date - 14
>> July,
>>  2017
>> > (date may be shifted due to the blockers).
>> > I'll start preparing test RC on Thursday to solve any release
>> >> preparation
>> > issues beforehand.
>> >
>> > *Blockers*
>> > DRILL-5660 - Vitalii
>> >
>> > *Possible blockers*
>> > DRILL-5468 - Jinfeng
>> > NPE on CSV source - Paul
>> > DRILL-5659 - Parth
>> >
>> > *Will be included if done before the cut-off date*
>> > DRILL-5616 / DRILL-5665 - Boaz
>> > DRILL-5634 - Parth will take a look at Apache commons implementation
>> >> and
>> > update.
>> >
>> > Kind regards
>> > Arina
>> >
>> > On Tue, Jul 11, 2017 at 3:14 PM, Charles Givre 
>> >> wrote:
>> >
>> >> Hi Arina,
>> >> I can’t make the hangout, but we have a few options.  Basically,
>> since
>> > all
>> >> the hashing algorithms have no export restrictions, we could:
>> >> 1.  Remove the AES encrypt/decrypt until I can figure out what we
>> have
>>  to
>> >> do (and do it)
>> >> 2.  I can try to research it today or tomorrow to figure out
>> exactly
>>  what
>> >> we have to do to include AES.
>> >> 3.  Remove the entire package
>> >>
>> >> I’d like to see the hashing functions at least included because
>> we’ve
>>  had
>> >> a lot of requests for that functionality.   I also don’t want this
>> to
>> > hold
>> >> things up, so please let me know your preference.  Regardless, I’m
>>  going
>> > to
>> >> start trying to figure out the restrictions so we’ll know for the
>>  future.
>> >>
>> >> Best,
>> >> — C
>> >>
>> >>
>> >>
>> >>> On Jul 11,