From 217ccc71e9b97dd9c0e1d168b4d626470cc3b160 Mon Sep 17 00:00:00 2001 From: Jurriaan Mous Date: Sat, 11 Jun 2016 13:34:56 +0200 Subject: [PATCH] HBASE-15978 Netty API leaked into public API --- .../org/apache/hadoop/hbase/client/Future.java | 20 ++++++++++++--- .../apache/hadoop/hbase/client/InternalFuture.java | 29 ++++++++++++++++++++++ .../hbase/client/ResponseFutureListener.java | 16 +++++++++--- .../apache/hadoop/hbase/ipc/AsyncRpcClient.java | 8 +++++- .../java/org/apache/hadoop/hbase/ipc/Promise.java | 15 +++++++++-- 5 files changed, 79 insertions(+), 9 deletions(-) create mode 100644 hbase-client/src/main/java/org/apache/hadoop/hbase/client/InternalFuture.java diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Future.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Future.java index 99a8baa..682a209 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Future.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Future.java @@ -28,7 +28,21 @@ import org.apache.hadoop.hbase.classification.InterfaceStability; @InterfaceStability.Evolving @edu.umd.cs.findbugs.annotations.SuppressWarnings(value="NM_SAME_SIMPLE_NAME_AS_INTERFACE", - justification="Agree that this can be confusing but folks will pull in this and think twice " - + "about pulling in netty; incidence of confusion should be rare in this case.") -public interface Future extends io.netty.util.concurrent.Future { + justification="Agree that this can be confusing but people will likely pick the HBase Future " + + "instead of java future; incidence of confusion should be rare in this case.") +public interface Future extends java.util.concurrent.Future { + + /** + * Add a listener to listen to future responses. If the future is already done the listener will + * be called immediately + * @param listener with code to run when future is done + */ + void addListener(ResponseFutureListener listener); + + /** + * Remove a specified listener from this future. It will then not be notified of any future + * completions. If listener is not added to this future it will do nothing. + * @param listener + */ + void removeListener(ResponseFutureListener listener); } \ No newline at end of file diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/InternalFuture.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/InternalFuture.java new file mode 100644 index 0000000..f8354dd --- /dev/null +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/InternalFuture.java @@ -0,0 +1,29 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hbase.client; + +import org.apache.hadoop.hbase.classification.InterfaceAudience; + +/** + * This is the internal Future used within the RPCClient. It combines both the Netty future with the + * HBase future. Netty future should not leak out. + * @param Type of response expected from the future + */ +@InterfaceAudience.Private +public interface InternalFuture extends Future, io.netty.util.concurrent.Future { +} diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ResponseFutureListener.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ResponseFutureListener.java index f23dc8f..a381f84 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ResponseFutureListener.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ResponseFutureListener.java @@ -19,12 +19,22 @@ package org.apache.hadoop.hbase.client; import io.netty.util.concurrent.GenericFutureListener; import org.apache.hadoop.hbase.classification.InterfaceAudience; +import org.apache.hadoop.hbase.classification.InterfaceStability; /** * Specific interface for the Response future listener * @param Value type. */ -@InterfaceAudience.Private -public interface ResponseFutureListener - extends GenericFutureListener> { +@InterfaceAudience.Public +@InterfaceStability.Evolving +public abstract class ResponseFutureListener + implements GenericFutureListener> { + + public abstract void complete(Future future) throws Exception; + + @Override + @InterfaceAudience.Private + public void operationComplete(InternalFuture future) throws Exception { + this.complete(future); + } } diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/AsyncRpcClient.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/AsyncRpcClient.java index 723a234..44ab62b 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/AsyncRpcClient.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/AsyncRpcClient.java @@ -57,6 +57,7 @@ import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.hbase.client.Future; +import org.apache.hadoop.hbase.client.InternalFuture; import org.apache.hadoop.hbase.client.MetricsConnection; import org.apache.hadoop.hbase.client.ResponseFutureListener; import org.apache.hadoop.hbase.security.User; @@ -293,7 +294,12 @@ public class AsyncRpcClient extends AbstractRpcClient { ResponseFutureListener listener = new ResponseFutureListener() { @Override - public void operationComplete(Future future) throws Exception { + public void complete(Future future) throws Exception { + // We override the netty future method here so implementing this one is not needed. + } + + @Override + public void operationComplete(InternalFuture future) throws Exception { if (!future.isSuccess()) { Throwable cause = future.cause(); if (cause instanceof IOException) { diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/Promise.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/Promise.java index 0d05db8..2c7b7aa 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/Promise.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/Promise.java @@ -20,14 +20,15 @@ package org.apache.hadoop.hbase.ipc; import io.netty.util.concurrent.DefaultPromise; import io.netty.util.concurrent.EventExecutor; import org.apache.hadoop.hbase.classification.InterfaceAudience; -import org.apache.hadoop.hbase.client.Future; +import org.apache.hadoop.hbase.client.InternalFuture; +import org.apache.hadoop.hbase.client.ResponseFutureListener; /** * Abstract response promise * @param Type of result contained in Promise */ @InterfaceAudience.Private -public class Promise extends DefaultPromise implements Future { +public class Promise extends DefaultPromise implements InternalFuture { /** * Constructor * @param eventLoop to handle events on @@ -35,4 +36,14 @@ public class Promise extends DefaultPromise implements Future { public Promise(EventExecutor eventLoop) { super(eventLoop); } + + @Override + public void addListener(ResponseFutureListener listener) { + super.addListener(listener); + } + + @Override + public void removeListener(ResponseFutureListener listener) { + super.removeListener(listener); + } } -- 2.5.0