Re: [ANNOUNCE] Apache Drill 1.9.0 Released

2016-11-29 Thread Abhishek Girish
Congrats all!

I'd like to mention that the 1.9.0 release also includes the MapR format
plugin, to read MapR-DB Binary and JSON Tables.

On Tue, Nov 29, 2016 at 3:52 PM, Sudheesh Katkam 
wrote:

> On behalf of the Apache Drill community, I am happy to announce the release
> of Apache Drill 1.9.0.
>
> For information about Apache Drill, and to get involved, visit the project
> website:
>
> https://drill.apache.org/
>
> This release introduces new features and enhancements, including
> asynchronous Parquet reader, Parquet filter pushdown, dynamic UDF support
> and HTTPD format plugin. In all, 70 issues have been resolved.
>
> The binary and source artifacts are available here:
>
> https://drill.apache.org/download/
>
> Review the release notes for a complete list of fixes and enhancements:
>
> https://drill.apache.org/docs/apache-drill-1-9-0-release-notes/
>
> Thanks to everyone in the community who contributed to this release!
>
> - Sudheesh
>


[ANNOUNCE] Apache Drill 1.9.0 Released

2016-11-29 Thread Sudheesh Katkam
On behalf of the Apache Drill community, I am happy to announce the release
of Apache Drill 1.9.0.

For information about Apache Drill, and to get involved, visit the project
website:

https://drill.apache.org/

This release introduces new features and enhancements, including
asynchronous Parquet reader, Parquet filter pushdown, dynamic UDF support
and HTTPD format plugin. In all, 70 issues have been resolved.

The binary and source artifacts are available here:

https://drill.apache.org/download/

Review the release notes for a complete list of fixes and enhancements:

https://drill.apache.org/docs/apache-drill-1-9-0-release-notes/

Thanks to everyone in the community who contributed to this release!

- Sudheesh


[GitHub] drill issue #671: DRILL-4347: Propagate distinct row count for joins from lo...

2016-11-29 Thread amansinha100
Github user amansinha100 commented on the issue:

https://github.com/apache/drill/pull/671
  
Thanks @julianhyde... CALCITE-604 should potentially help with this.  
Drill's calcite version has not caught up to this yet.  Let me confer with 
@jinfengni  sometime next week (he is on vacation until then) and get back on 
what can be done to get this into Drill.  In the meantime, even though my patch 
addresses the hang issue, I will hold it for now. 


---
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 #648: DRILL-5015: Randomly select the drillbit from the l...

2016-11-29 Thread sohami
Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/648#discussion_r89928539
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java ---
@@ -245,14 +327,15 @@ public synchronized void connect(String connect, 
Properties props) throws RpcExc
   throw new RpcException("Failure setting up ZK for client.", e);
 }
   }
-
-  final ArrayList endpoints = new 
ArrayList<>(clusterCoordinator.getAvailableEndpoints());
-  checkState(!endpoints.isEmpty(), "No DrillbitEndpoint can be found");
-  // shuffle the collection then get the first endpoint
-  Collections.shuffle(endpoints);
-  endpoint = endpoints.iterator().next();
+  endpoints.addAll(clusterCoordinator.getAvailableEndpoints());
+  // Make sure we have at least one endpoint in the list
+  checkState(!endpoints.isEmpty(), "No active Drillbit endpoint found 
from zookeeper");
--- End diff --

"checkState" is only called when drillbit information is populated using 
ZooKeeper. For connection string case we are throwing 
InvalidConnectionInfoException. Changed the string to "ZooKeeper"


---
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 #648: DRILL-5015: Randomly select the drillbit from the l...

2016-11-29 Thread sohami
Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/648#discussion_r90090906
  
--- Diff: 
exec/java-exec/src/test/java/org/apache/drill/exec/client/DrillClientTest.java 
---
@@ -0,0 +1,258 @@
+/**
+ * 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.client;
+
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.exec.DrillSystemTestBase;
+import org.apache.drill.exec.ExecConstants;
+import org.apache.drill.exec.proto.CoordinationProtos;
+import org.apache.drill.exec.rpc.InvalidConnectionInfoException;
+import org.junit.Test;
+import java.util.List;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+
+/**
+ * The unit test case will read a physical plan in json format. The 
physical plan contains a "trace" operator,
+ * which will produce a dump file.  The dump file will be input into 
DumpCat to test query mode and batch mode.
+ */
+
+public class DrillClientTest extends DrillSystemTestBase {
+
+  private final DrillConfig config = DrillConfig.create();
+
+  @Test
+  public void testParseAndVerifyEndpointsSingleDrillbitIp() throws 
Exception {
+
+// Test with single drillbit ip
+final String drillBitConnection = "10.10.100.161";
+final List endpointsList = 
DrillClient.parseAndVerifyEndpoints
+  (drillBitConnection, 
config.getString(ExecConstants.INITIAL_USER_PORT));
+final CoordinationProtos.DrillbitEndpoint endpoint = 
endpointsList.get(0);
+assertEquals(endpointsList.size(), 1);
+assertEquals(endpoint.getAddress(), drillBitConnection);
+assertEquals(endpoint.getUserPort(), 
config.getInt(ExecConstants.INITIAL_USER_PORT));
+  }
+
+  @Test
+  public void testParseAndVerifyEndpointsSingleDrillbitIpPort() throws 
Exception {
+
+// Test with single drillbit ip:port
+final String drillBitConnection = "10.10.100.161:5000";
+final String[] ipAndPort = drillBitConnection.split(":");
+final List endpointsList = 
DrillClient.parseAndVerifyEndpoints
+  (drillBitConnection, 
config.getString(ExecConstants.INITIAL_USER_PORT));
+assertEquals(endpointsList.size(), 1);
+
+final CoordinationProtos.DrillbitEndpoint endpoint = 
endpointsList.get(0);
+assertEquals(endpoint.getAddress(), ipAndPort[0]);
+assertEquals(endpoint.getUserPort(), Integer.parseInt(ipAndPort[1]));
+  }
+
+  @Test
+  public void testParseAndVerifyEndpointsMultipleDrillbitIp() throws 
Exception {
+
+// Test with multiple drillbit ip
+final String drillBitConnection = "10.10.100.161,10.10.100.162";
+final List endpointsList = 
DrillClient.parseAndVerifyEndpoints
+  (drillBitConnection, 
config.getString(ExecConstants.INITIAL_USER_PORT));
+assertEquals(endpointsList.size(), 2);
+
+CoordinationProtos.DrillbitEndpoint endpoint = endpointsList.get(0);
+assertEquals(endpoint.getAddress(), "10.10.100.161");
+assertEquals(endpoint.getUserPort(), 
config.getInt(ExecConstants.INITIAL_USER_PORT));
+
+endpoint = endpointsList.get(1);
+assertEquals(endpoint.getAddress(), "10.10.100.162");
+assertEquals(endpoint.getUserPort(), 
config.getInt(ExecConstants.INITIAL_USER_PORT));
+  }
+
+  @Test
+  public void testParseAndVerifyEndpointsMultipleDrillbitIpPort() throws 
Exception {
+
+// Test with multiple drillbit ip:port
+final String drillBitConnection = 
"10.10.100.161:5000,10.10.100.162:5000";
+final List endpointsList = 
DrillClient.parseAndVerifyEndpoints
+  (drillBitConnection, 
config.getString(ExecConstants.INITIAL_USER_PORT));
+assertEquals(endpointsList.size(), 2);
+
+CoordinationProtos.DrillbitEndpoint endpoint = endpointsList.get(0);
+assertEquals(endpoint.getAddress(), "10.10.100.161");
+assertEquals(endpoint.getUserPort(), 5000);
+
 

[GitHub] drill pull request #648: DRILL-5015: Randomly select the drillbit from the l...

2016-11-29 Thread sohami
Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/648#discussion_r89928643
  
--- Diff: 
exec/java-exec/src/test/java/org/apache/drill/exec/client/DrillClientSystemTest.java
 ---
@@ -17,11 +17,15 @@
  */
 package org.apache.drill.exec.client;
 
+import java.util.ArrayList;
--- End diff --

I will remove this file from the list since have moved all the changes to 
different file "DrillClientTest.java"


---
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 #648: DRILL-5015: Randomly select the drillbit from the l...

2016-11-29 Thread sohami
Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/648#discussion_r90089781
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java ---
@@ -223,19 +224,100 @@ public void connect(Properties props) throws 
RpcException {
 connect(null, props);
   }
 
+  /**
+   * Populates the endpointlist with drillbits information provided in the 
connection string by client.
+   * For direct connection we can have connection string with drillbit 
property as below:
+   * 
+   *   drillbit=ip
+   *   use the ip specified as the Foreman ip with default port in 
config file
+   *   drillbit=ip:port
+   *   use the ip and port specified as the Foreman ip and port
+   *   drillbit=ip1:port1,ip2:port2,...
+   *   randomly select the ip and port pair from the specified list as 
the Foreman ip and port.
+   * 
+   *
+   * @param drillbits string with drillbit value provided in connection 
string
+   * @param defaultUserPort string with default userport of drillbit 
specified in config file
+   * @return list of drillbit endpoints parsed from connection string
+   * @throws InvalidConnectionInfoException if the connection string has 
invalid or no drillbit information
+   */
+  static List parseAndVerifyEndpoints(String drillbits, 
String defaultUserPort)
+throws InvalidConnectionInfoException {
+// If no drillbits is provided then throw exception
+drillbits = drillbits.trim();
+if (drillbits.isEmpty()) {
+  throw new InvalidConnectionInfoException("No drillbit information 
specified in the connection string");
+}
+
+ArrayList endpointList = new ArrayList<>();
+final String[] connectInfo = drillbits.split(",");
+
+// Fetch ip address and port information for each drillbit and 
populate the list
+for (String drillbit : connectInfo) {
+
+  // Trim all the empty spaces and check if the entry is empty string.
+  // Ignore the empty ones.
+  drillbit = drillbit.trim();
+
+  if (!drillbit.isEmpty()) {
+// Verify if we have only ":" or only ":port" pattern
+if (drillbit.charAt(0) == ':') {
+  // Invalid drillbit information
+  throw new InvalidConnectionInfoException("Malformed connection 
string with drillbit hostname or " +
+ "hostaddress missing 
for an entry: " + drillbit);
+}
+
+// We are now sure that each ip:port entry will have both the 
values atleast once.
+// Split each drillbit connection string to get ip address and 
port value
+final String[] drillbitInfo = drillbit.split(":");
+
+// Check if we have more than one port
+if (drillbitInfo.length > 2) {
+  throw new InvalidConnectionInfoException("Malformed connection 
string with more than one port in a " +
+ "drillbit entry: " + 
drillbit);
+}
+
+// At this point we are sure that drillbitInfo has atleast 
hostname or host address
+// trim all the empty spaces which might be present in front of 
hostname or
+// host address information
+final String ipAddress = drillbitInfo[0].trim();
+String port = defaultUserPort;
+
+if (drillbitInfo.length == 2) {
+  // We have a port value also given by user. trim all the empty 
spaces between : and port value before
+  // validating the correctness of value.
+  port = drillbitInfo[1].trim();
+}
+
+try {
+  final DrillbitEndpoint endpoint = DrillbitEndpoint.newBuilder()
+.setAddress(ipAddress)
+
.setUserPort(Integer.parseInt(port))
+.build();
+
+  endpointList.add(endpoint);
+} catch (NumberFormatException e) {
+  throw new InvalidConnectionInfoException("Malformed port value 
in entry: " + ipAddress + ":" + port + " " +
+ "passed in connection 
string");
+}
+  }
+}
+if(endpointList.size() == 0){
--- 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 #648: DRILL-5015: Randomly select the drillbit from the l...

2016-11-29 Thread sohami
Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/648#discussion_r90089750
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java ---
@@ -223,19 +224,100 @@ public void connect(Properties props) throws 
RpcException {
 connect(null, props);
   }
 
+  /**
+   * Populates the endpointlist with drillbits information provided in the 
connection string by client.
+   * For direct connection we can have connection string with drillbit 
property as below:
+   * 
+   *   drillbit=ip
+   *   use the ip specified as the Foreman ip with default port in 
config file
+   *   drillbit=ip:port
+   *   use the ip and port specified as the Foreman ip and port
+   *   drillbit=ip1:port1,ip2:port2,...
+   *   randomly select the ip and port pair from the specified list as 
the Foreman ip and port.
+   * 
+   *
+   * @param drillbits string with drillbit value provided in connection 
string
+   * @param defaultUserPort string with default userport of drillbit 
specified in config file
+   * @return list of drillbit endpoints parsed from connection string
+   * @throws InvalidConnectionInfoException if the connection string has 
invalid or no drillbit information
+   */
+  static List parseAndVerifyEndpoints(String drillbits, 
String defaultUserPort)
+throws InvalidConnectionInfoException {
+// If no drillbits is provided then throw exception
+drillbits = drillbits.trim();
+if (drillbits.isEmpty()) {
+  throw new InvalidConnectionInfoException("No drillbit information 
specified in the connection string");
+}
+
+ArrayList endpointList = new ArrayList<>();
--- 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 #648: DRILL-5015: Randomly select the drillbit from the l...

2016-11-29 Thread sohami
Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/648#discussion_r89929966
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/InvalidConnectionInfoException.java
 ---
@@ -0,0 +1,29 @@
+/**
+ * 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.rpc;
+
+/**
+ * Exception for malformed connection string from client
+ */
+public class InvalidConnectionInfoException extends RpcException {
--- End diff --

The connect method only throws RPCException. Also didn't want to extend the 
method signature to throw one more checked exception since overtime that can 
grow.


---
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 #648: DRILL-5015: Randomly select the drillbit from the l...

2016-11-29 Thread sohami
Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/648#discussion_r90091315
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/InvalidConnectionInfoException.java
 ---
@@ -0,0 +1,29 @@
+/**
+ * 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.rpc;
--- End diff --

discussed offline to keep it as is.


---
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 #671: DRILL-4347: Propagate distinct row count for joins from lo...

2016-11-29 Thread julianhyde
Github user julianhyde commented on the issue:

https://github.com/apache/drill/pull/671
  
Yes, CachingRelMetadataProvider is meant for this.

After https://issues.apache.org/jira/browse/CALCITE-604, providers are 
considerably more efficient (not called via reflection), and RelMetadataQuery 
contains a per-request cache (because some metadata does change, but slowly).


---
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 #670: DRILL-5065 - Optimize count( * ) queries on MapR-DB JSON T...

2016-11-29 Thread adityakishore
Github user adityakishore commented on the issue:

https://github.com/apache/drill/pull/670
  
Could you please add couple of unit test to verify the plan?


---
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 #671: DRILL-4347: Propagate distinct row count for joins from lo...

2016-11-29 Thread amansinha100
Github user amansinha100 commented on the issue:

https://github.com/apache/drill/pull/671
  
It is quite likely that the CachingRelMetadataProvider is meant for this.  
Based on the stack trace, there are multiple instances of "at 
org.apache.calcite.rel.metadata.CachingRelMetadataProvider$CachingInvocationHandler.invoke(CachingRelMetadataProvider.java:132)"
  and that line # indicates that there was either a cache miss or the entry was 
stale.   So, the caching provider does in fact get used but then subsequently 
gets stuck in the apply() method of the ReflectiveRelMetadataProvider.  I did 
not attempt to debug why it got stuck there...partly because I am not very 
familiar with the way reflection is used in this provider.  Hence, my fix is an 
attempt to circumvent the 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.
---


Re: [DISCUSS] Apache Drill Version after 1.9.0, etc.

2016-11-29 Thread Parth Chandra
Aren't we sidestepping the real questions -

What, if any, are the breaking changes we are looking at in the near
(or distant) term?
If we don't have any such changes on the horizon, then when are we
going to do a 2.0 release?

Before we finalize this I would like us to have the criteria for moving to
2.0 decided.

Ideally, we would want a roadmap of items that we consider breaking
changes. (I would also recommend starting a 2.0 branch for those breaking
changes).  I know some client side breaking changes slipped in without
causing any harm in a couple of previous releases, so is this even a
criterion? If it is, then let's have a plan (even a somewhat hopeful one)
for 2.0.

Parth


On Tue, Nov 29, 2016 at 9:08 AM, Neeraja Rentachintala <
nrentachint...@maprtech.com> wrote:

> +1 on continuing to 1.10 version after 1.9 release.
>
>
> On Mon, Nov 28, 2016 at 7:49 PM, Aman Sinha  wrote:
>
> > (A) I am leaning to 1.10 for the reasons already mentioned in your email.
> > (B) sounds good.
> > (C) Does it matter if there are a few commits in master branch already ?
> > What's the implication of just updating the pom files (not force-push).
> >
> > On Mon, Nov 28, 2016 at 3:25 PM, Sudheesh Katkam 
> > wrote:
> >
> > > Hi all,
> > >
> > > -
> > >
> > > (A) I had asked the question about what the release version should be
> > > after 1.9.0. Since this is part of the next release plan, a vote is
> > > required based on the discussion. For approval, the vote requires a
> lazy
> > > majority of active committers over 3 days.
> > >
> > > Here are some comments from that thread:
> > >
> > > Quoting Paul:
> > >
> > > > For release numbers, 1.10 (then 1.11, 1.12, …) seems like a good
> idea.
> > > >
> > > > At first it may seem odd to go to 1.10 from 1.9. Might people get
> > > confused between 1.10 and 1.1.0? But, there is precedence. Tomcat’s
> > latest
> > > 7-series release is 7.0.72. Java is on 8u112. And so on.
> > > >
> > > > I like the idea of moving to 2.0 later when the team introduces a
> major
> > > change, rather than by default just because the numbers roll around.
> For
> > > example, Hadoop when to 2.x when YARN was introduced. Impala appears to
> > > have moved to 2.0 when they added Spill to disk for some (all?)
> > operators.
> > >
> > >
> > > Quoting Parth:
> > >
> > > > Specifically what did you want to discuss about the release number
> > after
> > > 1.9?  Ordinarily you would just go to 2.0. The only reason for holding
> > off
> > > on 2.0, that I can think of, is if you want to make breaking changes in
> > the
> > > 2.0 release and those are not going to be ready for the next release
> > cycle.
> > > Are any dev's planning on such breaking changes? If so we should
> discuss
> > > that (or any other reason we might have for deferring 2.0) in a
> separate
> > > thread?
> > > > I'm +0 on any version number we chose.
> > >
> > >
> > > I am +1 on Paul’s suggestion for 1.10.0, unless, as Parth noted, we
> plan
> > > to make breaking changes in the next release cycle.
> > >
> > > @Jacques, any comments? You had mentioned about this a while back [1].
> > >
> > > -
> > >
> > > (B) Until discussion on (A) is complete, which may take a while, I
> > propose
> > > we move the master to 1.10.0-SNAPSHOT to unblock committing to master
> > > branch. If there are no objections, I will do this tomorrow, once 1.9.0
> > > release artifacts are propagated.
> > >
> > > -
> > >
> > > (C) I noticed there are some changes committed to master branch before
> > the
> > > commit that moves to the next snapshot version. Did we face this issue
> in
> > > the past? If so, how did we resolve the issue? Is 'force push' an
> option?
> > >
> > > -
> > >
> > > Thank you,
> > > Sudheesh
> > >
> > > [1] http://mail-archives.apache.org/mod_mbox/drill-dev/201604.mbox/%
> > > 3CCAJrw0OTiXLnmW25K0aQtsVmh3A4vxfwZzvHntxeYJjPdd-PnYQ%40mail.gmail.com
> > %3E
> > >  > > 3ccajrw0otixlnmw25k0aqtsvmh3a4vxfwzzvhntxeyjjpdd-p...@mail.gmail.com
> %3E>
> >
>


Re: [PROPOSAL] Apache Jira Workflow for Code Reviews

2016-11-29 Thread Julian Hyde
I like 2 also. 

Is the following variant of 2 possible in JIRA? Make the "ready to commit" flag 
into a status. Thus status changes from "open" to "in progress" to "reviewable" 
to "ready to commit" (or "approved").

The inability to search by assignee should not be a huge problem - it is in the 
interests of the project to ensure that the number of cases in "reviewable" or 
"ready to commit" state at any moment is small. 

And furthermore, the author of a bug often doesn't know or care who will review 
or commit it. So the very notion of an assignee is suspect.

Julian

> On Nov 29, 2016, at 07:53, Aman Sinha  wrote:
> 
> My preference is for #2 because of the reasons you mention.  For #1 my
> recollection is the DrillCommitter was used at a time when there were only
> couple of committers.  Today there are many.  Changing to DrillCommitter
> will leave ambiguity about which committer should look at it.
> Additionally,  I feel that today we are not using the field 'Reviewer' for
> its correct purpose.  This argues for #2.
> 
> One note about #2 worth mentioning is that folks would have to change their
> search criteria  to 'assigned *OR reviewer*' to find out what is on their
> plate.
> 
>> On Mon, Nov 28, 2016 at 4:51 PM, Zelaine Fong  wrote:
>> 
>> I'd like to propose a small change to the current process for code reviews
>> in Apache Drill.  We need this change because non-committers are becoming
>> more involved in code reviews.
>> 
>> The current process is described at
>> https://drill.apache.org/docs/apache-drill-contribution-
>> guidelines/#step-3:-get-your-code-reviewed-and-committed-to-the-project.
>> It assumes that the individuals doing code reviews are code committers.
>> Here's a brief summary of the process:
>> 
>> *Current Process:*
>> Currently, the workflow is based on updating the Status and Assignee
>> fields.  When a pull request has been posted and is ready for review, the
>> issue status is changed to "Reviewable" and the Assignee is changed to the
>> designated code reviewer.  If the review is completed and the reviewer is a
>> committer, the reviewer/committer will commit the change. If the code
>> reviewer has comments that require the contributor to address, the issue
>> status changes back to "In Progress" and the Assignee is changed back to
>> the contributor.
>> 
>> *Proposed Change:*
>> The proposed change is to address the case where another step may be needed
>> to commit changes, if the changes have been reviewed by a non-committer.
>> Here are a couple of proposals on how to address this.  In both cases, once
>> a review has been satisfactorily completed on a pull request, a committer
>> will take over, bless the changes based on the prior review comments, do a
>> +1 on the patch, and then commit.
>> 
>> 
>> *Proposal #1:*When a fix is ready to be reviewed, set the Assignee to
>> "DrillCommitter".
>> 
>> Pros: Simple.  User "DrillCommitter" already exists.
>> Cons: Similar to changing the Assignee to the code reviewer, you lose track
>> of who is the original contributor of an issue.  Yes, it's in the comment
>> history, but there is no automated/easy way to get this info.
>> 
>> *Proposal #2:*
>> Leave the Assignee field unchanged.  This is also means changing the
>> current process of changing the Assignee to the code reviewer.  Instead,
>> 
>> 1) Continue to set the Status to "Reviewable" to indicate that the fix is
>> ready for review, but set the Reviewer field to the designated code
>> reviewer.
>> 2) If changes are needed after code review, change the status of the issue
>> back to "In Progress" as today.  But again, don't change the Assignee.
>> 3) If the changes are acceptable, the reviewer applies the
>> "ready-to-commit" tag on the Jira.
>> 
>> Pros: Maintains Assignee field
>> Cons: Requires querying for the tag "ready-to-commit" to find commit-ready
>> fixes.
>> 
>> Please comment with your preferences for proposal 1 or 2, or if you have
>> other suggestions.
>> 
>> Thanks.
>> 
>> -- Zelaine
>> 


Re: [DISCUSS] Apache Drill Version after 1.9.0, etc.

2016-11-29 Thread Neeraja Rentachintala
+1 on continuing to 1.10 version after 1.9 release.


On Mon, Nov 28, 2016 at 7:49 PM, Aman Sinha  wrote:

> (A) I am leaning to 1.10 for the reasons already mentioned in your email.
> (B) sounds good.
> (C) Does it matter if there are a few commits in master branch already ?
> What's the implication of just updating the pom files (not force-push).
>
> On Mon, Nov 28, 2016 at 3:25 PM, Sudheesh Katkam 
> wrote:
>
> > Hi all,
> >
> > -
> >
> > (A) I had asked the question about what the release version should be
> > after 1.9.0. Since this is part of the next release plan, a vote is
> > required based on the discussion. For approval, the vote requires a lazy
> > majority of active committers over 3 days.
> >
> > Here are some comments from that thread:
> >
> > Quoting Paul:
> >
> > > For release numbers, 1.10 (then 1.11, 1.12, …) seems like a good idea.
> > >
> > > At first it may seem odd to go to 1.10 from 1.9. Might people get
> > confused between 1.10 and 1.1.0? But, there is precedence. Tomcat’s
> latest
> > 7-series release is 7.0.72. Java is on 8u112. And so on.
> > >
> > > I like the idea of moving to 2.0 later when the team introduces a major
> > change, rather than by default just because the numbers roll around. For
> > example, Hadoop when to 2.x when YARN was introduced. Impala appears to
> > have moved to 2.0 when they added Spill to disk for some (all?)
> operators.
> >
> >
> > Quoting Parth:
> >
> > > Specifically what did you want to discuss about the release number
> after
> > 1.9?  Ordinarily you would just go to 2.0. The only reason for holding
> off
> > on 2.0, that I can think of, is if you want to make breaking changes in
> the
> > 2.0 release and those are not going to be ready for the next release
> cycle.
> > Are any dev's planning on such breaking changes? If so we should discuss
> > that (or any other reason we might have for deferring 2.0) in a separate
> > thread?
> > > I'm +0 on any version number we chose.
> >
> >
> > I am +1 on Paul’s suggestion for 1.10.0, unless, as Parth noted, we plan
> > to make breaking changes in the next release cycle.
> >
> > @Jacques, any comments? You had mentioned about this a while back [1].
> >
> > -
> >
> > (B) Until discussion on (A) is complete, which may take a while, I
> propose
> > we move the master to 1.10.0-SNAPSHOT to unblock committing to master
> > branch. If there are no objections, I will do this tomorrow, once 1.9.0
> > release artifacts are propagated.
> >
> > -
> >
> > (C) I noticed there are some changes committed to master branch before
> the
> > commit that moves to the next snapshot version. Did we face this issue in
> > the past? If so, how did we resolve the issue? Is 'force push' an option?
> >
> > -
> >
> > Thank you,
> > Sudheesh
> >
> > [1] http://mail-archives.apache.org/mod_mbox/drill-dev/201604.mbox/%
> > 3CCAJrw0OTiXLnmW25K0aQtsVmh3A4vxfwZzvHntxeYJjPdd-PnYQ%40mail.gmail.com
> %3E
> >  > 3ccajrw0otixlnmw25k0aqtsvmh3a4vxfwzzvhntxeyjjpdd-p...@mail.gmail.com%3E>
>


Re: [DISCUSS] Apache Drill Version after 1.9.0, etc.

2016-11-29 Thread Jacques Nadeau
Hey Sudheesh,

Thanks for asking my opinion given my statements back in April. I
appreciate the thought but I prefer to defer to others who are more
actively contributing than myself.

With regards to (C): I ran numerous releases previously where we simply
forward ported any fixes that were wanted from a release branch back into
master. There is no formal requirement for a release commit to be on the
master branch. In general, once the release branch is started, I typically
suggest simply rolling forward the master branch to the next snapshot
version immediately. Note that different projects think about this
differently. As an example: on the Calcite project we typically lock the
master branch so that the release is in the master branch.

--
Jacques Nadeau
CTO and Co-Founder, Dremio

On Mon, Nov 28, 2016 at 7:49 PM, Aman Sinha  wrote:

> (A) I am leaning to 1.10 for the reasons already mentioned in your email.
> (B) sounds good.
> (C) Does it matter if there are a few commits in master branch already ?
> What's the implication of just updating the pom files (not force-push).
>
> On Mon, Nov 28, 2016 at 3:25 PM, Sudheesh Katkam 
> wrote:
>
> > Hi all,
> >
> > -
> >
> > (A) I had asked the question about what the release version should be
> > after 1.9.0. Since this is part of the next release plan, a vote is
> > required based on the discussion. For approval, the vote requires a lazy
> > majority of active committers over 3 days.
> >
> > Here are some comments from that thread:
> >
> > Quoting Paul:
> >
> > > For release numbers, 1.10 (then 1.11, 1.12, …) seems like a good idea.
> > >
> > > At first it may seem odd to go to 1.10 from 1.9. Might people get
> > confused between 1.10 and 1.1.0? But, there is precedence. Tomcat’s
> latest
> > 7-series release is 7.0.72. Java is on 8u112. And so on.
> > >
> > > I like the idea of moving to 2.0 later when the team introduces a major
> > change, rather than by default just because the numbers roll around. For
> > example, Hadoop when to 2.x when YARN was introduced. Impala appears to
> > have moved to 2.0 when they added Spill to disk for some (all?)
> operators.
> >
> >
> > Quoting Parth:
> >
> > > Specifically what did you want to discuss about the release number
> after
> > 1.9?  Ordinarily you would just go to 2.0. The only reason for holding
> off
> > on 2.0, that I can think of, is if you want to make breaking changes in
> the
> > 2.0 release and those are not going to be ready for the next release
> cycle.
> > Are any dev's planning on such breaking changes? If so we should discuss
> > that (or any other reason we might have for deferring 2.0) in a separate
> > thread?
> > > I'm +0 on any version number we chose.
> >
> >
> > I am +1 on Paul’s suggestion for 1.10.0, unless, as Parth noted, we plan
> > to make breaking changes in the next release cycle.
> >
> > @Jacques, any comments? You had mentioned about this a while back [1].
> >
> > -
> >
> > (B) Until discussion on (A) is complete, which may take a while, I
> propose
> > we move the master to 1.10.0-SNAPSHOT to unblock committing to master
> > branch. If there are no objections, I will do this tomorrow, once 1.9.0
> > release artifacts are propagated.
> >
> > -
> >
> > (C) I noticed there are some changes committed to master branch before
> the
> > commit that moves to the next snapshot version. Did we face this issue in
> > the past? If so, how did we resolve the issue? Is 'force push' an option?
> >
> > -
> >
> > Thank you,
> > Sudheesh
> >
> > [1] http://mail-archives.apache.org/mod_mbox/drill-dev/201604.mbox/%
> > 3CCAJrw0OTiXLnmW25K0aQtsVmh3A4vxfwZzvHntxeYJjPdd-PnYQ%40mail.gmail.com
> %3E
> >  > 3ccajrw0otixlnmw25k0aqtsvmh3a4vxfwzzvhntxeyjjpdd-p...@mail.gmail.com%3E>
>


[GitHub] drill issue #671: DRILL-4347: Propagate distinct row count for joins from lo...

2016-11-29 Thread jacques-n
Github user jacques-n commented on the issue:

https://github.com/apache/drill/pull/671
  
Random question: won't a caching metadata provider achieve the same thing 
without having to do per rel node changes? At least that is what I thought it 
was for. Maybe @julianhyde can provide additional 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 #671: DRILL-4347: Propagate distinct row count for joins ...

2016-11-29 Thread amansinha100
GitHub user amansinha100 opened a pull request:

https://github.com/apache/drill/pull/671

DRILL-4347: Propagate distinct row count for joins from logical plann…

…ing to physical planning phase.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/amansinha100/incubator-drill DRILL-4347-1

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/drill/pull/671.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 #671


commit f2036036b7e61bc100ab23da5003c32350327c08
Author: Aman Sinha 
Date:   2016-11-28T16:42:15Z

DRILL-4347: Propagate distinct row count for joins from logical planning to 
physical planning phase.




---
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: [PROPOSAL] Apache Jira Workflow for Code Reviews

2016-11-29 Thread Aman Sinha
My preference is for #2 because of the reasons you mention.  For #1 my
recollection is the DrillCommitter was used at a time when there were only
couple of committers.  Today there are many.  Changing to DrillCommitter
will leave ambiguity about which committer should look at it.
Additionally,  I feel that today we are not using the field 'Reviewer' for
its correct purpose.  This argues for #2.

One note about #2 worth mentioning is that folks would have to change their
search criteria  to 'assigned *OR reviewer*' to find out what is on their
plate.

On Mon, Nov 28, 2016 at 4:51 PM, Zelaine Fong  wrote:

> I'd like to propose a small change to the current process for code reviews
> in Apache Drill.  We need this change because non-committers are becoming
> more involved in code reviews.
>
> The current process is described at
> https://drill.apache.org/docs/apache-drill-contribution-
> guidelines/#step-3:-get-your-code-reviewed-and-committed-to-the-project.
> It assumes that the individuals doing code reviews are code committers.
> Here's a brief summary of the process:
>
> *Current Process:*
> Currently, the workflow is based on updating the Status and Assignee
> fields.  When a pull request has been posted and is ready for review, the
> issue status is changed to "Reviewable" and the Assignee is changed to the
> designated code reviewer.  If the review is completed and the reviewer is a
> committer, the reviewer/committer will commit the change. If the code
> reviewer has comments that require the contributor to address, the issue
> status changes back to "In Progress" and the Assignee is changed back to
> the contributor.
>
> *Proposed Change:*
> The proposed change is to address the case where another step may be needed
> to commit changes, if the changes have been reviewed by a non-committer.
> Here are a couple of proposals on how to address this.  In both cases, once
> a review has been satisfactorily completed on a pull request, a committer
> will take over, bless the changes based on the prior review comments, do a
> +1 on the patch, and then commit.
>
>
> *Proposal #1:*When a fix is ready to be reviewed, set the Assignee to
> "DrillCommitter".
>
> Pros: Simple.  User "DrillCommitter" already exists.
> Cons: Similar to changing the Assignee to the code reviewer, you lose track
> of who is the original contributor of an issue.  Yes, it's in the comment
> history, but there is no automated/easy way to get this info.
>
> *Proposal #2:*
> Leave the Assignee field unchanged.  This is also means changing the
> current process of changing the Assignee to the code reviewer.  Instead,
>
> 1) Continue to set the Status to "Reviewable" to indicate that the fix is
> ready for review, but set the Reviewer field to the designated code
> reviewer.
> 2) If changes are needed after code review, change the status of the issue
> back to "In Progress" as today.  But again, don't change the Assignee.
> 3) If the changes are acceptable, the reviewer applies the
> "ready-to-commit" tag on the Jira.
>
> Pros: Maintains Assignee field
> Cons: Requires querying for the tag "ready-to-commit" to find commit-ready
> fixes.
>
> Please comment with your preferences for proposal 1 or 2, or if you have
> other suggestions.
>
> Thanks.
>
> -- Zelaine
>


[GitHub] drill pull request #669: DRILL-5044: After the dynamic registration of multi...

2016-11-29 Thread arina-ielchiieva
Github user arina-ielchiieva commented on a diff in the pull request:

https://github.com/apache/drill/pull/669#discussion_r89993422
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DropFunctionHandler.java
 ---
@@ -143,7 +143,7 @@ private Jar unregister(String jarName, 
RemoteFunctionRegistry remoteFunctionRegi
 if (retryAttempts-- == 0) {
   throw new DrillRuntimeException("Failed to update remote 
function registry. Exceeded retry attempts limit.");
 }
-unregister(jarName, remoteFunctionRegistry, retryAttempts);
+return unregister(jarName, remoteFunctionRegistry, retryAttempts);
--- End diff --

Agree. 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 #669: DRILL-5044: After the dynamic registration of multi...

2016-11-29 Thread arina-ielchiieva
Github user arina-ielchiieva commented on a diff in the pull request:

https://github.com/apache/drill/pull/669#discussion_r89994171
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateFunctionHandler.java
 ---
@@ -175,22 +175,20 @@ private void initRemoteRegistration(List 
functions,
 List remoteJars = 
remoteRegistry.getRegistry(version).getJarList();
 validateAgainstRemoteRegistry(remoteJars, jarManager.getBinaryName(), 
functions);
 jarManager.copyToRegistryArea();
-boolean cleanUp = true;
 List jars = Lists.newArrayList(remoteJars);
 
jars.add(Jar.newBuilder().setName(jarManager.getBinaryName()).addAllFunctionSignature(functions).build());
 Registry updatedRegistry = 
Registry.newBuilder().addAllJar(jars).build();
 try {
   remoteRegistry.updateRegistry(updatedRegistry, version);
-  cleanUp = false;
 } catch (VersionMismatchException ex) {
+  jarManager.deleteQuietlyFromRegistryArea();
--- End diff --

1. I guess having fixed number of retries is enough. Having retry and wait 
logic, may lead us to the point where user will have to wait for a long time 
till registration completes in case of busy system. With only retry logic we 
notify user pretty quickly that the system is busy and it's up to the user to 
decide when to try to register the function again,

2. Totally agree about recursion, since user may modify number of retry 
attempts, it's much better to have while loop to avoid stack overflow.


---
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 #669: DRILL-5044: After the dynamic registration of multi...

2016-11-29 Thread arina-ielchiieva
Github user arina-ielchiieva commented on a diff in the pull request:

https://github.com/apache/drill/pull/669#discussion_r89994657
  
--- Diff: 
exec/java-exec/src/test/java/org/apache/drill/TestDynamicUDFSupport.java ---
@@ -271,6 +271,75 @@ public void testDuplicatedFunctionsInLocalRegistry() 
throws Exception {
   }
 
   @Test
+  public void testSuccessfulRegistrationAfterSeveralRetryAttempts() throws 
Exception {
--- End diff --

Actually test `testSuccessfulRegistrationAfterSeveralRetryAttempts()` 
covers this case. To produce this issue we don't need several drillbits, it's 
just enough to issue several concurrent create function commands. As for this 
test case, I just mock method that updates remote function registry to throw 
VersionMismatchException two times and then call this method for real.


---
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 #669: DRILL-5044: After the dynamic registration of multi...

2016-11-29 Thread arina-ielchiieva
Github user arina-ielchiieva commented on a diff in the pull request:

https://github.com/apache/drill/pull/669#discussion_r89995323
  
--- Diff: 
exec/java-exec/src/test/java/org/apache/drill/TestDynamicUDFSupport.java ---
@@ -271,6 +271,75 @@ public void testDuplicatedFunctionsInLocalRegistry() 
throws Exception {
   }
 
   @Test
+  public void testSuccessfulRegistrationAfterSeveralRetryAttempts() throws 
Exception {
+RemoteFunctionRegistry remoteFunctionRegistry = 
spyRemoteFunctionRegistry();
+copyDefaultJarsToStagingArea();
+
+doThrow(new VersionMismatchException("Version mismatch detected", 1))
+.doThrow(new VersionMismatchException("Version mismatch 
detected", 1))
+.doCallRealMethod()
+
.when(remoteFunctionRegistry).updateRegistry(any(Registry.class), 
any(DataChangeVersion.class));
+
+String summary = "The following UDFs in jar %s have been 
registered:\n" +
+"[custom_lower(VARCHAR-REQUIRED)]";
+
+testBuilder()
+.sqlQuery("create function using jar '%s'", 
default_binary_name)
+.unOrdered()
+.baselineColumns("ok", "summary")
+.baselineValues(true, String.format(summary, 
default_binary_name))
+.go();
+
+verify(remoteFunctionRegistry, times(3))
+.updateRegistry(any(Registry.class), 
any(DataChangeVersion.class));
+
+FileSystem fs = remoteFunctionRegistry.getFs();
+
+assertFalse("Staging area should be empty", 
fs.listFiles(remoteFunctionRegistry.getStagingArea(), false).hasNext());
+assertFalse("Temporary area should be empty", 
fs.listFiles(remoteFunctionRegistry.getTmpArea(), false).hasNext());
+
+assertTrue("Binary should be present in registry area",
+fs.exists(new Path(remoteFunctionRegistry.getRegistryArea(), 
default_binary_name)));
+assertTrue("Source should be present in registry area",
+fs.exists(new Path(remoteFunctionRegistry.getRegistryArea(), 
default_source_name)));
+
+Registry registry = remoteFunctionRegistry.getRegistry();
+assertEquals("Registry should contain one jar", 
registry.getJarList().size(), 1);
+assertEquals(registry.getJar(0).getName(), default_binary_name);
+  }
+
+  @Test
+  public void testSuccessfulUnregistrationAfterSeveralRetryAttempts() 
throws Exception {
+RemoteFunctionRegistry remoteFunctionRegistry = 
spyRemoteFunctionRegistry();
+copyDefaultJarsToStagingArea();
+test("create function using jar '%s'", default_binary_name);
+
+reset(remoteFunctionRegistry);
+doThrow(new VersionMismatchException("Version mismatch detected", 1))
--- End diff --

It's Mockito functionality. You can mock the method to return failure or 
any result when it is being called.
In this case we mock `updateRegistry()` method to return 
VersionMismatchException first two times. This way we simulate the situation 
that someone has updated remote function registry before us.
After that we instruct to call real method.


---
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 #669: DRILL-5044: After the dynamic registration of multi...

2016-11-29 Thread arina-ielchiieva
Github user arina-ielchiieva commented on a diff in the pull request:

https://github.com/apache/drill/pull/669#discussion_r89993367
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateFunctionHandler.java
 ---
@@ -175,22 +175,20 @@ private void initRemoteRegistration(List 
functions,
 List remoteJars = 
remoteRegistry.getRegistry(version).getJarList();
 validateAgainstRemoteRegistry(remoteJars, jarManager.getBinaryName(), 
functions);
 jarManager.copyToRegistryArea();
-boolean cleanUp = true;
 List jars = Lists.newArrayList(remoteJars);
 
jars.add(Jar.newBuilder().setName(jarManager.getBinaryName()).addAllFunctionSignature(functions).build());
 Registry updatedRegistry = 
Registry.newBuilder().addAllJar(jars).build();
 try {
   remoteRegistry.updateRegistry(updatedRegistry, version);
-  cleanUp = false;
 } catch (VersionMismatchException ex) {
+  jarManager.deleteQuietlyFromRegistryArea();
   if (retryAttempts-- == 0) {
 throw new DrillRuntimeException("Failed to update remote function 
registry. Exceeded retry attempts limit.");
   }
   initRemoteRegistration(functions, jarManager, remoteRegistry, 
retryAttempts);
-} finally {
-  if (cleanUp) {
-jarManager.deleteQuietlyFromRegistryArea();
-  }
+} catch (Exception e) {
--- End diff --

You are right. Updated the code, so now we delete jars only once on error.


---
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: find bug in drill version 1.80

2016-11-29 Thread Khurram Faraaz
Hello Yun,

Can you please share your SQL and the explain plan ?
And is the query over parquet, CSV or JSON data ?

Thanks,
Khurram

On Tue, Nov 29, 2016 at 10:51 AM, 马云  wrote:

> Hi Drill dev
>
>
> I used drill 1.80 to setup 3 node's drill cluster.
> but sometimes when I execute sqls, it will get no response even wait a
> very long time.
> and I checked the drill web console. I found that some there are some
> fragements status is always SENDING.
> Why it is always SENDING and no timeout? Could you tell me what I should
> do to avoid this issue
>
>
>
>
>
> thanks
> Yun
>
>
>
>
>
>
>
>
>
>
>