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 2022/04/26 11:07:26 UTC

[GitHub] [pulsar-client-go] lhotari opened a new pull request, #767: Revert "Fix stuck when reconnect broker (#703)"

lhotari opened a new pull request, #767:
URL: https://github.com/apache/pulsar-client-go/pull/767

   Fixes #766 
   
   ### Motivation
   
   The change made in #703 is invalid. See #766 .
   
   One of the motivation of #703 was to implement similar behavior as there is in Java SDK. The change didn't do that.
   In the Java SDK, the concepts (classes) are fairly misleading, and something that might seem to be a connection close isn't one. The ClientHandler is really a reference to a connection in the Java SDK. The ClientCnx (which holds the TCP/IP connection) isn't closed in the Java SDK in ClientHandler.connectionClosed:
   https://github.com/apache/pulsar/blob/04aa9e8e51869d1621a7e25402a656084eebfc09/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConnectionHandler.java#L122-L141
   
   ### Modifications
   
   This reverts #703 / commit 1a8432cfd3aa231f8eb3c97171a47eab98a8f20a.
   
   
   
   


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

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar-client-go] wolfstudy commented on pull request #767: Revert "Fix stuck when reconnect broker (#703)"

Posted by GitBox <gi...@apache.org>.
wolfstudy commented on PR #767:
URL: https://github.com/apache/pulsar-client-go/pull/767#issuecomment-1120167137

   Hello @lhotari 
   
   The processing of #703 is indeed a bit rough. For scenarios where multiple topics reuse the same connection, frequent reconnection does have a big impact.
   
   The #703 originally wanted to solve the problem that the #693 could not reconnect during the reconnection process. In the Go SDK, there is currently a no good way to release the reference of the current connection object like the Java SDK, and then return the real TCP connection to the connection pool. Therefore, the method of closing the connection is temporarily used in the repair of #703.
   
   If there is a more elegant way to handle this, please feel free to communicate.


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

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar-client-go] wolfstudy commented on pull request #767: Revert "Fix stuck when reconnect broker (#703)"

Posted by GitBox <gi...@apache.org>.
wolfstudy commented on PR #767:
URL: https://github.com/apache/pulsar-client-go/pull/767#issuecomment-1134139915

   Closing the TCP connection here is indeed an inelegant operation, I merge the change first, and then use state management to fix the problem


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

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar-client-go] billowqiu commented on pull request #767: Revert "Fix stuck when reconnect broker (#703)"

Posted by GitBox <gi...@apache.org>.
billowqiu commented on PR #767:
URL: https://github.com/apache/pulsar-client-go/pull/767#issuecomment-1362616255

   > @billowqiu @zwb1234567 @jiaqingchen Would you mind explaining why you reverted this change in your fork with [billowqiu#15](https://github.com/billowqiu/pulsar-client-go/pull/15) ? It would be useful if you'd share the details so that we could improve the official Apache Pulsar Go client. /cc @wolfstudy @zzzming @pgier
   
   In some cases, not closing the connection causes the producer to enter an abnormal state in which all send requests are fail with "request timed out"


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

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar-client-go] wolfstudy merged pull request #767: Revert "Fix stuck when reconnect broker (#703)"

Posted by GitBox <gi...@apache.org>.
wolfstudy merged PR #767:
URL: https://github.com/apache/pulsar-client-go/pull/767


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

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar-client-go] wolfstudy commented on pull request #767: Revert "Fix stuck when reconnect broker (#703)"

Posted by GitBox <gi...@apache.org>.
wolfstudy commented on PR #767:
URL: https://github.com/apache/pulsar-client-go/pull/767#issuecomment-1120167328

   The #703 fix does fix the #693 's problem, but it's probably not an elegant way to do 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.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar-client-go] lhotari commented on pull request #767: Revert "Fix stuck when reconnect broker (#703)"

Posted by GitBox <gi...@apache.org>.
lhotari commented on PR #767:
URL: https://github.com/apache/pulsar-client-go/pull/767#issuecomment-1362599846

   @billowqiu @zwb1234567 @jiaqingchen Would you mind explaining why you reverted this change in your fork with https://github.com/billowqiu/pulsar-client-go/pull/15 ? It would be useful if you'd share the details so that we could improve the official Apache Pulsar Go client. 
   /cc @wolfstudy @zzzming @pgier 


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

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org