You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by GitBox <gi...@apache.org> on 2020/03/17 06:25:23 UTC

[GitHub] [servicecomb-java-chassis] GuoYL123 opened a new pull request #1638: [SCB-1803] kie support long polling

GuoYL123 opened a new pull request #1638: [SCB-1803] kie support long polling
URL: https://github.com/apache/servicecomb-java-chassis/pull/1638
 
 
   Follow this checklist to help us incorporate your contribution quickly and easily:
   
    - [ ] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/SCB) filed for the change (usually before you start working on it).  Trivial changes like typos do not require a JIRA issue.  Your pull request should address just this issue, without pulling in other changes.
    - [ ] Each commit in the pull request should have a meaningful subject line and body.
    - [ ] Format the pull request title like `[SCB-XXX] Fixes bug in ApproximateQuantiles`, where you replace `SCB-XXX` with the appropriate JIRA issue.
    - [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
    - [ ] Run `mvn clean install -Pit` to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.
    - [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
   
   ---
   

----------------------------------------------------------------
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] [servicecomb-java-chassis] GuoYL123 commented on a change in pull request #1638: [SCB-1803] kie support long polling

Posted by GitBox <gi...@apache.org>.
GuoYL123 commented on a change in pull request #1638: [SCB-1803] kie support long polling
URL: https://github.com/apache/servicecomb-java-chassis/pull/1638#discussion_r394776101
 
 

 ##########
 File path: dynamic-config/config-kie/src/main/java/org/apache/servicecomb/config/kie/client/KieClient.java
 ##########
 @@ -84,6 +101,8 @@ private void deployConfigClient() throws InterruptedException {
     Vertx vertx = VertxUtils.getOrCreateVertxByName("kie", vertxOptions);
 
     HttpClientOptions httpClientOptions = new HttpClientOptions();
+    httpClientOptions.setKeepAlive(true);
+    httpClientOptions.setIdleTimeout(SOCKET_TIMOUT);
 
 Review comment:
   I see the vert.x said :
   
   >    * Set the keep alive timeout for HTTP/1.x, in seconds.
   >    * This value determines how long a connection remains unused in the pool before being evicted and closed. 
   
   I think the default value is ok

----------------------------------------------------------------
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] [servicecomb-java-chassis] coveralls commented on issue #1638: [SCB-1803] kie support long polling

Posted by GitBox <gi...@apache.org>.
coveralls commented on issue #1638: [SCB-1803] kie support long polling
URL: https://github.com/apache/servicecomb-java-chassis/pull/1638#issuecomment-600400558
 
 
   
   [![Coverage Status](https://coveralls.io/builds/29434017/badge)](https://coveralls.io/builds/29434017)
   
   Coverage increased (+0.006%) to 84.774% when pulling **edb82d2b1565bcd366d9901ff017f2def82abf4c on GuoYL123:kie-l** into **192e4acde4554e7faaf2cf1273a5e6aa8a549e38 on apache:master**.
   

----------------------------------------------------------------
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] [servicecomb-java-chassis] liubao68 commented on a change in pull request #1638: [SCB-1803] kie support long polling

Posted by GitBox <gi...@apache.org>.
liubao68 commented on a change in pull request #1638: [SCB-1803] kie support long polling
URL: https://github.com/apache/servicecomb-java-chassis/pull/1638#discussion_r393554469
 
 

 ##########
 File path: dynamic-config/config-kie/src/main/java/org/apache/servicecomb/config/kie/client/KieClient.java
 ##########
 @@ -48,16 +49,27 @@
 
   private static final Logger LOGGER = LoggerFactory.getLogger(KieClient.class);
 
-  private ScheduledExecutorService EXECUTOR = Executors.newScheduledThreadPool(1);
+  private ScheduledExecutorService EXECUTOR = Executors.newScheduledThreadPool(1, (r) -> {
+    Thread thread = new Thread(r);
+    thread.setName("org.apache.servicecomb.config.kie");
+    thread.setDaemon(true);
+    return thread;
+  });
 
   private static final long TIME_OUT = 10000;
 
+  private static AtomicBoolean IS_FIRST_PULL = new AtomicBoolean(true);
+
+  private static final int LONG_POLLING_WAIT_TIME = 30;
 
 Review comment:
   long polling timeout is equals to client request timeout, will this cause client timeout frequently?

----------------------------------------------------------------
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] [servicecomb-java-chassis] coveralls edited a comment on issue #1638: [SCB-1803] kie support long polling

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #1638: [SCB-1803] kie support long polling
URL: https://github.com/apache/servicecomb-java-chassis/pull/1638#issuecomment-600400558
 
 
   
   [![Coverage Status](https://coveralls.io/builds/29434080/badge)](https://coveralls.io/builds/29434080)
   
   Coverage increased (+0.004%) to 84.771% when pulling **edb82d2b1565bcd366d9901ff017f2def82abf4c on GuoYL123:kie-l** into **192e4acde4554e7faaf2cf1273a5e6aa8a549e38 on apache:master**.
   

----------------------------------------------------------------
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] [servicecomb-java-chassis] GuoYL123 commented on a change in pull request #1638: [SCB-1803] kie support long polling

Posted by GitBox <gi...@apache.org>.
GuoYL123 commented on a change in pull request #1638: [SCB-1803] kie support long polling
URL: https://github.com/apache/servicecomb-java-chassis/pull/1638#discussion_r394077504
 
 

 ##########
 File path: dynamic-config/config-kie/src/main/java/org/apache/servicecomb/config/kie/client/KieClient.java
 ##########
 @@ -140,23 +160,23 @@ void refreshConfig() {
                         e.getMessage());
                   }
                 });
-                // latch.countDown();
               } else if (rsp.statusCode() == HttpStatus.SC_NOT_FOUND) {
                 EventManager.post(new ConnSuccEvent());
-//                latch.countDown();
               } else {
                 EventManager.post(new ConnFailEvent("fetch config fail"));
                 LOGGER.error("Config update from {} failed. Error message is [{}].",
                     serviceUri,
                     rsp.statusMessage());
               }
+              if (enableLongPolling) {
 
 Review comment:
   first time also need this code 

----------------------------------------------------------------
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] [servicecomb-java-chassis] liubao68 merged pull request #1638: [SCB-1803] kie support long polling

Posted by GitBox <gi...@apache.org>.
liubao68 merged pull request #1638: [SCB-1803] kie support long polling
URL: https://github.com/apache/servicecomb-java-chassis/pull/1638
 
 
   

----------------------------------------------------------------
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] [servicecomb-java-chassis] coveralls edited a comment on issue #1638: [SCB-1803] kie support long polling

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #1638: [SCB-1803] kie support long polling
URL: https://github.com/apache/servicecomb-java-chassis/pull/1638#issuecomment-600400558
 
 
   
   [![Coverage Status](https://coveralls.io/builds/29434041/badge)](https://coveralls.io/builds/29434041)
   
   Coverage increased (+0.001%) to 84.769% when pulling **edb82d2b1565bcd366d9901ff017f2def82abf4c on GuoYL123:kie-l** into **192e4acde4554e7faaf2cf1273a5e6aa8a549e38 on apache:master**.
   

----------------------------------------------------------------
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] [servicecomb-java-chassis] GuoYL123 commented on a change in pull request #1638: [SCB-1803] kie support long polling

Posted by GitBox <gi...@apache.org>.
GuoYL123 commented on a change in pull request #1638: [SCB-1803] kie support long polling
URL: https://github.com/apache/servicecomb-java-chassis/pull/1638#discussion_r394774783
 
 

 ##########
 File path: dynamic-config/config-kie/src/main/java/org/apache/servicecomb/config/kie/client/KieClient.java
 ##########
 @@ -84,6 +101,8 @@ private void deployConfigClient() throws InterruptedException {
     Vertx vertx = VertxUtils.getOrCreateVertxByName("kie", vertxOptions);
 
     HttpClientOptions httpClientOptions = new HttpClientOptions();
+    httpClientOptions.setKeepAlive(true);
+    httpClientOptions.setIdleTimeout(SOCKET_TIMOUT);
 
 Review comment:
    keepAliveTimeout default is 60s

----------------------------------------------------------------
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] [servicecomb-java-chassis] liubao68 commented on a change in pull request #1638: [SCB-1803] kie support long polling

Posted by GitBox <gi...@apache.org>.
liubao68 commented on a change in pull request #1638: [SCB-1803] kie support long polling
URL: https://github.com/apache/servicecomb-java-chassis/pull/1638#discussion_r393557036
 
 

 ##########
 File path: dynamic-config/config-kie/src/main/java/org/apache/servicecomb/config/kie/client/KieClient.java
 ##########
 @@ -114,17 +129,22 @@ public void run() {
       }
     }
 
-    //todo : latch down
     @SuppressWarnings("deprecation")
     void refreshConfig() {
       String path = "/v1/"
           + KieConfig.INSTANCE.getDomainName()
           + "/kie/kv?label=app:"
           + KieConfig.INSTANCE.getAppName();
+      if (!IS_FIRST_PULL.get()) {
 
 Review comment:
   Maybe need check enableLongPolling?

----------------------------------------------------------------
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] [servicecomb-java-chassis] liubao68 commented on issue #1638: [SCB-1803] kie support long polling

Posted by GitBox <gi...@apache.org>.
liubao68 commented on issue #1638: [SCB-1803] kie support long polling
URL: https://github.com/apache/servicecomb-java-chassis/pull/1638#issuecomment-600952236
 
 
   I added a sub task for documentation: https://issues.apache.org/jira/browse/SCB-1807

----------------------------------------------------------------
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] [servicecomb-java-chassis] GuoYL123 commented on a change in pull request #1638: [SCB-1803] kie support long polling

Posted by GitBox <gi...@apache.org>.
GuoYL123 commented on a change in pull request #1638: [SCB-1803] kie support long polling
URL: https://github.com/apache/servicecomb-java-chassis/pull/1638#discussion_r394077313
 
 

 ##########
 File path: dynamic-config/config-kie/src/main/java/org/apache/servicecomb/config/kie/client/KieClient.java
 ##########
 @@ -114,17 +129,22 @@ public void run() {
       }
     }
 
-    //todo : latch down
     @SuppressWarnings("deprecation")
     void refreshConfig() {
       String path = "/v1/"
           + KieConfig.INSTANCE.getDomainName()
           + "/kie/kv?label=app:"
           + KieConfig.INSTANCE.getAppName();
+      if (!IS_FIRST_PULL.get()) {
 
 Review comment:
   done

----------------------------------------------------------------
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] [servicecomb-java-chassis] liubao68 commented on a change in pull request #1638: [SCB-1803] kie support long polling

Posted by GitBox <gi...@apache.org>.
liubao68 commented on a change in pull request #1638: [SCB-1803] kie support long polling
URL: https://github.com/apache/servicecomb-java-chassis/pull/1638#discussion_r393554469
 
 

 ##########
 File path: dynamic-config/config-kie/src/main/java/org/apache/servicecomb/config/kie/client/KieClient.java
 ##########
 @@ -48,16 +49,27 @@
 
   private static final Logger LOGGER = LoggerFactory.getLogger(KieClient.class);
 
-  private ScheduledExecutorService EXECUTOR = Executors.newScheduledThreadPool(1);
+  private ScheduledExecutorService EXECUTOR = Executors.newScheduledThreadPool(1, (r) -> {
+    Thread thread = new Thread(r);
+    thread.setName("org.apache.servicecomb.config.kie");
+    thread.setDaemon(true);
+    return thread;
+  });
 
   private static final long TIME_OUT = 10000;
 
+  private static AtomicBoolean IS_FIRST_PULL = new AtomicBoolean(true);
+
+  private static final int LONG_POLLING_WAIT_TIME = 30;
 
 Review comment:
   long polling timeout is less than client request timeout, will this cause client timeout frequently?

----------------------------------------------------------------
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] [servicecomb-java-chassis] liubao68 commented on a change in pull request #1638: [SCB-1803] kie support long polling

Posted by GitBox <gi...@apache.org>.
liubao68 commented on a change in pull request #1638: [SCB-1803] kie support long polling
URL: https://github.com/apache/servicecomb-java-chassis/pull/1638#discussion_r393557608
 
 

 ##########
 File path: dynamic-config/config-kie/src/main/java/org/apache/servicecomb/config/kie/client/KieClient.java
 ##########
 @@ -140,23 +160,23 @@ void refreshConfig() {
                         e.getMessage());
                   }
                 });
-                // latch.countDown();
               } else if (rsp.statusCode() == HttpStatus.SC_NOT_FOUND) {
                 EventManager.post(new ConnSuccEvent());
-//                latch.countDown();
               } else {
                 EventManager.post(new ConnFailEvent("fetch config fail"));
                 LOGGER.error("Config update from {} failed. Error message is [{}].",
                     serviceUri,
                     rsp.statusMessage());
               }
+              if (enableLongPolling) {
 
 Review comment:
   Maybe need check if first call?

----------------------------------------------------------------
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] [servicecomb-java-chassis] liubao68 commented on a change in pull request #1638: [SCB-1803] kie support long polling

Posted by GitBox <gi...@apache.org>.
liubao68 commented on a change in pull request #1638: [SCB-1803] kie support long polling
URL: https://github.com/apache/servicecomb-java-chassis/pull/1638#discussion_r394752163
 
 

 ##########
 File path: dynamic-config/config-kie/src/main/java/org/apache/servicecomb/config/kie/client/KieClient.java
 ##########
 @@ -84,6 +101,8 @@ private void deployConfigClient() throws InterruptedException {
     Vertx vertx = VertxUtils.getOrCreateVertxByName("kie", vertxOptions);
 
     HttpClientOptions httpClientOptions = new HttpClientOptions();
+    httpClientOptions.setKeepAlive(true);
+    httpClientOptions.setIdleTimeout(SOCKET_TIMOUT);
 
 Review comment:
   Please look at RestTransportClient to setting HttpClientOptions, if keepAliveTimeout not set, IdelTimeout may not work as expected. SOCKET_TIMEOUT should better be IdleTimeout and maybe better add a configuration for 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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [servicecomb-java-chassis] GuoYL123 commented on a change in pull request #1638: [SCB-1803] kie support long polling

Posted by GitBox <gi...@apache.org>.
GuoYL123 commented on a change in pull request #1638: [SCB-1803] kie support long polling
URL: https://github.com/apache/servicecomb-java-chassis/pull/1638#discussion_r394077261
 
 

 ##########
 File path: dynamic-config/config-kie/src/main/java/org/apache/servicecomb/config/kie/client/KieClient.java
 ##########
 @@ -48,16 +49,27 @@
 
   private static final Logger LOGGER = LoggerFactory.getLogger(KieClient.class);
 
-  private ScheduledExecutorService EXECUTOR = Executors.newScheduledThreadPool(1);
+  private ScheduledExecutorService EXECUTOR = Executors.newScheduledThreadPool(1, (r) -> {
+    Thread thread = new Thread(r);
+    thread.setName("org.apache.servicecomb.config.kie");
+    thread.setDaemon(true);
+    return thread;
+  });
 
   private static final long TIME_OUT = 10000;
 
+  private static AtomicBoolean IS_FIRST_PULL = new AtomicBoolean(true);
+
+  private static final int LONG_POLLING_WAIT_TIME = 30;
 
 Review comment:
   After checking , vert.x default strategy is never timeout. I set the idle timeout 50s.

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