[GitHub] [hbase] saintstack commented on a change in pull request #287: HBASE-21512 Reimplement sync client based on async client

2019-06-08 Thread GitBox
saintstack commented on a change in pull request #287: HBASE-21512 Reimplement 
sync client based on async client
URL: https://github.com/apache/hbase/pull/287#discussion_r291811462
 
 

 ##
 File path: 
hbase-client/src/main/java/org/apache/hadoop/hbase/client/ShortCircuitMasterConnection.java
 ##
 @@ -1,716 +0,0 @@
-/**
 
 Review comment:
   These TODOs are written up somewhere? In release notes on the issue maybe?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] saintstack commented on a change in pull request #287: HBASE-21512 Reimplement sync client based on async client

2019-06-08 Thread GitBox
saintstack commented on a change in pull request #287: HBASE-21512 Reimplement 
sync client based on async client
URL: https://github.com/apache/hbase/pull/287#discussion_r291811477
 
 

 ##
 File path: 
hbase-client/src/test/java/org/apache/hadoop/hbase/client/SimpleRegistry.java
 ##
 @@ -0,0 +1,83 @@
+/**
+ * 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.hadoop.hbase.client;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.DoNotRetryIOException;
+import org.apache.hadoop.hbase.HConstants;
+import org.apache.hadoop.hbase.HRegionLocation;
+import org.apache.hadoop.hbase.RegionLocations;
+import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.util.FutureUtils;
+
+/**
+ * Simple cluster registry inserted in place of our usual zookeeper based one.
 
 Review comment:
   Great. If a new patch, add comment that for testing (I don't think it there 
now..)


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] saintstack commented on a change in pull request #287: HBASE-21512 Reimplement sync client based on async client

2019-06-08 Thread GitBox
saintstack commented on a change in pull request #287: HBASE-21512 Reimplement 
sync client based on async client
URL: https://github.com/apache/hbase/pull/287#discussion_r291811434
 
 

 ##
 File path: 
hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java
 ##
 @@ -3381,7 +3364,7 @@ private void getProcedureResult(long procId, 
CompletableFuture future, int
   .call();
   }
 
-  private  ServerRequestCallerBuilder newServerCaller() {
+   ServerRequestCallerBuilder newServerCaller() {
 return this.connection.callerFactory. serverRequest()
   .rpcTimeout(rpcTimeoutNs, TimeUnit.NANOSECONDS)
   .operationTimeout(operationTimeoutNs, TimeUnit.NANOSECONDS)
 
 Review comment:
   I mean, file an issue to investigate and fix andy teething problems that 
might be in the way of h3 working against an h2 cluster.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] saintstack commented on a change in pull request #287: HBASE-21512 Reimplement sync client based on async client

2019-06-08 Thread GitBox
saintstack commented on a change in pull request #287: HBASE-21512 Reimplement 
sync client based on async client
URL: https://github.com/apache/hbase/pull/287#discussion_r291811426
 
 

 ##
 File path: 
hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java
 ##
 @@ -3381,7 +3364,7 @@ private void getProcedureResult(long procId, 
CompletableFuture future, int
   .call();
   }
 
-  private  ServerRequestCallerBuilder newServerCaller() {
+   ServerRequestCallerBuilder newServerCaller() {
 return this.connection.callerFactory. serverRequest()
   .rpcTimeout(rpcTimeoutNs, TimeUnit.NANOSECONDS)
   .operationTimeout(operationTimeoutNs, TimeUnit.NANOSECONDS)
 
 Review comment:
   File an issue? Would be sweet if the h3 client worked against h2.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] saintstack commented on a change in pull request #287: HBASE-21512 Reimplement sync client based on async client

2019-06-08 Thread GitBox
saintstack commented on a change in pull request #287: HBASE-21512 Reimplement 
sync client based on async client
URL: https://github.com/apache/hbase/pull/287#discussion_r291811395
 
 

 ##
 File path: 
hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionUtils.java
 ##
 @@ -695,4 +606,94 @@ static void updateStats(Optional 
optStats,
 metrics -> ResultStatsUtil.updateStats(metrics, serverName, 
regionName, regionLoadStats));
 });
   }
+
+  @FunctionalInterface
+  interface Converter {
+D convert(I info, S src) throws IOException;
+  }
+
+  @FunctionalInterface
+  interface RpcCall {
+void call(ClientService.Interface stub, HBaseRpcController controller, REQ 
req,
+RpcCallback done);
+  }
+
+  static  CompletableFuture 
call(HBaseRpcController controller,
+  HRegionLocation loc, ClientService.Interface stub, REQ req,
+  Converter reqConvert, RpcCall rpcCall,
+  Converter respConverter) {
+CompletableFuture future = new CompletableFuture<>();
+try {
+  rpcCall.call(stub, controller, 
reqConvert.convert(loc.getRegion().getRegionName(), req),
+new RpcCallback() {
+
+  @Override
+  public void run(PRESP resp) {
+if (controller.failed()) {
+  future.completeExceptionally(controller.getFailed());
+} else {
+  try {
+future.complete(respConverter.convert(controller, resp));
+  } catch (IOException e) {
+future.completeExceptionally(e);
+  }
+}
+  }
+});
+} catch (IOException e) {
+  future.completeExceptionally(e);
+}
+return future;
+  }
+
+  static ThreadPoolExecutor getThreadPool(Configuration conf, int maxThreads, 
int coreThreads,
 
 Review comment:
   Does this need more comments saying that its for CPs only? Or maybe there 
are comments to this effect elsewhere, just not down here at this level?
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] saintstack commented on a change in pull request #287: HBASE-21512 Reimplement sync client based on async client

2019-06-08 Thread GitBox
saintstack commented on a change in pull request #287: HBASE-21512 Reimplement 
sync client based on async client
URL: https://github.com/apache/hbase/pull/287#discussion_r291811409
 
 

 ##
 File path: 
hbase-client/src/main/java/org/apache/hadoop/hbase/client/NoncedRegionServerCallable.java
 ##
 @@ -1,62 +0,0 @@
-/**
 
 Review comment:
   Man, so much cruft has been stripped from the client. It is great.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] saintstack commented on a change in pull request #287: HBASE-21512 Reimplement sync client based on async client

2019-06-08 Thread GitBox
saintstack commented on a change in pull request #287: HBASE-21512 Reimplement 
sync client based on async client
URL: https://github.com/apache/hbase/pull/287#discussion_r291811416
 
 

 ##
 File path: 
hbase-client/src/main/java/org/apache/hadoop/hbase/client/NoncedRegionServerCallable.java
 ##
 @@ -1,62 +0,0 @@
-/**
 
 Review comment:
   It is like a startover.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] saintstack commented on a change in pull request #287: HBASE-21512 Reimplement sync client based on async client

2019-06-08 Thread GitBox
saintstack commented on a change in pull request #287: HBASE-21512 Reimplement 
sync client based on async client
URL: https://github.com/apache/hbase/pull/287#discussion_r291811366
 
 

 ##
 File path: 
hbase-client/src/main/java/org/apache/hadoop/hbase/client/BufferedMutator.java
 ##
 @@ -62,7 +62,11 @@
 public interface BufferedMutator extends Closeable {
   /**
* Key to use setting non-default BufferedMutator implementation in 
Configuration.
+   * 
+   * @deprecated Since 3.0.0, will be removed in 4.0.0. For internal test use 
only, do not use it
 
 Review comment:
   Interested in what you are thinking here.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] saintstack commented on a change in pull request #287: HBASE-21512 Reimplement sync client based on async client

2019-06-06 Thread GitBox
saintstack commented on a change in pull request #287: HBASE-21512 Reimplement 
sync client based on async client
URL: https://github.com/apache/hbase/pull/287#discussion_r291333121
 
 

 ##
 File path: 
hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnectionImpl.java
 ##
 @@ -365,11 +393,6 @@ public Hbck getHbck(ServerName masterServer) throws 
IOException {
   rpcClient.createBlockingRpcChannel(masterServer, user, rpcTimeout)), 
rpcControllerFactory);
   }
 
-  @Override
-  public void clearRegionLocationCache() {
 
 Review comment:
   Ok to remove this?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] saintstack commented on a change in pull request #287: HBASE-21512 Reimplement sync client based on async client

2019-06-06 Thread GitBox
saintstack commented on a change in pull request #287: HBASE-21512 Reimplement 
sync client based on async client
URL: https://github.com/apache/hbase/pull/287#discussion_r291392619
 
 

 ##
 File path: 
hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionUtils.java
 ##
 @@ -695,4 +606,94 @@ static void updateStats(Optional 
optStats,
 metrics -> ResultStatsUtil.updateStats(metrics, serverName, 
regionName, regionLoadStats));
 });
   }
+
+  @FunctionalInterface
+  interface Converter {
+D convert(I info, S src) throws IOException;
+  }
+
+  @FunctionalInterface
+  interface RpcCall {
+void call(ClientService.Interface stub, HBaseRpcController controller, REQ 
req,
+RpcCallback done);
+  }
+
+  static  CompletableFuture 
call(HBaseRpcController controller,
+  HRegionLocation loc, ClientService.Interface stub, REQ req,
+  Converter reqConvert, RpcCall rpcCall,
+  Converter respConverter) {
+CompletableFuture future = new CompletableFuture<>();
+try {
+  rpcCall.call(stub, controller, 
reqConvert.convert(loc.getRegion().getRegionName(), req),
+new RpcCallback() {
+
+  @Override
+  public void run(PRESP resp) {
+if (controller.failed()) {
+  future.completeExceptionally(controller.getFailed());
+} else {
+  try {
+future.complete(respConverter.convert(controller, resp));
+  } catch (IOException e) {
+future.completeExceptionally(e);
+  }
+}
+  }
+});
+} catch (IOException e) {
+  future.completeExceptionally(e);
+}
+return future;
+  }
+
+  static ThreadPoolExecutor getThreadPool(Configuration conf, int maxThreads, 
int coreThreads,
 
 Review comment:
   This pool is shared by what in the client?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] saintstack commented on a change in pull request #287: HBASE-21512 Reimplement sync client based on async client

2019-06-06 Thread GitBox
saintstack commented on a change in pull request #287: HBASE-21512 Reimplement 
sync client based on async client
URL: https://github.com/apache/hbase/pull/287#discussion_r291284477
 
 

 ##
 File path: 
hbase-client/src/main/java/org/apache/hadoop/hbase/client/AdminOverAsyncAdmin.java
 ##
 @@ -0,0 +1,945 @@
+/**
+ * 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.hadoop.hbase.client;
+
+import static 
org.apache.hadoop.hbase.client.ConnectionUtils.setCoprocessorError;
+import static org.apache.hadoop.hbase.util.FutureUtils.get;
+
+import com.google.protobuf.Descriptors.MethodDescriptor;
+import com.google.protobuf.Message;
+import com.google.protobuf.RpcCallback;
+import com.google.protobuf.RpcChannel;
+import com.google.protobuf.RpcController;
+import com.google.protobuf.ServiceException;
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.Future;
+import java.util.regex.Pattern;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.CacheEvictionStats;
+import org.apache.hadoop.hbase.ClusterMetrics;
+import org.apache.hadoop.hbase.ClusterMetrics.Option;
+import org.apache.hadoop.hbase.HConstants;
+import org.apache.hadoop.hbase.NamespaceDescriptor;
+import org.apache.hadoop.hbase.NamespaceNotFoundException;
+import org.apache.hadoop.hbase.RegionMetrics;
+import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.TableExistsException;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.TableNotFoundException;
+import org.apache.hadoop.hbase.client.replication.TableCFs;
+import org.apache.hadoop.hbase.client.security.SecurityCapability;
+import org.apache.hadoop.hbase.ipc.CoprocessorRpcChannel;
+import org.apache.hadoop.hbase.quotas.QuotaFilter;
+import org.apache.hadoop.hbase.quotas.QuotaSettings;
+import org.apache.hadoop.hbase.quotas.SpaceQuotaSnapshotView;
+import org.apache.hadoop.hbase.regionserver.wal.FailedLogCloseException;
+import org.apache.hadoop.hbase.replication.ReplicationPeerConfig;
+import org.apache.hadoop.hbase.replication.ReplicationPeerDescription;
+import org.apache.hadoop.hbase.replication.SyncReplicationState;
+import org.apache.hadoop.hbase.security.access.GetUserPermissionsRequest;
+import org.apache.hadoop.hbase.security.access.Permission;
+import org.apache.hadoop.hbase.security.access.UserPermission;
+import org.apache.hadoop.hbase.snapshot.HBaseSnapshotException;
+import org.apache.hadoop.hbase.snapshot.RestoreSnapshotException;
+import org.apache.hadoop.hbase.snapshot.SnapshotCreationException;
+import org.apache.hadoop.hbase.snapshot.UnknownSnapshotException;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * The {@link Admin} implementation which is based on an {@link AsyncAdmin}.
+ */
+@InterfaceAudience.Private
+class AdminOverAsyncAdmin implements Admin {
 
 Review comment:
   We implement Admin here and not AsyncAdmin?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] saintstack commented on a change in pull request #287: HBASE-21512 Reimplement sync client based on async client

2019-06-06 Thread GitBox
saintstack commented on a change in pull request #287: HBASE-21512 Reimplement 
sync client based on async client
URL: https://github.com/apache/hbase/pull/287#discussion_r291391113
 
 

 ##
 File path: 
hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionFactory.java
 ##
 @@ -211,25 +212,22 @@ public static Connection createConnection(Configuration 
conf, User user) throws
* @return Connection object for conf
*/
   public static Connection createConnection(Configuration conf, 
ExecutorService pool,
-final User user) throws IOException {
-String className = conf.get(ClusterConnection.HBASE_CLIENT_CONNECTION_IMPL,
-  ConnectionImplementation.class.getName());
-Class clazz;
-try {
-  clazz = Class.forName(className);
-} catch (ClassNotFoundException e) {
-  throw new IOException(e);
-}
-try {
-  // Default HCM#HCI is not accessible; make it so before invoking.
-  Constructor constructor = 
clazz.getDeclaredConstructor(Configuration.class,
-ExecutorService.class, User.class);
-  constructor.setAccessible(true);
-  return user.runAs(
-(PrivilegedExceptionAction)() ->
-  (Connection) constructor.newInstance(conf, pool, user));
-} catch (Exception e) {
-  throw new IOException(e);
+  final User user) throws IOException {
+Class clazz = 
conf.getClass(ConnectionUtils.HBASE_CLIENT_CONNECTION_IMPL,
+  ConnectionOverAsyncConnection.class, Connection.class);
 
 Review comment:
   At least the naming conention of '...OverAsync...' seems to be applied 
consistently. Thats good.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] saintstack commented on a change in pull request #287: HBASE-21512 Reimplement sync client based on async client

2019-06-06 Thread GitBox
saintstack commented on a change in pull request #287: HBASE-21512 Reimplement 
sync client based on async client
URL: https://github.com/apache/hbase/pull/287#discussion_r291333184
 
 

 ##
 File path: 
hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnectionImpl.java
 ##
 @@ -365,11 +393,6 @@ public Hbck getHbck(ServerName masterServer) throws 
IOException {
   rpcClient.createBlockingRpcChannel(masterServer, user, rpcTimeout)), 
rpcControllerFactory);
   }
 
-  @Override
-  public void clearRegionLocationCache() {
 
 Review comment:
   No deprecation?
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] saintstack commented on a change in pull request #287: HBASE-21512 Reimplement sync client based on async client

2019-06-06 Thread GitBox
saintstack commented on a change in pull request #287: HBASE-21512 Reimplement 
sync client based on async client
URL: https://github.com/apache/hbase/pull/287#discussion_r291331703
 
 

 ##
 File path: 
hbase-client/src/main/java/org/apache/hadoop/hbase/client/AdminOverAsyncAdmin.java
 ##
 @@ -0,0 +1,945 @@
+/**
+ * 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.hadoop.hbase.client;
+
+import static 
org.apache.hadoop.hbase.client.ConnectionUtils.setCoprocessorError;
+import static org.apache.hadoop.hbase.util.FutureUtils.get;
+
+import com.google.protobuf.Descriptors.MethodDescriptor;
+import com.google.protobuf.Message;
+import com.google.protobuf.RpcCallback;
+import com.google.protobuf.RpcChannel;
+import com.google.protobuf.RpcController;
+import com.google.protobuf.ServiceException;
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.Future;
+import java.util.regex.Pattern;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.CacheEvictionStats;
+import org.apache.hadoop.hbase.ClusterMetrics;
+import org.apache.hadoop.hbase.ClusterMetrics.Option;
+import org.apache.hadoop.hbase.HConstants;
+import org.apache.hadoop.hbase.NamespaceDescriptor;
+import org.apache.hadoop.hbase.NamespaceNotFoundException;
+import org.apache.hadoop.hbase.RegionMetrics;
+import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.TableExistsException;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.TableNotFoundException;
+import org.apache.hadoop.hbase.client.replication.TableCFs;
+import org.apache.hadoop.hbase.client.security.SecurityCapability;
+import org.apache.hadoop.hbase.ipc.CoprocessorRpcChannel;
+import org.apache.hadoop.hbase.quotas.QuotaFilter;
+import org.apache.hadoop.hbase.quotas.QuotaSettings;
+import org.apache.hadoop.hbase.quotas.SpaceQuotaSnapshotView;
+import org.apache.hadoop.hbase.regionserver.wal.FailedLogCloseException;
+import org.apache.hadoop.hbase.replication.ReplicationPeerConfig;
+import org.apache.hadoop.hbase.replication.ReplicationPeerDescription;
+import org.apache.hadoop.hbase.replication.SyncReplicationState;
+import org.apache.hadoop.hbase.security.access.GetUserPermissionsRequest;
+import org.apache.hadoop.hbase.security.access.Permission;
+import org.apache.hadoop.hbase.security.access.UserPermission;
+import org.apache.hadoop.hbase.snapshot.HBaseSnapshotException;
+import org.apache.hadoop.hbase.snapshot.RestoreSnapshotException;
+import org.apache.hadoop.hbase.snapshot.SnapshotCreationException;
+import org.apache.hadoop.hbase.snapshot.UnknownSnapshotException;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * The {@link Admin} implementation which is based on an {@link AsyncAdmin}.
+ */
+@InterfaceAudience.Private
+class AdminOverAsyncAdmin implements Admin {
 
 Review comment:
   The name of this class is a little awkward. Call it AdminImpl?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] saintstack commented on a change in pull request #287: HBASE-21512 Reimplement sync client based on async client

2019-06-06 Thread GitBox
saintstack commented on a change in pull request #287: HBASE-21512 Reimplement 
sync client based on async client
URL: https://github.com/apache/hbase/pull/287#discussion_r291287712
 
 

 ##
 File path: 
hbase-client/src/main/java/org/apache/hadoop/hbase/client/AdminOverAsyncAdmin.java
 ##
 @@ -0,0 +1,945 @@
+/**
+ * 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.hadoop.hbase.client;
+
+import static 
org.apache.hadoop.hbase.client.ConnectionUtils.setCoprocessorError;
+import static org.apache.hadoop.hbase.util.FutureUtils.get;
+
+import com.google.protobuf.Descriptors.MethodDescriptor;
+import com.google.protobuf.Message;
+import com.google.protobuf.RpcCallback;
+import com.google.protobuf.RpcChannel;
+import com.google.protobuf.RpcController;
+import com.google.protobuf.ServiceException;
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.Future;
+import java.util.regex.Pattern;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.CacheEvictionStats;
+import org.apache.hadoop.hbase.ClusterMetrics;
+import org.apache.hadoop.hbase.ClusterMetrics.Option;
+import org.apache.hadoop.hbase.HConstants;
+import org.apache.hadoop.hbase.NamespaceDescriptor;
+import org.apache.hadoop.hbase.NamespaceNotFoundException;
+import org.apache.hadoop.hbase.RegionMetrics;
+import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.TableExistsException;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.TableNotFoundException;
+import org.apache.hadoop.hbase.client.replication.TableCFs;
+import org.apache.hadoop.hbase.client.security.SecurityCapability;
+import org.apache.hadoop.hbase.ipc.CoprocessorRpcChannel;
+import org.apache.hadoop.hbase.quotas.QuotaFilter;
+import org.apache.hadoop.hbase.quotas.QuotaSettings;
+import org.apache.hadoop.hbase.quotas.SpaceQuotaSnapshotView;
+import org.apache.hadoop.hbase.regionserver.wal.FailedLogCloseException;
+import org.apache.hadoop.hbase.replication.ReplicationPeerConfig;
+import org.apache.hadoop.hbase.replication.ReplicationPeerDescription;
+import org.apache.hadoop.hbase.replication.SyncReplicationState;
+import org.apache.hadoop.hbase.security.access.GetUserPermissionsRequest;
+import org.apache.hadoop.hbase.security.access.Permission;
+import org.apache.hadoop.hbase.security.access.UserPermission;
+import org.apache.hadoop.hbase.snapshot.HBaseSnapshotException;
+import org.apache.hadoop.hbase.snapshot.RestoreSnapshotException;
+import org.apache.hadoop.hbase.snapshot.SnapshotCreationException;
+import org.apache.hadoop.hbase.snapshot.UnknownSnapshotException;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * The {@link Admin} implementation which is based on an {@link AsyncAdmin}.
+ */
+@InterfaceAudience.Private
+class AdminOverAsyncAdmin implements Admin {
 
 Review comment:
   How I get at CompletableFuture? I'd use AsyncAdmin direct?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] saintstack commented on a change in pull request #287: HBASE-21512 Reimplement sync client based on async client

2019-06-06 Thread GitBox
saintstack commented on a change in pull request #287: HBASE-21512 Reimplement 
sync client based on async client
URL: https://github.com/apache/hbase/pull/287#discussion_r291292714
 
 

 ##
 File path: 
hbase-client/src/main/java/org/apache/hadoop/hbase/client/AdminOverAsyncAdmin.java
 ##
 @@ -0,0 +1,945 @@
+/**
+ * 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.hadoop.hbase.client;
+
+import static 
org.apache.hadoop.hbase.client.ConnectionUtils.setCoprocessorError;
+import static org.apache.hadoop.hbase.util.FutureUtils.get;
+
+import com.google.protobuf.Descriptors.MethodDescriptor;
+import com.google.protobuf.Message;
+import com.google.protobuf.RpcCallback;
+import com.google.protobuf.RpcChannel;
+import com.google.protobuf.RpcController;
+import com.google.protobuf.ServiceException;
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.Future;
+import java.util.regex.Pattern;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.CacheEvictionStats;
+import org.apache.hadoop.hbase.ClusterMetrics;
+import org.apache.hadoop.hbase.ClusterMetrics.Option;
+import org.apache.hadoop.hbase.HConstants;
+import org.apache.hadoop.hbase.NamespaceDescriptor;
+import org.apache.hadoop.hbase.NamespaceNotFoundException;
+import org.apache.hadoop.hbase.RegionMetrics;
+import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.TableExistsException;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.TableNotFoundException;
+import org.apache.hadoop.hbase.client.replication.TableCFs;
+import org.apache.hadoop.hbase.client.security.SecurityCapability;
+import org.apache.hadoop.hbase.ipc.CoprocessorRpcChannel;
+import org.apache.hadoop.hbase.quotas.QuotaFilter;
+import org.apache.hadoop.hbase.quotas.QuotaSettings;
+import org.apache.hadoop.hbase.quotas.SpaceQuotaSnapshotView;
+import org.apache.hadoop.hbase.regionserver.wal.FailedLogCloseException;
+import org.apache.hadoop.hbase.replication.ReplicationPeerConfig;
+import org.apache.hadoop.hbase.replication.ReplicationPeerDescription;
+import org.apache.hadoop.hbase.replication.SyncReplicationState;
+import org.apache.hadoop.hbase.security.access.GetUserPermissionsRequest;
+import org.apache.hadoop.hbase.security.access.Permission;
+import org.apache.hadoop.hbase.security.access.UserPermission;
+import org.apache.hadoop.hbase.snapshot.HBaseSnapshotException;
+import org.apache.hadoop.hbase.snapshot.RestoreSnapshotException;
+import org.apache.hadoop.hbase.snapshot.SnapshotCreationException;
+import org.apache.hadoop.hbase.snapshot.UnknownSnapshotException;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * The {@link Admin} implementation which is based on an {@link AsyncAdmin}.
+ */
+@InterfaceAudience.Private
+class AdminOverAsyncAdmin implements Admin {
 
 Review comment:
   So, I suppose Admin is by nature synchronous... most of the calls in it are 
so you are just implementing them as is.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] saintstack commented on a change in pull request #287: HBASE-21512 Reimplement sync client based on async client

2019-06-06 Thread GitBox
saintstack commented on a change in pull request #287: HBASE-21512 Reimplement 
sync client based on async client
URL: https://github.com/apache/hbase/pull/287#discussion_r291392359
 
 

 ##
 File path: 
hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionUtils.java
 ##
 @@ -695,4 +606,94 @@ static void updateStats(Optional 
optStats,
 metrics -> ResultStatsUtil.updateStats(metrics, serverName, 
regionName, regionLoadStats));
 });
   }
+
+  @FunctionalInterface
+  interface Converter {
+D convert(I info, S src) throws IOException;
+  }
+
+  @FunctionalInterface
+  interface RpcCall {
+void call(ClientService.Interface stub, HBaseRpcController controller, REQ 
req,
+RpcCallback done);
+  }
+
+  static  CompletableFuture 
call(HBaseRpcController controller,
+  HRegionLocation loc, ClientService.Interface stub, REQ req,
+  Converter reqConvert, RpcCall rpcCall,
+  Converter respConverter) {
+CompletableFuture future = new CompletableFuture<>();
+try {
+  rpcCall.call(stub, controller, 
reqConvert.convert(loc.getRegion().getRegionName(), req),
+new RpcCallback() {
+
+  @Override
+  public void run(PRESP resp) {
+if (controller.failed()) {
+  future.completeExceptionally(controller.getFailed());
+} else {
+  try {
+future.complete(respConverter.convert(controller, resp));
+  } catch (IOException e) {
+future.completeExceptionally(e);
+  }
+}
+  }
+});
+} catch (IOException e) {
+  future.completeExceptionally(e);
+}
+return future;
+  }
+
+  static ThreadPoolExecutor getThreadPool(Configuration conf, int maxThreads, 
int coreThreads,
+  Supplier threadName, BlockingQueue passedWorkQueue) {
+// shared HTable thread executor not yet initialized
+if (maxThreads == 0) {
+  maxThreads = Runtime.getRuntime().availableProcessors() * 8;
 
 Review comment:
   *8 seems like a lot of threads. What you thinking? Should this method be 
synchronized?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] saintstack commented on a change in pull request #287: HBASE-21512 Reimplement sync client based on async client

2019-06-06 Thread GitBox
saintstack commented on a change in pull request #287: HBASE-21512 Reimplement 
sync client based on async client
URL: https://github.com/apache/hbase/pull/287#discussion_r291336541
 
 

 ##
 File path: 
hbase-client/src/main/java/org/apache/hadoop/hbase/client/Cancellable.java
 ##
 @@ -1,31 +0,0 @@
-/**
- *
- * 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.hadoop.hbase.client;
-import org.apache.yetus.audience.InterfaceAudience;
-
-/**
- * This should be implemented by the Get/Scan implementations that
- * talk to replica regions. When an RPC response is received from one
- * of the replicas, the RPCs to the other replicas are cancelled.
- */
-@InterfaceAudience.Private
-interface Cancellable {
-  public void cancel();
-  public boolean isCancelled();
-}
 
 Review comment:
   Read replicas have changed in their implementation? They still work?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] saintstack commented on a change in pull request #287: HBASE-21512 Reimplement sync client based on async client

2019-06-06 Thread GitBox
saintstack commented on a change in pull request #287: HBASE-21512 Reimplement 
sync client based on async client
URL: https://github.com/apache/hbase/pull/287#discussion_r291335476
 
 

 ##
 File path: 
hbase-client/src/main/java/org/apache/hadoop/hbase/client/BufferedMutatorParams.java
 ##
 @@ -141,17 +149,23 @@ public BufferedMutatorParams pool(ExecutorService pool) {
   }
 
   /**
-   * @return Name of the class we will use when we construct a
-   * {@link BufferedMutator} instance or null if default implementation.
+   * @return Name of the class we will use when we construct a {@link 
BufferedMutator} instance or
+   * null if default implementation.
+   * @deprecated Since 3.0.0, will be removed in 4.0.0. You can not set it any 
more as the
+   * implementation has to use too many internal stuffs in HBase.
 
 Review comment:
   Ok. Makes sense. Nice comment.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] saintstack commented on a change in pull request #287: HBASE-21512 Reimplement sync client based on async client

2019-06-06 Thread GitBox
saintstack commented on a change in pull request #287: HBASE-21512 Reimplement 
sync client based on async client
URL: https://github.com/apache/hbase/pull/287#discussion_r291395722
 
 

 ##
 File path: 
hbase-client/src/main/java/org/apache/hadoop/hbase/client/ShortCircuitMasterConnection.java
 ##
 @@ -1,716 +0,0 @@
-/**
 
 Review comment:
   Short-circuit works still in async world?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] saintstack commented on a change in pull request #287: HBASE-21512 Reimplement sync client based on async client

2019-06-06 Thread GitBox
saintstack commented on a change in pull request #287: HBASE-21512 Reimplement 
sync client based on async client
URL: https://github.com/apache/hbase/pull/287#discussion_r291395120
 
 

 ##
 File path: 
hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java
 ##
 @@ -3381,7 +3364,7 @@ private void getProcedureResult(long procId, 
CompletableFuture future, int
   .call();
   }
 
-  private  ServerRequestCallerBuilder newServerCaller() {
+   ServerRequestCallerBuilder newServerCaller() {
 return this.connection.callerFactory. serverRequest()
   .rpcTimeout(rpcTimeoutNs, TimeUnit.NANOSECONDS)
   .operationTimeout(operationTimeoutNs, TimeUnit.NANOSECONDS)
 
 Review comment:
   An hbase3 client will not be able to talk to an hbase2 cluster, right? Or 
will it?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] saintstack commented on a change in pull request #287: HBASE-21512 Reimplement sync client based on async client

2019-06-06 Thread GitBox
saintstack commented on a change in pull request #287: HBASE-21512 Reimplement 
sync client based on async client
URL: https://github.com/apache/hbase/pull/287#discussion_r291393009
 
 

 ##
 File path: 
hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionUtils.java
 ##
 @@ -695,4 +606,94 @@ static void updateStats(Optional 
optStats,
 metrics -> ResultStatsUtil.updateStats(metrics, serverName, 
regionName, regionLoadStats));
 });
   }
+
+  @FunctionalInterface
+  interface Converter {
+D convert(I info, S src) throws IOException;
+  }
+
+  @FunctionalInterface
+  interface RpcCall {
+void call(ClientService.Interface stub, HBaseRpcController controller, REQ 
req,
+RpcCallback done);
+  }
+
+  static  CompletableFuture 
call(HBaseRpcController controller,
+  HRegionLocation loc, ClientService.Interface stub, REQ req,
+  Converter reqConvert, RpcCall rpcCall,
+  Converter respConverter) {
+CompletableFuture future = new CompletableFuture<>();
+try {
+  rpcCall.call(stub, controller, 
reqConvert.convert(loc.getRegion().getRegionName(), req),
+new RpcCallback() {
+
+  @Override
+  public void run(PRESP resp) {
+if (controller.failed()) {
+  future.completeExceptionally(controller.getFailed());
+} else {
+  try {
+future.complete(respConverter.convert(controller, resp));
+  } catch (IOException e) {
+future.completeExceptionally(e);
+  }
+}
+  }
+});
+} catch (IOException e) {
+  future.completeExceptionally(e);
+}
+return future;
+  }
+
+  static ThreadPoolExecutor getThreadPool(Configuration conf, int maxThreads, 
int coreThreads,
 
 Review comment:
   Or it is just generic utility? Want to say in comment what choices it makes 
internally when say maxThreads == 0.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] saintstack commented on a change in pull request #287: HBASE-21512 Reimplement sync client based on async client

2019-06-06 Thread GitBox
saintstack commented on a change in pull request #287: HBASE-21512 Reimplement 
sync client based on async client
URL: https://github.com/apache/hbase/pull/287#discussion_r291394944
 
 

 ##
 File path: 
hbase-client/src/main/java/org/apache/hadoop/hbase/client/NoncedRegionServerCallable.java
 ##
 @@ -1,62 +0,0 @@
-/**
 
 Review comment:
   We still support nonces right? In async too?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] saintstack commented on a change in pull request #287: HBASE-21512 Reimplement sync client based on async client

2019-06-06 Thread GitBox
saintstack commented on a change in pull request #287: HBASE-21512 Reimplement 
sync client based on async client
URL: https://github.com/apache/hbase/pull/287#discussion_r291391414
 
 

 ##
 File path: 
hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionOverAsyncConnection.java
 ##
 @@ -0,0 +1,189 @@
+/**
+ * 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.hadoop.hbase.client;
+
+import java.io.IOException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.TimeUnit;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.log.HBaseMarkers;
+import org.apache.hadoop.hbase.util.FutureUtils;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hbase.thirdparty.com.google.common.io.Closeables;
+
+/**
+ * The connection implementation based on {@link AsyncConnection}.
+ */
+@InterfaceAudience.Private
+class ConnectionOverAsyncConnection implements Connection {
 
 Review comment:
   Is it possible to get at methods that return CompletableFuture?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] saintstack commented on a change in pull request #287: HBASE-21512 Reimplement sync client based on async client

2019-06-06 Thread GitBox
saintstack commented on a change in pull request #287: HBASE-21512 Reimplement 
sync client based on async client
URL: https://github.com/apache/hbase/pull/287#discussion_r291334211
 
 

 ##
 File path: 
hbase-client/src/main/java/org/apache/hadoop/hbase/client/BufferedMutator.java
 ##
 @@ -62,7 +62,11 @@
 public interface BufferedMutator extends Closeable {
   /**
* Key to use setting non-default BufferedMutator implementation in 
Configuration.
+   * 
+   * @deprecated Since 3.0.0, will be removed in 4.0.0. For internal test use 
only, do not use it
 
 Review comment:
   Oh. So how is this functionality done going forward?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] saintstack commented on a change in pull request #287: HBASE-21512 Reimplement sync client based on async client

2019-06-06 Thread GitBox
saintstack commented on a change in pull request #287: HBASE-21512 Reimplement 
sync client based on async client
URL: https://github.com/apache/hbase/pull/287#discussion_r291396210
 
 

 ##
 File path: 
hbase-client/src/test/java/org/apache/hadoop/hbase/client/SimpleRegistry.java
 ##
 @@ -0,0 +1,83 @@
+/**
+ * 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.hadoop.hbase.client;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.DoNotRetryIOException;
+import org.apache.hadoop.hbase.HConstants;
+import org.apache.hadoop.hbase.HRegionLocation;
+import org.apache.hadoop.hbase.RegionLocations;
+import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.util.FutureUtils;
+
+/**
+ * Simple cluster registry inserted in place of our usual zookeeper based one.
 
 Review comment:
   When is this used?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] saintstack commented on a change in pull request #287: HBASE-21512 Reimplement sync client based on async client

2019-06-06 Thread GitBox
saintstack commented on a change in pull request #287: HBASE-21512 Reimplement 
sync client based on async client
URL: https://github.com/apache/hbase/pull/287#discussion_r291333835
 
 

 ##
 File path: 
hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncProcess.java
 ##
 @@ -1,482 +0,0 @@
-/*
- *
- * 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.hadoop.hbase.client;
-
-
-import 
org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting;
-
-import java.io.IOException;
-import java.io.InterruptedIOException;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Map;
-import java.util.Objects;
-import java.util.concurrent.atomic.AtomicLong;
-import java.util.function.Consumer;
-
-import org.apache.hadoop.conf.Configuration;
-import org.apache.hadoop.hbase.HConstants;
-import org.apache.hadoop.hbase.HRegionLocation;
-import org.apache.hadoop.hbase.RegionLocations;
-import org.apache.hadoop.hbase.ServerName;
-import org.apache.hadoop.hbase.TableName;
-import org.apache.yetus.audience.InterfaceAudience;
-import org.apache.yetus.audience.InterfaceStability;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-import org.apache.hadoop.hbase.client.AsyncProcessTask.SubmittedRows;
-import org.apache.hadoop.hbase.client.RequestController.ReturnCode;
-import org.apache.hadoop.hbase.ipc.RpcControllerFactory;
-import org.apache.hadoop.hbase.util.Bytes;
-
-/**
- * This class  allows a continuous flow of requests. It's written to be 
compatible with a
- * synchronous caller such as HTable.
- * 
- * The caller sends a buffer of operation, by calling submit. This class 
extract from this list
- * the operations it can send, i.e. the operations that are on region that are 
not considered
- * as busy. The process is asynchronous, i.e. it returns immediately when if 
has finished to
- * iterate on the list. If, and only if, the maximum number of current task is 
reached, the call
- * to submit will block. Alternatively, the caller can call submitAll, in 
which case all the
- * operations will be sent. Each call to submit returns a future-like object 
that can be used
- * to track operation progress.
- * 
- * 
- * The class manages internally the retries.
- * 
- * 
- * The errors are tracked inside the Future object that is returned.
- * The results are always tracked inside the Future object and can be 
retrieved when the call
- * has finished. Partial results can also be retrieved if some part of 
multi-request failed.
- * 
- * 
- * This class is thread safe.
- * Internally, the class is thread safe enough to manage simultaneously new 
submission and results
- * arising from older operations.
- * 
- * 
- * Internally, this class works with {@link Row}, this mean it could be 
theoretically used for
- * gets as well.
- * 
- */
-@InterfaceAudience.Private
-@InterfaceStability.Evolving
-class AsyncProcess {
-  private static final Logger LOG = 
LoggerFactory.getLogger(AsyncProcess.class);
-  private static final AtomicLong COUNTER = new AtomicLong();
-
-  public static final String PRIMARY_CALL_TIMEOUT_KEY = 
"hbase.client.primaryCallTimeout.multiget";
-
-  /**
-   * Configure the number of failures after which the client will start 
logging. A few failures
-   * is fine: region moved, then is not opened, then is overloaded. We try to 
have an acceptable
-   * heuristic for the number of errors we don't log. 5 was chosen because we 
wait for 1s at
-   * this stage.
-   */
-  public static final String START_LOG_ERRORS_AFTER_COUNT_KEY =
-  "hbase.client.start.log.errors.counter";
-  public static final int DEFAULT_START_LOG_ERRORS_AFTER_COUNT = 5;
-
-  /**
-   * Configuration to decide whether to log details for batch error
-   */
-  public static final String LOG_DETAILS_FOR_BATCH_ERROR = 
"hbase.client.log.batcherrors.details";
-
-  /**
-   * Return value from a submit that didn't contain any requests.
-   */
-  private static final AsyncRequestFuture NO_REQS_RESULT = new 
AsyncRequestFuture() {
-final Object[] result = new Object[0];
-
-@Override
-public boolean hasError() {
-  r

[GitHub] [hbase] saintstack commented on a change in pull request #287: HBASE-21512 Reimplement sync client based on async client

2019-06-06 Thread GitBox
saintstack commented on a change in pull request #287: HBASE-21512 Reimplement 
sync client based on async client
URL: https://github.com/apache/hbase/pull/287#discussion_r291337125
 
 

 ##
 File path: 
hbase-client/src/main/java/org/apache/hadoop/hbase/client/ClientAsyncPrefetchScanner.java
 ##
 @@ -1,193 +0,0 @@
-/**
 
 Review comment:
   Prefetch is gone. No longer supported (it didn't work anyways?)


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [hbase] saintstack commented on a change in pull request #287: HBASE-21512 Reimplement sync client based on async client

2019-06-06 Thread GitBox
saintstack commented on a change in pull request #287: HBASE-21512 Reimplement 
sync client based on async client
URL: https://github.com/apache/hbase/pull/287#discussion_r291334547
 
 

 ##
 File path: 
hbase-client/src/main/java/org/apache/hadoop/hbase/client/BufferedMutator.java
 ##
 @@ -62,7 +62,11 @@
 public interface BufferedMutator extends Closeable {
   /**
* Key to use setting non-default BufferedMutator implementation in 
Configuration.
+   * 
+   * @deprecated Since 3.0.0, will be removed in 4.0.0. For internal test use 
only, do not use it
 
 Review comment:
   Or is it just that you cannot override BM w/ your own impl going forward?
   
   There was at least one attempt at extension... have BM spool locally if it 
could not add edits to remote cluster.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services