[GitHub] drill pull request: DRILL-4287: During initial DrillTable creation...

2016-02-15 Thread jinfengni
Github user jinfengni commented on a diff in the pull request:

https://github.com/apache/drill/pull/376#discussion_r52973451
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetGroupScan.java
 ---
@@ -529,6 +549,36 @@ public long getRowCount() {
 
   }
 
+
+  // Create and return a new file selection based on reading the metadata 
cache file.
+  // This function also initializes a few of ParquetGroupScan's fields as 
appropriate.
+  private FileSelection
+  initFromMetadataCache(DrillFileSystem fs, FileSelection selection) 
throws IOException {
+FileStatus metaRootDir = selection.getFirstPath(fs);
+Path metaFilePath = new Path(metaRootDir.getPath(), 
Metadata.METADATA_FILENAME);
+
+// get (and set internal field) the metadata for the directory by 
reading the metadata file
+this.parquetTableMetadata = Metadata.readBlockMeta(fs, 
metaFilePath.toString());
+List fileNames = Lists.newArrayList();
+for (Metadata.ParquetFileMetadata file : 
parquetTableMetadata.getFiles()) {
+  fileNames.add(file.getPath());
+}
+// when creating the file selection, set the selection root in the 
form /a/b instead of
+// file:/a/b.  The reason is that the file names above have been 
created in the form
+// /a/b/c.parquet and the format of the selection root must match that 
of the file names
+// otherwise downstream operations such as partition pruning can break.
+final Path metaRootPath = 
Path.getPathWithoutSchemeAndAuthority(metaRootDir.getPath());
+this.selectionRoot = metaRootPath.toString();
+
+// Use the FileSelection constructor directly here instead of the 
FileSelection.create() method
+// because create() changes the root to include the scheme and 
authority; In future, if create()
+// is the preferred way to instantiate a file selection, we may need 
to do something different...
+FileSelection newSelection = new 
FileSelection(selection.getStatuses(fs), fileNames, metaRootPath.toString());
--- End diff --

Seems the first argument : selection.getStatuses(fs) is the file status 
before being expanded from cache, while the second argument fileNames 
represents the expanded file names from cache.  Will such difference cause any 
problem? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4287: During initial DrillTable creation...

2016-02-15 Thread jinfengni
Github user jinfengni commented on a diff in the pull request:

https://github.com/apache/drill/pull/376#discussion_r52973315
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/FileSelection.java 
---
@@ -68,6 +79,7 @@ protected FileSelection(final FileSelection selection) {
 this.statuses = selection.statuses;
 this.files = selection.files;
 this.selectionRoot = selection.selectionRoot;
+this.dirStatus = new BitSet(StatusType.values().length);
--- End diff --

Should this constructor copy the dirStatus from the input "selection" as 
well?  


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


Question about generating the markdown pages for the Drill site

2016-02-15 Thread Jason Altekruse
Hello Devs,

I am currently trying to publish a blog post for the 1.5 release before I
send out the announcement to the mailing list, and I am having a little
trouble with the Jekyll site generation.

I pulled down the latest gh-pages branch and wrote a new blog post based on
a draft from Bridget:
https://github.com/jaltekruse/incubator-drill/commit/0e88b88a6acd4dcfe61ef849c4ed13aa878485d7

When I went to go generate the site using the commands from the readme on
the gh-pages branch I am getting errors for old doc pages that are missing
dates. I can fix each of the pages by editing it and committing to cause
the date auto-generation, but it appears that there are quite a few pages
missing the dates.

Are these the up-to-date commands that have been used recently? Is there
something I should be doing to make it skip the missing dates?

(taken from the readme page)

jekyll build --config _config.yml,_config-prod.yml
_tools/createdatadocs.py
jekyll build --config _config.yml,_config-prod.yml

Thanks in advance for any help!
Jason


[GitHub] drill pull request: DRILL-4287: During initial DrillTable creation...

2016-02-15 Thread amansinha100
Github user amansinha100 commented on the pull request:

https://github.com/apache/drill/pull/376#issuecomment-184481180
  
I have incorporated the review comments and updated the PR.  Please take a 
look when you get a chance.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread hnfgns
Github user hnfgns commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52962267
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/store/provider/LocalPersistentStoreProvider.java
 ---
@@ -0,0 +1,74 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.store.sys.store.provider;
+
+import java.io.IOException;
+
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.exec.ExecConstants;
+import org.apache.drill.exec.exception.StoreException;
+import org.apache.drill.exec.store.dfs.DrillFileSystem;
+import org.apache.drill.exec.store.sys.PersistentStoreRegistry;
+import org.apache.drill.exec.store.sys.PersistentStore;
+import org.apache.drill.exec.store.sys.PersistentStoreConfig;
+import org.apache.drill.exec.store.sys.store.LocalPersistentStore;
+import org.apache.drill.exec.testing.store.NoWriteLocalStore;
+import org.apache.hadoop.fs.Path;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * A really simple provider that stores data in the local file system, one 
value per file.
+ */
+public class LocalPersistentStoreProvider extends 
BasePersistentStoreProvider {
+  private static final Logger logger = 
LoggerFactory.getLogger(LocalPersistentStoreProvider.class);
+
+  private final Path path;
+  private final DrillFileSystem fs;
--- End diff --

done.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread hnfgns
Github user hnfgns commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52962256
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/store/provider/ZookeeperPersistentStoreProvider.java
 ---
@@ -0,0 +1,85 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.store.sys.store.provider;
+
+import java.io.IOException;
+
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.curator.framework.CuratorFramework;
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.exec.coord.zk.ZKClusterCoordinator;
+import org.apache.drill.exec.exception.StoreException;
+import org.apache.drill.exec.store.dfs.DrillFileSystem;
+import org.apache.drill.exec.store.sys.PersistentStore;
+import org.apache.drill.exec.store.sys.PersistentStoreRegistry;
+import org.apache.drill.exec.store.sys.PersistentStoreConfig;
+import org.apache.drill.exec.store.sys.store.LocalPersistentStore;
+import org.apache.drill.exec.store.sys.store.ZookeeperPersistentStore;
+import org.apache.hadoop.fs.Path;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ZookeeperPersistentStoreProvider extends 
BasePersistentStoreProvider {
+  private static final Logger logger = 
LoggerFactory.getLogger(ZookeeperPersistentStoreProvider.class);
+
+  private static final String DRILL_EXEC_SYS_STORE_PROVIDER_ZK_BLOBROOT = 
"drill.exec.sys.store.provider.zk.blobroot";
+
+  private final CuratorFramework curator;
+  private final DrillFileSystem fs;
+  private final Path blobRoot;
+
+  public ZookeeperPersistentStoreProvider(final 
PersistentStoreRegistry registry) throws StoreException {
+this(registry.getConfig(), registry.getCoordinator().getCurator());
+  }
+
+  @VisibleForTesting
+  public ZookeeperPersistentStoreProvider(final DrillConfig config, final 
CuratorFramework curator) throws StoreException {
+this.curator = curator;
+
+if (config.hasPath(DRILL_EXEC_SYS_STORE_PROVIDER_ZK_BLOBROOT)) {
+  blobRoot = new 
Path(config.getString(DRILL_EXEC_SYS_STORE_PROVIDER_ZK_BLOBROOT));
+}else{
+  blobRoot = LocalPersistentStore.getLogDir();
+}
+
+try {
+  this.fs = LocalPersistentStore.getFileSystem(config, blobRoot);
+} catch (IOException ex) {
+  throw new StoreException("unable to get filesystem", ex);
+}
+  }
+
+  @Override
+  public  PersistentStore getOrCreateStore(final 
PersistentStoreConfig config) throws StoreException {
+switch(config.getMode()){
+case BLOB_PERSISTENT:
+  return new LocalPersistentStore<>(fs, blobRoot, config);
--- End diff --

zk relays blob persistency to fs.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread hnfgns
Github user hnfgns commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52962203
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/store/provider/CachingPersistentStoreProvider.java
 ---
@@ -0,0 +1,72 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.store.sys.store.provider;
+
+import java.util.concurrent.ConcurrentMap;
+
+import com.google.common.collect.Maps;
+import org.apache.drill.exec.exception.StoreException;
+import org.apache.drill.exec.store.sys.PersistentStore;
+import org.apache.drill.exec.store.sys.PersistentStoreConfig;
+import org.apache.drill.exec.store.sys.PersistentStoreProvider;
+
+public class CachingPersistentStoreProvider extends 
BasePersistentStoreProvider {
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(CachingPersistentStoreProvider.class);
+
+  private final ConcurrentMap storeCache = Maps.newConcurrentMap();
+  private final PersistentStoreProvider provider;
+
+  public CachingPersistentStoreProvider(PersistentStoreProvider provider) {
+this.provider = provider;
+  }
+
+  @SuppressWarnings("unchecked")
+  public  PersistentStore getOrCreateStore(final 
PersistentStoreConfig config) throws StoreException {
+final PersistentStore store = storeCache.get(config);
+if (store == null) {
+  final PersistentStore newStore = 
provider.getOrCreateStore(config);
+  final PersistentStore finalStore = storeCache.putIfAbsent(config, 
newStore);
+  if (finalStore == null) {
+return (PersistentStore)newStore;
+  }
+  try {
+newStore.close();
+  } catch (Exception ex) {
+throw new StoreException(ex);
+  }
+}
+
+return (PersistentStore) store;
+
+  }
+
+  @Override
+  public void start() throws Exception {
+provider.start();
+  }
+
+  @Override
+  public void close() throws Exception {
+for(final PersistentStore store : storeCache.values()){
+  store.close();
--- End diff --

AutoCloseables.close(storecache.values() + provider). Done.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread hnfgns
Github user hnfgns commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52962000
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java
 ---
@@ -146,38 +154,37 @@ public QProfiles(List runningQueries, 
List finishedQue
   @Path("/profiles.json")
   @Produces(MediaType.APPLICATION_JSON)
   public QProfiles getProfilesJSON() {
-PStore completed = null;
-PStore running = null;
-try {
-  completed = provider().getStore(QueryManager.QUERY_PROFILE);
-  running = provider().getStore(QueryManager.RUNNING_QUERY_INFO);
-} catch (IOException e) {
-  logger.debug("Failed to get profiles from persistent or ephemeral 
store.");
-  return new QProfiles(new ArrayList(), new 
ArrayList());
-}
-
-List runningQueries = Lists.newArrayList();
-
-for (Map.Entry entry : running) {
-  QueryInfo profile = entry.getValue();
-  if (principal.canManageProfileOf(profile.getUser())) {
-runningQueries.add(new ProfileInfo(entry.getKey(), 
profile.getStart(), profile.getForeman().getAddress(),
-profile.getQuery(), profile.getState().name(), 
profile.getUser()));
+try (
+final PersistentStore completed = 
getProvider().getOrCreateStore(QueryManager.QUERY_PROFILE);
--- End diff --

Fixed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread sudheeshkatkam
Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52961092
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/store/provider/LocalPersistentStoreProvider.java
 ---
@@ -0,0 +1,74 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.store.sys.store.provider;
+
+import java.io.IOException;
+
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.exec.ExecConstants;
+import org.apache.drill.exec.exception.StoreException;
+import org.apache.drill.exec.store.dfs.DrillFileSystem;
+import org.apache.drill.exec.store.sys.PersistentStoreRegistry;
+import org.apache.drill.exec.store.sys.PersistentStore;
+import org.apache.drill.exec.store.sys.PersistentStoreConfig;
+import org.apache.drill.exec.store.sys.store.LocalPersistentStore;
+import org.apache.drill.exec.testing.store.NoWriteLocalStore;
+import org.apache.hadoop.fs.Path;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * A really simple provider that stores data in the local file system, one 
value per file.
+ */
+public class LocalPersistentStoreProvider extends 
BasePersistentStoreProvider {
+  private static final Logger logger = 
LoggerFactory.getLogger(LocalPersistentStoreProvider.class);
+
+  private final Path path;
+  private final DrillFileSystem fs;
--- End diff --

Shouldn't fs be closed in close()?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread sudheeshkatkam
Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52960719
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/store/provider/CachingPersistentStoreProvider.java
 ---
@@ -0,0 +1,72 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.store.sys.store.provider;
+
+import java.util.concurrent.ConcurrentMap;
+
+import com.google.common.collect.Maps;
+import org.apache.drill.exec.exception.StoreException;
+import org.apache.drill.exec.store.sys.PersistentStore;
+import org.apache.drill.exec.store.sys.PersistentStoreConfig;
+import org.apache.drill.exec.store.sys.PersistentStoreProvider;
+
+public class CachingPersistentStoreProvider extends 
BasePersistentStoreProvider {
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(CachingPersistentStoreProvider.class);
+
+  private final ConcurrentMap storeCache = Maps.newConcurrentMap();
+  private final PersistentStoreProvider provider;
+
+  public CachingPersistentStoreProvider(PersistentStoreProvider provider) {
+this.provider = provider;
+  }
+
+  @SuppressWarnings("unchecked")
+  public  PersistentStore getOrCreateStore(final 
PersistentStoreConfig config) throws StoreException {
+final PersistentStore store = storeCache.get(config);
+if (store == null) {
+  final PersistentStore newStore = 
provider.getOrCreateStore(config);
+  final PersistentStore finalStore = storeCache.putIfAbsent(config, 
newStore);
+  if (finalStore == null) {
+return (PersistentStore)newStore;
+  }
+  try {
+newStore.close();
+  } catch (Exception ex) {
+throw new StoreException(ex);
+  }
+}
+
+return (PersistentStore) store;
+
+  }
+
+  @Override
+  public void start() throws Exception {
+provider.start();
+  }
+
+  @Override
+  public void close() throws Exception {
+for(final PersistentStore store : storeCache.values()){
+  store.close();
--- End diff --

AutoCloseables.close(storeCache.values());


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread sudheeshkatkam
Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52960717
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/PersistentStoreMode.java
 ---
@@ -6,22 +6,18 @@
  * to you under the Apache License, Version 2.0 (the
  * "License"); you may not use this file except in compliance
  * with the License.  You may obtain a copy of the License at
- *
+ * 
  * http://www.apache.org/licenses/LICENSE-2.0
- *
+ * 
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-
 package org.apache.drill.exec.store.sys;
 
-
-/**
- * Interface to define the provider which return EStore.
- */
-
-public interface EStoreProvider extends PStoreProvider {
+public enum PersistentStoreMode {
--- End diff --

javadoc


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread sudheeshkatkam
Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52960723
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/store/provider/ZookeeperPersistentStoreProvider.java
 ---
@@ -0,0 +1,85 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.store.sys.store.provider;
+
+import java.io.IOException;
+
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.curator.framework.CuratorFramework;
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.exec.coord.zk.ZKClusterCoordinator;
+import org.apache.drill.exec.exception.StoreException;
+import org.apache.drill.exec.store.dfs.DrillFileSystem;
+import org.apache.drill.exec.store.sys.PersistentStore;
+import org.apache.drill.exec.store.sys.PersistentStoreRegistry;
+import org.apache.drill.exec.store.sys.PersistentStoreConfig;
+import org.apache.drill.exec.store.sys.store.LocalPersistentStore;
+import org.apache.drill.exec.store.sys.store.ZookeeperPersistentStore;
+import org.apache.hadoop.fs.Path;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ZookeeperPersistentStoreProvider extends 
BasePersistentStoreProvider {
+  private static final Logger logger = 
LoggerFactory.getLogger(ZookeeperPersistentStoreProvider.class);
+
+  private static final String DRILL_EXEC_SYS_STORE_PROVIDER_ZK_BLOBROOT = 
"drill.exec.sys.store.provider.zk.blobroot";
+
+  private final CuratorFramework curator;
+  private final DrillFileSystem fs;
+  private final Path blobRoot;
+
+  public ZookeeperPersistentStoreProvider(final 
PersistentStoreRegistry registry) throws StoreException {
+this(registry.getConfig(), registry.getCoordinator().getCurator());
+  }
+
+  @VisibleForTesting
+  public ZookeeperPersistentStoreProvider(final DrillConfig config, final 
CuratorFramework curator) throws StoreException {
+this.curator = curator;
+
+if (config.hasPath(DRILL_EXEC_SYS_STORE_PROVIDER_ZK_BLOBROOT)) {
+  blobRoot = new 
Path(config.getString(DRILL_EXEC_SYS_STORE_PROVIDER_ZK_BLOBROOT));
+}else{
+  blobRoot = LocalPersistentStore.getLogDir();
+}
+
+try {
+  this.fs = LocalPersistentStore.getFileSystem(config, blobRoot);
+} catch (IOException ex) {
+  throw new StoreException("unable to get filesystem", ex);
+}
+  }
+
+  @Override
+  public  PersistentStore getOrCreateStore(final 
PersistentStoreConfig config) throws StoreException {
+switch(config.getMode()){
+case BLOB_PERSISTENT:
+  return new LocalPersistentStore<>(fs, blobRoot, config);
--- End diff --

LocalPersistentStore in ZookeeperPersistentStoreProvider?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread sudheeshkatkam
Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52960456
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java
 ---
@@ -217,11 +223,14 @@ private QueryProfile getQueryProfile(String queryId) 
throws IOException {
 }
 
 // then check blob store
-PStore profiles = 
provider().getStore(QueryManager.QUERY_PROFILE);
-QueryProfile queryProfile = profiles.get(queryId);
-if (queryProfile != null) {
-  checkOrThrowProfileViewAuthorization(queryProfile);
-  return queryProfile;
+try (final PersistentStore profiles = 
getProvider().getOrCreateStore(QueryManager.QUERY_PROFILE)) {
--- End diff --

same here


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread sudheeshkatkam
Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52960464
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java
 ---
@@ -267,9 +276,8 @@ public String cancelQuery(@PathParam("queryid") String 
queryId) throws IOExcepti
 }
 
 // then check remote running
-try{
-  PStore runningQueries = 
provider().getStore(QueryManager.RUNNING_QUERY_INFO);
-  QueryInfo info = runningQueries.get(queryId);
+try (final TransientStore running = 
getCoordinator().getOrCreateTransientStore(QueryManager.RUNNING_QUERY_INFO)) {
--- End diff --

same here


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread sudheeshkatkam
Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52960453
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java
 ---
@@ -200,9 +207,8 @@ private QueryProfile getQueryProfile(String queryId) 
throws IOException {
 }
 
 // then check remote running
-try{
-  PStore runningQueries = 
provider().getStore(QueryManager.RUNNING_QUERY_INFO);
-  QueryInfo info = runningQueries.get(queryId);
+try (final TransientStore running = 
getCoordinator().getOrCreateTransientStore(QueryManager.RUNNING_QUERY_INFO)) {
--- End diff --

same here


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread sudheeshkatkam
Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52960439
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java
 ---
@@ -146,38 +154,37 @@ public QProfiles(List runningQueries, 
List finishedQue
   @Path("/profiles.json")
   @Produces(MediaType.APPLICATION_JSON)
   public QProfiles getProfilesJSON() {
-PStore completed = null;
-PStore running = null;
-try {
-  completed = provider().getStore(QueryManager.QUERY_PROFILE);
-  running = provider().getStore(QueryManager.RUNNING_QUERY_INFO);
-} catch (IOException e) {
-  logger.debug("Failed to get profiles from persistent or ephemeral 
store.");
-  return new QProfiles(new ArrayList(), new 
ArrayList());
-}
-
-List runningQueries = Lists.newArrayList();
-
-for (Map.Entry entry : running) {
-  QueryInfo profile = entry.getValue();
-  if (principal.canManageProfileOf(profile.getUser())) {
-runningQueries.add(new ProfileInfo(entry.getKey(), 
profile.getStart(), profile.getForeman().getAddress(),
-profile.getQuery(), profile.getState().name(), 
profile.getUser()));
+try (
+final PersistentStore completed = 
getProvider().getOrCreateStore(QueryManager.QUERY_PROFILE);
--- End diff --

This should not use try-with-resources statement, which will close the 
resources at the end of the block.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread hnfgns
Github user hnfgns commented on the pull request:

https://github.com/apache/drill/pull/374#issuecomment-184463952
  
Please let me know of further comments and remember to +1 if you are comfy 
with the patch. If nothing severe is seen, let's take all comments on follow-up 
PRs. This patch is long waiting I'd like to check this in. Thanks for all 
feedback.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread hnfgns
Github user hnfgns commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52958072
  
--- Diff: exec/java-exec/src/main/resources/drill-module.conf ---
@@ -120,7 +120,7 @@ drill.exec: {
 affinity.factor: 1.2
   },
   sys.store.provider: {
-class: "org.apache.drill.exec.store.sys.zk.ZkPStoreProvider",
+class: 
"org.apache.drill.exec.store.sys.store.provider.ZookeeperPersistentStoreProvider",
--- End diff --

All included.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread sudheeshkatkam
Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52957626
  
--- Diff: exec/java-exec/src/main/resources/drill-module.conf ---
@@ -120,7 +120,7 @@ drill.exec: {
 affinity.factor: 1.2
   },
   sys.store.provider: {
-class: "org.apache.drill.exec.store.sys.zk.ZkPStoreProvider",
+class: 
"org.apache.drill.exec.store.sys.store.provider.ZookeeperPersistentStoreProvider",
--- End diff --

What about other *PStoreProvider? This will still impact those users.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread hnfgns
Github user hnfgns commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52957306
  
--- Diff: 
common/src/main/java/org/apache/drill/common/collections/ImmutableEntry.java ---
@@ -0,0 +1,62 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.common.collections;
+
+import java.util.Map;
+
+import com.google.common.base.Objects;
+import com.google.common.base.Preconditions;
+
+public class ImmutableEntry implements Map.Entry  {
+  private final K key;
+  private final V value;
+
+  public ImmutableEntry(final K key, final V value) {
+this.key = Preconditions.checkNotNull(key, "key is required");
+this.value = Preconditions.checkNotNull(value, "value is required");
+  }
+
+  @Override
+  public K getKey() {
+return key;
+  }
+
+  @Override
+  public V getValue() {
+return value;
+  }
+
+  @Override
+  public V setValue(final V value) {
+throw new UnsupportedOperationException("entry is immutable");
+  }
+
+  @Override
+  public boolean equals(final Object other) {
+if (other instanceof ImmutableEntry && other.getClass() == getClass()) 
{
+  final ImmutableEntry entry = (ImmutableEntry)other;
+  return Objects.equal(key, entry.key) && Objects.equal(value, 
entry.value);
--- End diff --

Cool. I think we should consider replacing all uses on a separate patch 
once this is deprecated.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread hnfgns
Github user hnfgns commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52957167
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKClusterCoordinator.java
 ---
@@ -206,6 +209,16 @@ public DistributedSemaphore getSemaphore(String name, 
int maximumLeases) {
 return new ZkDistributedSemaphore(curator, "/semaphore/" + name, 
maximumLeases);
   }
 
+  @Override
+  public  TransientStore getOrCreateTransientStore(final 
TransientStoreConfig config) {
+final ZkEphemeralStore store = 
(ZkEphemeralStore)factory.getOrCreateStore(config);
+try {
+  store.start();
--- End diff --

That logic now is in factory which calls start once.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread hnfgns
Github user hnfgns commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52956885
  
--- Diff: exec/java-exec/src/main/resources/drill-module.conf ---
@@ -120,7 +120,7 @@ drill.exec: {
 affinity.factor: 1.2
   },
   sys.store.provider: {
-class: "org.apache.drill.exec.store.sys.zk.ZkPStoreProvider",
+class: 
"org.apache.drill.exec.store.sys.store.provider.ZookeeperPersistentStoreProvider",
--- End diff --

The new patch includes a deprecated ZkPStoreProvider that is to be removed 
in 1.7. Now old configuration should work as is. We should also state this out 
in 1.6 release notes. Let me know if you guys have any other concerns as for 
compatibility.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread sudheeshkatkam
Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52956715
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKClusterCoordinator.java
 ---
@@ -206,6 +209,16 @@ public DistributedSemaphore getSemaphore(String name, 
int maximumLeases) {
 return new ZkDistributedSemaphore(curator, "/semaphore/" + name, 
maximumLeases);
   }
 
+  @Override
+  public  TransientStore getOrCreateTransientStore(final 
TransientStoreConfig config) {
+final ZkEphemeralStore store = 
(ZkEphemeralStore)factory.getOrCreateStore(config);
+try {
+  store.start();
--- End diff --

Accessing profiles will trigger starting the store again, which will cause 
an "already started" exception.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread sudheeshkatkam
Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52956166
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/coord/local/LocalClusterCoordinator.java
 ---
@@ -125,6 +127,11 @@ public DistributedSemaphore getSemaphore(final String 
name, final int maximumLea
 return semaphores.get(name);
   }
 
+  @Override
+  public  TransientStore getOrCreateTransientStore(final 
TransientStoreConfig config) {
+return new MapBackedStore<>(config);
--- End diff --

This would otherwise break embedded mode _running_ query profiles.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread hnfgns
Github user hnfgns commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52955476
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/coord/local/LocalClusterCoordinator.java
 ---
@@ -125,6 +127,11 @@ public DistributedSemaphore getSemaphore(final String 
name, final int maximumLea
 return semaphores.get(name);
   }
 
+  @Override
+  public  TransientStore getOrCreateTransientStore(final 
TransientStoreConfig config) {
+return new MapBackedStore<>(config);
--- End diff --

I think it's quite cheap here to do so here. Added caching too.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread hnfgns
Github user hnfgns commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52955131
  
--- Diff: 
contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/config/HBasePersistentStoreProvider.java
 ---
@@ -41,10 +39,8 @@
 import org.apache.hadoop.hbase.client.HTableInterface;
 import org.apache.hadoop.hbase.util.Bytes;
 
-import com.google.common.annotations.VisibleForTesting;
-
-public class HBasePStoreProvider implements PStoreProvider {
-  static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(HBasePStoreProvider.class);
+public class HBasePersistentStoreProvider extends 
BasePersistentStoreProvider {
+  static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(HBasePersistentStoreProvider.class);
--- End diff --

not relevant to my changes but did my best to privatize them all ;)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread hnfgns
Github user hnfgns commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52955096
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZookeeperClient.java
 ---
@@ -0,0 +1,238 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.coord.zk;
+
+import java.util.Iterator;
+import java.util.Map;
+
+import javax.annotation.Nullable;
+
+import com.google.common.base.Function;
+import com.google.common.base.Preconditions;
+import com.google.common.base.Strings;
+import com.google.common.collect.Iterables;
+import org.apache.curator.framework.CuratorFramework;
+import org.apache.curator.framework.recipes.cache.ChildData;
+import org.apache.curator.framework.recipes.cache.PathChildrenCache;
+import org.apache.drill.common.collections.ImmutableEntry;
+import org.apache.drill.common.exceptions.DrillRuntimeException;
+import org.apache.zookeeper.CreateMode;
+
+/**
+ * A namespace aware Zookeper client.
--- End diff --

seriously :+1: 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread sudheeshkatkam
Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52954392
  
--- Diff: 
contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/config/HBasePersistentStoreProvider.java
 ---
@@ -41,10 +39,8 @@
 import org.apache.hadoop.hbase.client.HTableInterface;
 import org.apache.hadoop.hbase.util.Bytes;
 
-import com.google.common.annotations.VisibleForTesting;
-
-public class HBasePStoreProvider implements PStoreProvider {
-  static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(HBasePStoreProvider.class);
+public class HBasePersistentStoreProvider extends 
BasePersistentStoreProvider {
+  static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(HBasePersistentStoreProvider.class);
--- End diff --

_private_ for all loggers of *Provider and *Store would be nice :)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread hnfgns
Github user hnfgns commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52954382
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryManager.java
 ---
@@ -237,11 +237,17 @@ void unpauseExecutingFragments(final DrillbitContext 
drillbitContext) {
 }
   }
 
+  @Override
+  public void close() throws Exception {
+profileStore.close();
--- End diff --

done.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread hnfgns
Github user hnfgns commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52954225
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/PersistentStore.java
 ---
@@ -0,0 +1,77 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.store.sys;
+
+import java.util.Iterator;
+import java.util.Map;
+
+/**
+ * An abstraction used to store and retrieve instances of given value type.
+ *
+ * @param   value type
+ */
+public interface PersistentStore extends AutoCloseable {
--- End diff --

Mainly because of pagination and being more explicit both at interface and 
pagination control level.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread hnfgns
Github user hnfgns commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52954240
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/PersistentStoreProvider.java
 ---
@@ -17,17 +17,26 @@
  */
 package org.apache.drill.exec.store.sys;
 
-import java.util.Map;
-
+import org.apache.drill.exec.exception.StoreException;
 
 /**
- * Interface for reading and writing values to a persistent storage 
provider.  Iterators are guaranteed to be returned in key order.
- * @param 
+ * A factory used to create {@link PersistentStore store} instances.
+ *
  */
-public interface PStore extends Iterable> {
-  public V get(String key);
-  public void put(String key, V value);
-  public boolean putIfAbsent(String key, V value);
-  public void delete(String key);
-  public void close();
+public interface PersistentStoreProvider extends AutoCloseable {
+  /**
+   * Gets or creates a {@link PersistentStore persistent store} for the 
given configuration.
+   *
+   * Note that implementors have liberty to cache previous {@link 
PersistentStore store} instances.
+   *
+   * @param config  store configuration
+   * @param   store value type
+   */
+   PersistentStore getStore(PersistentStoreConfig config) throws 
StoreException;
--- End diff --

done.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread sudheeshkatkam
Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52954150
  
--- Diff: 
common/src/main/java/org/apache/drill/common/collections/ImmutableEntry.java ---
@@ -0,0 +1,62 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.common.collections;
+
+import java.util.Map;
+
+import com.google.common.base.Objects;
+import com.google.common.base.Preconditions;
+
+public class ImmutableEntry implements Map.Entry  {
+  private final K key;
+  private final V value;
+
+  public ImmutableEntry(final K key, final V value) {
+this.key = Preconditions.checkNotNull(key, "key is required");
+this.value = Preconditions.checkNotNull(value, "value is required");
+  }
+
+  @Override
+  public K getKey() {
+return key;
+  }
+
+  @Override
+  public V getValue() {
+return value;
+  }
+
+  @Override
+  public V setValue(final V value) {
+throw new UnsupportedOperationException("entry is immutable");
+  }
+
+  @Override
+  public boolean equals(final Object other) {
+if (other instanceof ImmutableEntry && other.getClass() == getClass()) 
{
+  final ImmutableEntry entry = (ImmutableEntry)other;
+  return Objects.equal(key, entry.key) && Objects.equal(value, 
entry.value);
--- End diff --

There is no deprecated annotation, but the doc. for the guava 18 that we 
use has this note as well.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread hnfgns
Github user hnfgns commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52954122
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java
 ---
@@ -146,38 +155,37 @@ public QProfiles(List runningQueries, 
List finishedQue
   @Path("/profiles.json")
   @Produces(MediaType.APPLICATION_JSON)
   public QProfiles getProfilesJSON() {
-PStore completed = null;
-PStore running = null;
-try {
-  completed = provider().getStore(QueryManager.QUERY_PROFILE);
-  running = provider().getStore(QueryManager.RUNNING_QUERY_INFO);
-} catch (IOException e) {
-  logger.debug("Failed to get profiles from persistent or ephemeral 
store.");
-  return new QProfiles(new ArrayList(), new 
ArrayList());
-}
-
-List runningQueries = Lists.newArrayList();
-
-for (Map.Entry entry : running) {
-  QueryInfo profile = entry.getValue();
-  if (principal.canManageProfileOf(profile.getUser())) {
-runningQueries.add(new ProfileInfo(entry.getKey(), 
profile.getStart(), profile.getForeman().getAddress(),
-profile.getQuery(), profile.getState().name(), 
profile.getUser()));
+try (
+final PersistentStore completed = 
getProvider().getStore(QueryManager.QUERY_PROFILE);
+final TransientStore running = 
getCoordinator().newTransientStore(QueryManager.RUNNING_QUERY_INFO);
--- End diff --

nope instances are now cached. I will rename the method to reflect that 
change.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread hnfgns
Github user hnfgns commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52953970
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/coord/store/TransientStoreListener.java
 ---
@@ -0,0 +1,22 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.coord.store;
+
+public interface TransientStoreListener {
--- End diff --

done.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread hnfgns
Github user hnfgns commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52953875
  
--- Diff: 
contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/config/HBasePersistentStore.java
 ---
@@ -148,13 +154,13 @@ private void delete(byte[] row) {
 private Result current = null;
 private Result last = null;
 private boolean done = false;
-private int rowsRead = 0;
 
-Iter() {
+Iter(int take) {
   try {
 Scan scan = new Scan(tableNameStartKey, tableNameStopKey);
 scan.addColumn(FAMILY, QUALIFIER);
-scan.setCaching(config.getMaxIteratorSize() > 100 ? 100 : 
config.getMaxIteratorSize());
+scan.setCaching(Math.min(take, 100));
--- End diff --

nope. this patch introduces a way to paginate results via #getRange. this 
option is no longer needed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread jinfengni
Github user jinfengni commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52953882
  
--- Diff: exec/java-exec/src/main/resources/drill-module.conf ---
@@ -120,7 +120,7 @@ drill.exec: {
 affinity.factor: 1.2
   },
   sys.store.provider: {
-class: "org.apache.drill.exec.store.sys.zk.ZkPStoreProvider",
+class: 
"org.apache.drill.exec.store.sys.store.provider.ZookeeperPersistentStoreProvider",
--- End diff --

I second on Sudheesh's concern. Unless there is a strong reason that we 
have to make a breaking change such that "all references to ZkPStoreProvider 
will fail following this patch", I feel we need to think twice before we make 
such change.  Is this the only way that we can go? Have we assessed the impact 
to Drill's current users (After all, Drill is post 1.0 release)? Do we have a 
well-documented plan to help them to migrate, if the backward compatibility is 
broken in new release?

 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread hnfgns
Github user hnfgns commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52953797
  
--- Diff: 
common/src/main/java/org/apache/drill/common/collections/ImmutableEntry.java ---
@@ -0,0 +1,62 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.common.collections;
+
+import java.util.Map;
+
+import com.google.common.base.Objects;
+import com.google.common.base.Preconditions;
+
+public class ImmutableEntry implements Map.Entry  {
+  private final K key;
+  private final V value;
+
+  public ImmutableEntry(final K key, final V value) {
+this.key = Preconditions.checkNotNull(key, "key is required");
+this.value = Preconditions.checkNotNull(value, "value is required");
+  }
+
+  @Override
+  public K getKey() {
+return key;
+  }
+
+  @Override
+  public V getValue() {
+return value;
+  }
+
+  @Override
+  public V setValue(final V value) {
+throw new UnsupportedOperationException("entry is immutable");
+  }
+
+  @Override
+  public boolean equals(final Object other) {
+if (other instanceof ImmutableEntry && other.getClass() == getClass()) 
{
+  final ImmutableEntry entry = (ImmutableEntry)other;
+  return Objects.equal(key, entry.key) && Objects.equal(value, 
entry.value);
--- End diff --

That sure makes sense. You are probably looking at the cutting edge. We are 
on guava-18 that does not deprecate these. Thanks for the reminder. Going 
forward we should keep that in mind.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread sudheeshkatkam
Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52953622
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZookeeperClient.java
 ---
@@ -0,0 +1,238 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.coord.zk;
+
+import java.util.Iterator;
+import java.util.Map;
+
+import javax.annotation.Nullable;
+
+import com.google.common.base.Function;
+import com.google.common.base.Preconditions;
+import com.google.common.base.Strings;
+import com.google.common.collect.Iterables;
+import org.apache.curator.framework.CuratorFramework;
+import org.apache.curator.framework.recipes.cache.ChildData;
+import org.apache.curator.framework.recipes.cache.PathChildrenCache;
+import org.apache.drill.common.collections.ImmutableEntry;
+import org.apache.drill.common.exceptions.DrillRuntimeException;
+import org.apache.zookeeper.CreateMode;
+
+/**
+ * A namespace aware Zookeper client.
--- End diff --

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: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread sudheeshkatkam
Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52953108
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/PersistentStoreProvider.java
 ---
@@ -17,17 +17,26 @@
  */
 package org.apache.drill.exec.store.sys;
 
-import java.util.Map;
-
+import org.apache.drill.exec.exception.StoreException;
 
 /**
- * Interface for reading and writing values to a persistent storage 
provider.  Iterators are guaranteed to be returned in key order.
- * @param 
+ * A factory used to create {@link PersistentStore store} instances.
+ *
  */
-public interface PStore extends Iterable> {
-  public V get(String key);
-  public void put(String key, V value);
-  public boolean putIfAbsent(String key, V value);
-  public void delete(String key);
-  public void close();
+public interface PersistentStoreProvider extends AutoCloseable {
+  /**
+   * Gets or creates a {@link PersistentStore persistent store} for the 
given configuration.
+   *
+   * Note that implementors have liberty to cache previous {@link 
PersistentStore store} instances.
+   *
+   * @param config  store configuration
+   * @param   store value type
+   */
+   PersistentStore getStore(PersistentStoreConfig config) throws 
StoreException;
--- End diff --

getOrCreateStore(...)?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread sudheeshkatkam
Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52953137
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryManager.java
 ---
@@ -237,11 +237,17 @@ void unpauseExecutingFragments(final DrillbitContext 
drillbitContext) {
 }
   }
 
+  @Override
+  public void close() throws Exception {
+profileStore.close();
--- End diff --

AutoCloseables.close(...)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread sudheeshkatkam
Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52953084
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java
 ---
@@ -267,9 +277,8 @@ public String cancelQuery(@PathParam("queryid") String 
queryId) throws IOExcepti
 }
 
 // then check remote running
-try{
-  PStore runningQueries = 
provider().getStore(QueryManager.RUNNING_QUERY_INFO);
-  QueryInfo info = runningQueries.get(queryId);
+try (final TransientStore running = 
getCoordinator().newTransientStore(QueryManager.RUNNING_QUERY_INFO)) {
--- End diff --

same here


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread sudheeshkatkam
Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52953078
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java
 ---
@@ -146,38 +155,37 @@ public QProfiles(List runningQueries, 
List finishedQue
   @Path("/profiles.json")
   @Produces(MediaType.APPLICATION_JSON)
   public QProfiles getProfilesJSON() {
-PStore completed = null;
-PStore running = null;
-try {
-  completed = provider().getStore(QueryManager.QUERY_PROFILE);
-  running = provider().getStore(QueryManager.RUNNING_QUERY_INFO);
-} catch (IOException e) {
-  logger.debug("Failed to get profiles from persistent or ephemeral 
store.");
-  return new QProfiles(new ArrayList(), new 
ArrayList());
-}
-
-List runningQueries = Lists.newArrayList();
-
-for (Map.Entry entry : running) {
-  QueryInfo profile = entry.getValue();
-  if (principal.canManageProfileOf(profile.getUser())) {
-runningQueries.add(new ProfileInfo(entry.getKey(), 
profile.getStart(), profile.getForeman().getAddress(),
-profile.getQuery(), profile.getState().name(), 
profile.getUser()));
+try (
+final PersistentStore completed = 
getProvider().getStore(QueryManager.QUERY_PROFILE);
+final TransientStore running = 
getCoordinator().newTransientStore(QueryManager.RUNNING_QUERY_INFO);
--- End diff --

This isn't creating a new store, right?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread sudheeshkatkam
Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52953099
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/PersistentStore.java
 ---
@@ -0,0 +1,77 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.store.sys;
+
+import java.util.Iterator;
+import java.util.Map;
+
+/**
+ * An abstraction used to store and retrieve instances of given value type.
+ *
+ * @param   value type
+ */
+public interface PersistentStore extends AutoCloseable {
--- End diff --

Any reason you chose to remove extending _java.lang.Iterable_? Most usages 
now look like Lists.newArrayList(store.getAll())


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread sudheeshkatkam
Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52953080
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java
 ---
@@ -200,9 +208,8 @@ private QueryProfile getQueryProfile(String queryId) 
throws IOException {
 }
 
 // then check remote running
-try{
-  PStore runningQueries = 
provider().getStore(QueryManager.RUNNING_QUERY_INFO);
-  QueryInfo info = runningQueries.get(queryId);
+try (final TransientStore running = 
getCoordinator().newTransientStore(QueryManager.RUNNING_QUERY_INFO)) {
--- End diff --

same here


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread sudheeshkatkam
Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52953064
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZkEphemeralStore.java
 ---
@@ -0,0 +1,145 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.coord.zk;
+
+import java.io.IOException;
+import java.util.Iterator;
+import java.util.Map;
+
+import javax.annotation.Nullable;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Function;
+import com.google.common.collect.Iterators;
+import com.google.common.collect.Lists;
+import org.apache.curator.framework.CuratorFramework;
+import 
org.apache.curator.framework.recipes.cache.PathChildrenCacheListener;
+import org.apache.drill.common.collections.ImmutableEntry;
+import org.apache.drill.common.exceptions.DrillRuntimeException;
+import org.apache.drill.exec.coord.store.BaseTransientStore;
+import org.apache.drill.exec.coord.store.TransientStoreConfig;
+import org.apache.drill.exec.coord.store.TransientStoreEvent;
+import org.apache.drill.exec.serialization.InstanceSerializer;
+import org.apache.zookeeper.CreateMode;
+
+public class ZkEphemeralStore extends BaseTransientStore {
+
+  @VisibleForTesting
+  protected final PathChildrenCacheListener dispatcher = new 
EventDispatcher<>(this);
+  private final ZookeeperClient client;
+
+  public ZkEphemeralStore(final TransientStoreConfig config, final 
CuratorFramework curator) {
+super(config);
+this.client = new ZookeeperClient(curator, PathUtils.join("/", 
config.getName()), CreateMode.EPHEMERAL);
+  }
+
+  public void start() throws Exception {
+getClient().getCache().getListenable().addListener(dispatcher);
+getClient().start();
+  }
+
+  protected ZookeeperClient getClient() {
+return client;
+  }
+
+  @Override
+  public V get(final String key) {
+final byte[] bytes = getClient().get(key);
+if (bytes == null) {
+  return null;
+}
+try {
+  return config.getSerializer().deserialize(bytes);
+} catch (final IOException e) {
+  throw new DrillRuntimeException(String.format("unable to deserialize 
value at %s", key), e);
+}
+  }
+
+  @Override
+  public V put(final String key, final V value) {
+final InstanceSerializer serializer = config.getSerializer();
+try {
+  final byte[] old = getClient().get(key);
+  final byte[] bytes = serializer.serialize(value);
+  getClient().put(key, bytes);
+  if (old == null) {
+return null;
+  }
+  return serializer.deserialize(old);
+} catch (final IOException e) {
+  throw new DrillRuntimeException(String.format("unable to 
de/serialize value of type %s", value.getClass()), e);
+}
+  }
+
+  @Override
+  public V putIfAbsent(final String key, final V value) {
+final V old = get(key);
+if (old == null) {
+  try {
+final byte[] bytes = config.getSerializer().serialize(value);
+getClient().put(key, bytes);
+  } catch (final IOException e) {
+throw new DrillRuntimeException(String.format("unable to serialize 
value of type %s", value.getClass()), e);
+  }
+}
+return old;
+  }
+
+  @Override
+  public V remove(final String key) {
+final V existing = get(key);
+if (existing != null) {
+  getClient().delete(key);
+}
+return existing;
+  }
+
+  @Override
+  public Iterator> entries() {
+return Iterators.transform(getClient().entries(), new 
Function, Map.Entry>() {
+  @Nullable
+  @Override
+  public Map.Entry apply(@Nullable Map.Entry input) {
+try {
 

[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread sudheeshkatkam
Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52953057
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/coord/store/TransientStoreListener.java
 ---
@@ -0,0 +1,22 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.coord.store;
+
+public interface TransientStoreListener {
--- End diff --

javadoc


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread sudheeshkatkam
Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52953044
  
--- Diff: 
contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/config/HBasePersistentStore.java
 ---
@@ -148,13 +154,13 @@ private void delete(byte[] row) {
 private Result current = null;
 private Result last = null;
 private boolean done = false;
-private int rowsRead = 0;
 
-Iter() {
+Iter(int take) {
   try {
 Scan scan = new Scan(tableNameStartKey, tableNameStopKey);
 scan.addColumn(FAMILY, QUALIFIER);
-scan.setCaching(config.getMaxIteratorSize() > 100 ? 100 : 
config.getMaxIteratorSize());
+scan.setCaching(Math.min(take, 100));
--- End diff --

Is _config.getMaxIteratorSize()_ not required?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread sudheeshkatkam
Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52953040
  
--- Diff: 
common/src/main/java/org/apache/drill/common/collections/ImmutableEntry.java ---
@@ -0,0 +1,62 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.common.collections;
+
+import java.util.Map;
+
+import com.google.common.base.Objects;
+import com.google.common.base.Preconditions;
+
+public class ImmutableEntry implements Map.Entry  {
+  private final K key;
+  private final V value;
+
+  public ImmutableEntry(final K key, final V value) {
+this.key = Preconditions.checkNotNull(key, "key is required");
+this.value = Preconditions.checkNotNull(value, "value is required");
+  }
+
+  @Override
+  public K getKey() {
+return key;
+  }
+
+  @Override
+  public V getValue() {
+return value;
+  }
+
+  @Override
+  public V setValue(final V value) {
+throw new UnsupportedOperationException("entry is immutable");
+  }
+
+  @Override
+  public boolean equals(final Object other) {
+if (other instanceof ImmutableEntry && other.getClass() == getClass()) 
{
+  final ImmutableEntry entry = (ImmutableEntry)other;
+  return Objects.equal(key, entry.key) && Objects.equal(value, 
entry.value);
--- End diff --

google.common.base.Objects#equal and #hashCode [should be treated as 
deprecated](https://github.com/google/guava/blob/master/guava/src/com/google/common/base/Objects.java#L55).


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


[jira] [Created] (DRILL-4386) Unify and move serialization logic to common module or package

2016-02-15 Thread Hanifi Gunes (JIRA)
Hanifi Gunes created DRILL-4386:
---

 Summary: Unify and move serialization logic to common module or 
package
 Key: DRILL-4386
 URL: https://issues.apache.org/jira/browse/DRILL-4386
 Project: Apache Drill
  Issue Type: Improvement
  Components: Execution - Codegen
Affects Versions: 1.5.0
Reporter: Hanifi Gunes
Priority: Minor


In many places around Drill we rely on custom SerDes. However, there seems some 
redundancy. For instance, Transient/PersistentStore(introduced by DRILL-4275) 
relies on InstanceSerializer whereas Controller uses CustomSerDe interface. 
Effectively these two are the same. We need to unify use cases, possibly moving 
SerDe logic to common module or some level up in the exec module. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread hnfgns
Github user hnfgns commented on the pull request:

https://github.com/apache/drill/pull/374#issuecomment-184436146
  
I have retracted unrelated mongo changes. Thanks for the feedback.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread hnfgns
Github user hnfgns commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52951701
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/serialization/InstanceSerializer.java
 ---
@@ -6,20 +6,20 @@
  * to you under the Apache License, Version 2.0 (the
  * "License"); you may not use this file except in compliance
  * with the License.  You may obtain a copy of the License at
- *
+ * 
  * http://www.apache.org/licenses/LICENSE-2.0
- *
+ * 
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.apache.drill.exec.store.sys.serialize;
+package org.apache.drill.exec.serialization;
 
 import java.io.IOException;
 
-public interface PClassSerializer {
-  public byte[] serialize(X val) throws IOException;
-  public X deserialize(byte[] bytes) throws IOException;
+public interface InstanceSerializer {
+  byte[] serialize(T instance) throws IOException;
--- End diff --

Sounds good. I can do that on a follow-up issue without messing with RPC on 
this patch.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread hnfgns
Github user hnfgns commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52951541
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/coord/store/TransientStoreListener.java
 ---
@@ -0,0 +1,22 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.coord.store;
+
+public interface TransientStoreListener {
+  void onChange(TransientStoreEvent event);
--- End diff --

I typically prefer this way as it allows extending capabilities(esp event 
types) via polymorphism without requiring to alter the interface. I am fine 
with either way though.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread hnfgns
Github user hnfgns commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52951273
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/PathUtils.java ---
@@ -0,0 +1,59 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.coord.zk;
+
+import com.google.common.base.Preconditions;
+import org.apache.parquet.Strings;
+
+public final class PathUtils {
--- End diff --

done.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread hnfgns
Github user hnfgns commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52950999
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/coord/ClusterCoordinator.java
 ---
@@ -60,16 +62,23 @@
   public abstract DistributedSemaphore getSemaphore(String name, int 
maximumLeases);
 
   /**
+   * Returns a new {@link TransientStore store} instance with the given 
{@link TransientStoreConfig configuration}.
+   * @param config  store configuration
+   * @param   value type for this store
+   */
+  public abstract  TransientStore 
newTransientStore(TransientStoreConfig config);
+
+  /**
* Actions to take when there are a set of new de-active drillbits.
* @param unregisteredBits
*/
-  public void drillbitUnregistered(Set unregisteredBits) 
{
+  protected void drillbitUnregistered(Set 
unregisteredBits) {
--- End diff --

Wrong use of this method outside of subclasses could easily mess the system 
up. To my understanding, this method should only be visible to implementor who 
is the ultimate authority to fire listeners on topology changes, so protected 
seems to make more sense here.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread hnfgns
Github user hnfgns commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52950554
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/serialization/JacksonSerializer.java
 ---
@@ -0,0 +1,59 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.serialization;
+
+import java.io.IOException;
+import java.util.Objects;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.ObjectReader;
+import com.fasterxml.jackson.databind.ObjectWriter;
+
+public class JacksonSerializer implements InstanceSerializer {
+  private final ObjectReader reader;
+  private final ObjectWriter writer;
+
+  public JacksonSerializer(final ObjectMapper mapper, final Class 
klazz) {
+this.reader = mapper.reader(klazz);
+this.writer = mapper.writer();
+  }
+
+  @Override
+  public T deserialize(final byte[] raw) throws IOException {
+return reader.readValue(raw);
+  }
+
+  @Override
+  public byte[] serialize(final T instance) throws IOException {
+return writer.writeValueAsBytes(instance);
+  }
+
+  @Override
+  public boolean equals(Object obj) {
+if (obj instanceof JacksonSerializer && 
obj.getClass().equals(getClass())) {
+  final JacksonSerializer other = (JacksonSerializer)obj;
+  return Objects.equals(reader, other.reader) && 
Objects.equals(writer, other.writer);
+}
+return false;
+  }
+
+  @Override
+  public int hashCode() {
+return super.hashCode();
--- End diff --

nice catch. thanks. done.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread hnfgns
Github user hnfgns commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52950533
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKClusterCoordinator.java
 ---
@@ -206,6 +205,16 @@ public DistributedSemaphore getSemaphore(String name, 
int maximumLeases) {
 return new ZkDistributedSemaphore(curator, "/semaphore/" + name, 
maximumLeases);
   }
 
+  @Override
+  public  TransientStore newTransientStore(final 
TransientStoreConfig config) {
+final ZkEphemeralStore store = new ZkEphemeralStore<>(config, 
curator);
--- End diff --

done.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request: DRILL-4275: create TransientStore for short-li...

2016-02-15 Thread hnfgns
Github user hnfgns commented on a diff in the pull request:

https://github.com/apache/drill/pull/374#discussion_r52950537
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/store/ZookeeperPersistentStore.java
 ---
@@ -0,0 +1,136 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.store.sys.store;
+
+import java.io.IOException;
+import java.util.Iterator;
+import java.util.Map;
+
+import javax.annotation.Nullable;
+
+import com.google.common.base.Function;
+import com.google.common.base.Preconditions;
+import com.google.common.collect.Iterators;
+import org.apache.curator.framework.CuratorFramework;
+import org.apache.drill.common.collections.ImmutableEntry;
+import org.apache.drill.common.exceptions.DrillRuntimeException;
+import org.apache.drill.exec.coord.zk.PathUtils;
+import org.apache.drill.exec.coord.zk.ZookeeperClient;
+import org.apache.drill.exec.exception.StoreException;
+import org.apache.drill.exec.serialization.InstanceSerializer;
+import org.apache.drill.exec.store.sys.BasePersistentStore;
+import org.apache.drill.exec.store.sys.PersistentStoreConfig;
+import org.apache.drill.exec.store.sys.PersistentStoreMode;
+import org.apache.zookeeper.CreateMode;
+
+/**
+ * This is the abstract class that is shared by ZkPStore (Persistent 
store) and ZkEStore (Ephemeral Store)
--- End diff --

done


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[jira] [Resolved] (DRILL-3547) IndexOutOfBoundsException on directory with ~20 subdirectories

2016-02-15 Thread Arina Ielchiieva (JIRA)

 [ 
https://issues.apache.org/jira/browse/DRILL-3547?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Arina Ielchiieva resolved DRILL-3547.
-
Resolution: Cannot Reproduce

> IndexOutOfBoundsException on directory with ~20 subdirectories
> --
>
> Key: DRILL-3547
> URL: https://issues.apache.org/jira/browse/DRILL-3547
> Project: Apache Drill
>  Issue Type: Bug
>  Components: Functions - Drill
>Affects Versions: 1.1.0
> Environment: RHEL 7
>Reporter: Philip Deegan
>Assignee: Arina Ielchiieva
> Fix For: 1.6.0
>
>
> Tested on 1.1 with commit id:
> {noformat}
> 0: jdbc:drill:zk=local> select commit_id from sys.version;
> +---+
> | commit_id |
> +---+
> | e3fc7e97bfe712dc09d43a8a055a5135c96b7344  |
> +---+
> {noformat}
> Directory has child directories "a" to "u", each contain json files.
> Running the query on each subdirectory indivudually does not cause an error.
> {noformat}
> java.lang.RuntimeException: java.sql.SQLException: SYSTEM ERROR: 
> IndexOutOfBoundsException: index: 0, length: 1 (expected: range(0, 0))
> Fragment 1:2
> [Error Id: 69a0879f-f718-4930-ae6f-c526de05528c on 
> ip-172-31-29-60.eu-central-1.compute.internal:31010]
>   at sqlline.IncrementalRows.hasNext(IncrementalRows.java:73)
>   at 
> sqlline.TableOutputFormat$ResizingRowsProvider.next(TableOutputFormat.java:87)
>   at sqlline.TableOutputFormat.print(TableOutputFormat.java:118)
>   at sqlline.SqlLine.print(SqlLine.java:1583)
>   at sqlline.Commands.execute(Commands.java:852)
>   at sqlline.Commands.sql(Commands.java:751)
>   at sqlline.SqlLine.dispatch(SqlLine.java:738)
>   at sqlline.SqlLine.begin(SqlLine.java:612)
>   at sqlline.SqlLine.start(SqlLine.java:366)
>   at sqlline.SqlLine.main(SqlLine.java:259)
> {noformat}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)