[GitHub] incubator-rocketmq issue #95: [ROCKETMQ-184]-It takes too long(3-33 seconds)...
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/95 @zhouxinyu @vongosling @shroman what's your advice, can this pr be merged? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #95: [ROCKETMQ-184]-It takes too long(3-33 seconds)...
Github user dongeforever commented on the issue: https://github.com/apache/incubator-rocketmq/pull/95 LGTM @zhouxinyu --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #95: [ROCKETMQ-184]-It takes too long(3-33 seconds)...
Github user lizhanhui commented on the issue: https://github.com/apache/incubator-rocketmq/pull/95 +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #95: [ROCKETMQ-184]-It takes too long(3-33 seconds)...
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/95 @lizhanhui @vsair @zhouxinyu @shroman any ideas for this updated solution? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #95: [ROCKETMQ-184]-It takes too long(3-33 seconds)...
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/95 @lizhanhui It has been refactored using the close event handler mechanism according to your advice, please review the pr from `close` callback in `NettyRemotingClient.java` --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #95: [ROCKETMQ-184]-It takes too long(3-33 seconds)...
Github user lizhanhui commented on the issue: https://github.com/apache/incubator-rocketmq/pull/95 > please review the updated pr Looks this PR is not updated. Do you forget to push your changes? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #95: [ROCKETMQ-184]-It takes too long(3-33 seconds)...
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/95 @zhouxinyu @lizhanhui please review the updated pr --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #95: [ROCKETMQ-184]-It takes too long(3-33 seconds)...
Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/95 [![Coverage Status](https://coveralls.io/builds/11211182/badge)](https://coveralls.io/builds/11211182) Coverage increased (+0.04%) to 37.894% when pulling **d8faa3364945c4f119e8fea0626183113bc45a80 on Jaskey:ROCKETMQ-184-slave-switch** into **6a9628b3c3e6835e37baf7b58ad9300364d4d384 on apache:develop**. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #95: [ROCKETMQ-184]-It takes too long(3-33 seconds)...
Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/95 [![Coverage Status](https://coveralls.io/builds/11210617/badge)](https://coveralls.io/builds/11210617) Coverage decreased (-0.09%) to 37.761% when pulling **abd61c8aa1b6e9dd583dd3e018286b778d7acab1 on Jaskey:ROCKETMQ-184-slave-switch** into **6a9628b3c3e6835e37baf7b58ad9300364d4d384 on apache:develop**. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #95: [ROCKETMQ-184]-It takes too long(3-33 seconds)...
Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/95 [![Coverage Status](https://coveralls.io/builds/11210617/badge)](https://coveralls.io/builds/11210617) Coverage decreased (-0.09%) to 37.761% when pulling **abd61c8aa1b6e9dd583dd3e018286b778d7acab1 on Jaskey:ROCKETMQ-184-slave-switch** into **6a9628b3c3e6835e37baf7b58ad9300364d4d384 on apache:develop**. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #95: [ROCKETMQ-184]-It takes too long(3-33 seconds)...
Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/95 [![Coverage Status](https://coveralls.io/builds/11210617/badge)](https://coveralls.io/builds/11210617) Coverage decreased (-0.09%) to 37.761% when pulling **abd61c8aa1b6e9dd583dd3e018286b778d7acab1 on Jaskey:ROCKETMQ-184-slave-switch** into **6a9628b3c3e6835e37baf7b58ad9300364d4d384 on apache:develop**. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #95: [ROCKETMQ-184]-It takes too long(3-33 seconds)...
Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/95 [![Coverage Status](https://coveralls.io/builds/11210574/badge)](https://coveralls.io/builds/11210574) Coverage increased (+0.004%) to 37.858% when pulling **abd61c8aa1b6e9dd583dd3e018286b778d7acab1 on Jaskey:ROCKETMQ-184-slave-switch** into **6a9628b3c3e6835e37baf7b58ad9300364d4d384 on apache:develop**. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #95: [ROCKETMQ-184]-It takes too long(3-33 seconds)...
Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/95 [![Coverage Status](https://coveralls.io/builds/11210427/badge)](https://coveralls.io/builds/11210427) Coverage increased (+0.02%) to 37.869% when pulling **40d77eaffe64cf4e3070f5e2440e7d0fa4281a0e on Jaskey:ROCKETMQ-184-slave-switch** into **6a9628b3c3e6835e37baf7b58ad9300364d4d384 on apache:develop**. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #95: [ROCKETMQ-184]-It takes too long(3-33 seconds)...
Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/95 [![Coverage Status](https://coveralls.io/builds/11210427/badge)](https://coveralls.io/builds/11210427) Coverage increased (+0.02%) to 37.869% when pulling **40d77eaffe64cf4e3070f5e2440e7d0fa4281a0e on Jaskey:ROCKETMQ-184-slave-switch** into **6a9628b3c3e6835e37baf7b58ad9300364d4d384 on apache:develop**. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #95: [ROCKETMQ-184]-It takes too long(3-33 seconds)...
Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/95 [![Coverage Status](https://coveralls.io/builds/11210427/badge)](https://coveralls.io/builds/11210427) Coverage increased (+0.02%) to 37.869% when pulling **40d77eaffe64cf4e3070f5e2440e7d0fa4281a0e on Jaskey:ROCKETMQ-184-slave-switch** into **6a9628b3c3e6835e37baf7b58ad9300364d4d384 on apache:develop**. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #95: [ROCKETMQ-184]-It takes too long(3-33 seconds)...
Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/95 [![Coverage Status](https://coveralls.io/builds/11210353/badge)](https://coveralls.io/builds/11210353) Coverage increased (+0.1%) to 37.974% when pulling **388e88bfc732ab67c62e510a24f72cf9acf50051 on Jaskey:ROCKETMQ-184-slave-switch** into **6a9628b3c3e6835e37baf7b58ad9300364d4d384 on apache:develop**. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #95: [ROCKETMQ-184]-It takes too long(3-33 seconds)...
Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/95 [![Coverage Status](https://coveralls.io/builds/11210353/badge)](https://coveralls.io/builds/11210353) Coverage increased (+0.1%) to 37.974% when pulling **388e88bfc732ab67c62e510a24f72cf9acf50051 on Jaskey:ROCKETMQ-184-slave-switch** into **6a9628b3c3e6835e37baf7b58ad9300364d4d384 on apache:develop**. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #95: [ROCKETMQ-184]-It takes too long(3-33 seconds)...
Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/95 [![Coverage Status](https://coveralls.io/builds/11210353/badge)](https://coveralls.io/builds/11210353) Coverage increased (+0.1%) to 37.974% when pulling **388e88bfc732ab67c62e510a24f72cf9acf50051 on Jaskey:ROCKETMQ-184-slave-switch** into **6a9628b3c3e6835e37baf7b58ad9300364d4d384 on apache:develop**. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #95: [ROCKETMQ-184]-It takes too long(3-33 seconds)...
Github user zhouxinyu commented on the issue: https://github.com/apache/incubator-rocketmq/pull/95 Thanks @Jaskey , agree with @lizhanhui , use NettyConnectManageHandler to handle this close event is a better way, no need to import two mechanisms. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #95: [ROCKETMQ-184]-It takes too long(3-33 seconds)...
Github user lizhanhui commented on the issue: https://github.com/apache/incubator-rocketmq/pull/95 You are right that more changes are required for my suggested approach. But, IMO, the suggested way is more unified in design and may also save a few memory footprint in case we have very large semaphore initial capacity. Indeed, this is the place we need to enhance. Let's bring more guys into discussion before you implement the suggested approach. They should easily conceive what's going on here via checking changes made in your PR. Any opinion on this issue? @zhouxinyu @shroman @vongosling --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #95: [ROCKETMQ-184]-It takes too long(3-33 seconds)...
Github user Jaskey commented on the issue: https://github.com/apache/incubator-rocketmq/pull/95 @lizhanhui I have considered that, which will takes more efforts to achieve the same goal. Since I need to change some structure to make the connect manager to get access to the responseFuture map. For my first implementation, I just want to issue this problem and involve you guys to discuss. If you think that is indeed a better approach, I will submit an updated implementations for that, and then let all guys to choose. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #95: [ROCKETMQ-184]-It takes too long(3-33 seconds)...
Github user lizhanhui commented on the issue: https://github.com/apache/incubator-rocketmq/pull/95 Thanks @Jaskey, this is indeed a good place to improve. For the implementation, I suggest an alternative generic way, instead of add a close future for each request, we add the opaque integer into a collection per channel. Remove the opaque integer on response or invalidate all of them in NettyConnectManageHandler. Suggested approach has fewer memory footprint and we may also easily cover the sync request scenario -- respond earlier before `timeoutMillis` amount of time elapsed in case channel experiences problems. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #95: [ROCKETMQ-184]-It takes too long(3-33 seconds)...
Github user lizhanhui commented on the issue: https://github.com/apache/incubator-rocketmq/pull/95 invokeOneway should also be updated if so, otherwise, it's likely to leak semaphore. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #95: [ROCKETMQ-184]-It takes too long(3-33 seconds)...
Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/95 [![Coverage Status](https://coveralls.io/builds/11176385/badge)](https://coveralls.io/builds/11176385) Coverage decreased (-0.0007%) to 34.631% when pulling **65df26a3d38a1a32814b85c9c02d95c63dec on Jaskey:ROCKETMQ-184-slave-switch** into **42f78c281cbeb5072b04eaf03b1a8059b8d281a7 on apache:develop**. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #95: [ROCKETMQ-184]-It takes too long(3-33 seconds)...
Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/95 [![Coverage Status](https://coveralls.io/builds/11165283/badge)](https://coveralls.io/builds/11165283) Coverage increased (+0.04%) to 34.666% when pulling **0dc37e1c10450f143e58faa857f5bf8ccbee1ca3 on Jaskey:ROCKETMQ-184-slave-switch** into **42f78c281cbeb5072b04eaf03b1a8059b8d281a7 on apache:develop**. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---