You are viewing a plain text version of this content. The canonical link for it is here.

Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2020/02/19 14:48:05 UTC

[GitHub] [pulsar] zymap opened a new pull request #6365: Add retry when using async method to perform request in AsyncHttpConnector

zymap opened a new pull request #6365: Add retry when using async method to perform request in AsyncHttpConnector
URL: https://github.com/apache/pulsar/pull/6365
 
 
   ---
   
   Master Issue: #<xyz>
   
   *Motivation*
   
   There are two methods in AsycnHttpConnector to perform a request
   for pulsar admin. One is the sync method and another is the async
   method. We only add the multi-host logic in the sync method,
   this PR makes the async method can handle the multi-host URL.
   
   *Modifications*
   
   - Make the async apply method in the `AsycnHttpConnector` can handle the multi-host
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [pulsar] yjshen commented on issue #6365: Add retry when using async method to perform request in AsyncHttpConnector

Posted by GitBox <gi...@apache.org>.

yjshen commented on issue #6365: Add retry when using async method to perform request in AsyncHttpConnector
URL: https://github.com/apache/pulsar/pull/6365#issuecomment-589915755
 
 
   /pulsarbot run-failure-checks
   
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [pulsar] zymap commented on issue #6365: Add retry when using async method to perform request in AsyncHttpConnector

Posted by GitBox <gi...@apache.org>.

zymap commented on issue #6365: Add retry when using async method to perform request in AsyncHttpConnector
URL: https://github.com/apache/pulsar/pull/6365#issuecomment-589064676
 
 
   /pulsarbot run-failure-checks

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [pulsar] zymap commented on issue #6365: Add retry when using async method to perform request in AsyncHttpConnector

Posted by GitBox <gi...@apache.org>.

zymap commented on issue #6365: Add retry when using async method to perform request in AsyncHttpConnector
URL: https://github.com/apache/pulsar/pull/6365#issuecomment-588972098
 
 
   /pulsarbot run-failure-checks

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [pulsar] zymap closed pull request #6365: Add retry when using async method to perform request in AsyncHttpConnector

Posted by GitBox <gi...@apache.org>.

zymap closed pull request #6365: Add retry when using async method to perform request in AsyncHttpConnector
URL: https://github.com/apache/pulsar/pull/6365
 
 
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [pulsar] jiazhai commented on issue #6365: Add retry when using async method to perform request in AsyncHttpConnector

Posted by GitBox <gi...@apache.org>.

jiazhai commented on issue #6365: Add retry when using async method to perform request in AsyncHttpConnector
URL: https://github.com/apache/pulsar/pull/6365#issuecomment-593989913
 
 
   @zymap for the new comments

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [pulsar] sijie commented on a change in pull request #6365: Add retry when using async method to perform request in AsyncHttpConnector

Posted by GitBox <gi...@apache.org>.

sijie commented on a change in pull request #6365: Add retry when using async method to perform request in AsyncHttpConnector
URL: https://github.com/apache/pulsar/pull/6365#discussion_r386057297
 
 

 ##########
 File path: pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/http/AsyncHttpConnector.java
 ##########
 @@ -176,39 +153,39 @@ private URI replaceWithNew(InetSocketAddress address, URI uri) {
         return URI.create(newUri);
     }
 
+    @Override
+    public Future<?> apply(ClientRequest jerseyRequest, AsyncConnectorCallback callback) {
+        final CompletableFuture<ClientResponse> respFuture = new CompletableFuture<>();
+        triedAddressees = new HashSet<>();
+        doNextRetry(jerseyRequest, callback, respFuture);
+        return respFuture;
+    }
 
-
-    private void resolveRequest(CompletableFuture<ClientResponse> future,
-                                ClientRequest jerseyRequest)
-        throws InterruptedException, ExecutionException, TimeoutException {
-        Future<?> resultFuture = apply(jerseyRequest, new AsyncConnectorCallback() {
-            @Override
-            public void response(ClientResponse response) {
-                future.complete(response);
-            }
-            @Override
-            public void failure(Throwable failure) {
-                future.completeExceptionally(failure);
-            }
-        });
-
-        Integer timeout = httpClient.getConfig().getRequestTimeout() / 3;
-
-        Object result = null;
-        if (timeout != null && timeout > 0) {
-            result = resultFuture.get(timeout, TimeUnit.MILLISECONDS);
-        } else {
-            result = resultFuture.get();
-        }
-
-        if (result != null && result instanceof Throwable) {
-            throw new ExecutionException((Throwable) result);
+    private void doNextRetry(ClientRequest jerseyRequest, AsyncConnectorCallback callback, CompletableFuture<ClientResponse> respFuture) {
+        InetSocketAddress nextTriedAddress = serviceNameResolver.resolveHost();
+        log.info("Trying to use address [{}] to send request.", nextTriedAddress.toString());
 
 Review comment:
   why is this logging level `info`?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [pulsar] yjshen commented on issue #6365: Add retry when using async method to perform request in AsyncHttpConnector

Posted by GitBox <gi...@apache.org>.

yjshen commented on issue #6365: Add retry when using async method to perform request in AsyncHttpConnector
URL: https://github.com/apache/pulsar/pull/6365#issuecomment-589922093
 
 
   /pulsarbot rerun-failure-checks

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [pulsar] tuteng commented on issue #6365: Add retry when using async method to perform request in AsyncHttpConnector

Posted by GitBox <gi...@apache.org>.

tuteng commented on issue #6365: Add retry when using async method to perform request in AsyncHttpConnector
URL: https://github.com/apache/pulsar/pull/6365#issuecomment-601988016
 
 
   This pull request is closed, so remove label release-2.5.1

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [pulsar] tuteng edited a comment on issue #6365: Add retry when using async method to perform request in AsyncHttpConnector

Posted by GitBox <gi...@apache.org>.

tuteng edited a comment on issue #6365: Add retry when using async method to perform request in AsyncHttpConnector
URL: https://github.com/apache/pulsar/pull/6365#issuecomment-601988016
 
 
   This pull request is closed not merged, so remove label release-2.5.1

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [pulsar] zymap commented on issue #6365: Add retry when using async method to perform request in AsyncHttpConnector

Posted by GitBox <gi...@apache.org>.

zymap commented on issue #6365: Add retry when using async method to perform request in AsyncHttpConnector
URL: https://github.com/apache/pulsar/pull/6365#issuecomment-595007183
 
 
   Thanks for reminder, I am trying to resolve it. This PR is caused some tests to be flaky.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [pulsar] sijie commented on a change in pull request #6365: Add retry when using async method to perform request in AsyncHttpConnector

Posted by GitBox <gi...@apache.org>.

sijie commented on a change in pull request #6365: Add retry when using async method to perform request in AsyncHttpConnector
URL: https://github.com/apache/pulsar/pull/6365#discussion_r386057412
 
 

 ##########
 File path: pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/http/AsyncHttpConnector.java
 ##########
 @@ -176,39 +153,39 @@ private URI replaceWithNew(InetSocketAddress address, URI uri) {
         return URI.create(newUri);
     }
 
+    @Override
+    public Future<?> apply(ClientRequest jerseyRequest, AsyncConnectorCallback callback) {
+        final CompletableFuture<ClientResponse> respFuture = new CompletableFuture<>();
+        triedAddressees = new HashSet<>();
+        doNextRetry(jerseyRequest, callback, respFuture);
+        return respFuture;
+    }
 
-
-    private void resolveRequest(CompletableFuture<ClientResponse> future,
-                                ClientRequest jerseyRequest)
-        throws InterruptedException, ExecutionException, TimeoutException {
-        Future<?> resultFuture = apply(jerseyRequest, new AsyncConnectorCallback() {
-            @Override
-            public void response(ClientResponse response) {
-                future.complete(response);
-            }
-            @Override
-            public void failure(Throwable failure) {
-                future.completeExceptionally(failure);
-            }
-        });
-
-        Integer timeout = httpClient.getConfig().getRequestTimeout() / 3;
-
-        Object result = null;
-        if (timeout != null && timeout > 0) {
-            result = resultFuture.get(timeout, TimeUnit.MILLISECONDS);
-        } else {
-            result = resultFuture.get();
-        }
-
-        if (result != null && result instanceof Throwable) {
-            throw new ExecutionException((Throwable) result);
+    private void doNextRetry(ClientRequest jerseyRequest, AsyncConnectorCallback callback, CompletableFuture<ClientResponse> respFuture) {
+        InetSocketAddress nextTriedAddress = serviceNameResolver.resolveHost();
+        log.info("Trying to use address [{}] to send request.", nextTriedAddress.toString());
+        if (triedAddressees.contains(nextTriedAddress)) {
+            Exception triedAllException = new ProcessingException("All addresses are tried and failed");
 
 Review comment:
   I don't think we should use this as the condition to exist. We need to check if we have tried at least multiple times, right?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [pulsar] zymap commented on issue #6365: Add retry when using async method to perform request in AsyncHttpConnector

Posted by GitBox <gi...@apache.org>.

zymap commented on issue #6365: Add retry when using async method to perform request in AsyncHttpConnector
URL: https://github.com/apache/pulsar/pull/6365#issuecomment-600395660
 
 
   This problem has fixed in the #6547 , close 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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [pulsar] tuteng edited a comment on issue #6365: Add retry when using async method to perform request in AsyncHttpConnector

Posted by GitBox <gi...@apache.org>.

tuteng edited a comment on issue #6365: Add retry when using async method to perform request in AsyncHttpConnector
URL: https://github.com/apache/pulsar/pull/6365#issuecomment-601988016
 
 
   This pull request is closed not merged , so remove label release-2.5.1

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services