[GitHub] [dubbo-go] AlexStocks commented on a change in pull request #1010: Fix:zk too many tcp conn

[GitBox <gi...@apache.org>]

AlexStocks commented on a change in pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#discussion_r584184754



##########
File path: remoting/exchange_client.go
##########
@@ -51,6 +52,8 @@ type ExchangeClient struct {
 	client Client
 	// the tag for init.
 	init bool
+	// the number of service using the exchangeClient
+	activeNum uint32

Review comment:
       IMO, uber atomic pkg is more useful.

##########
File path: remoting/exchange_client.go
##########
@@ -87,6 +91,21 @@ func (cl *ExchangeClient) doInit(url *common.URL) error {
 	return nil
 }
 
+// increase number of service using client
+func (client *ExchangeClient) IncreaseActiveNumber() {
+	atomic.AddUint32(&client.activeNum, 1)

Review comment:
       // increase number of service using client
   func (client *ExchangeClient) IncreaseActiveNumber() uint32 {
   	      return atomic.AddUint32(&client.activeNum, 1)
   }

##########
File path: remoting/zookeeper/client.go
##########
@@ -285,6 +285,11 @@ func (z *ZookeeperClient) HandleZkEvent(session <-chan zk.Event) {
 				if state == (int)(zk.StateHasSession) {
 					continue
 				}
+				if event.State == zk.StateHasSession {
+					atomic.StoreUint32(&z.valid, 1)
+					close(z.reconnectCh)
+					z.reconnectCh = make(chan struct{})

Review comment:
       pls add a lock to promise the following clause is executed sequentially .
   
   ```go
   					close(z.reconnectCh)
   					z.reconnectCh = make(chan struct{})
   ```
   

##########
File path: remoting/exchange_client.go
##########
@@ -87,6 +91,21 @@ func (cl *ExchangeClient) doInit(url *common.URL) error {
 	return nil
 }
 
+// increase number of service using client
+func (client *ExchangeClient) IncreaseActiveNumber() {
+	atomic.AddUint32(&client.activeNum, 1)
+}
+
+// decrease number of service using client
+func (client *ExchangeClient) DecreaseActiveNumber() {

Review comment:
       return value

##########
File path: remoting/zookeeper/client.go
##########
@@ -210,7 +220,7 @@ func NewMockZookeeperClient(name string, timeout time.Duration, opts ...Option)
 		name:          name,
 		ZkAddrs:       []string{},
 		Timeout:       timeout,
-		exit:          make(chan struct{}),
+		reconnectCh:   make(chan struct{}),

Review comment:
       pls add a 'reconnectChLock' to close it safely.

##########
File path: remoting/zookeeper/client.go
##########
@@ -285,6 +285,11 @@ func (z *ZookeeperClient) HandleZkEvent(session <-chan zk.Event) {
 				if state == (int)(zk.StateHasSession) {
 					continue
 				}
+				if event.State == zk.StateHasSession {
+					atomic.StoreUint32(&z.valid, 1)
+					close(z.reconnectCh)
+					z.reconnectCh = make(chan struct{})

Review comment:
       I think u need to use a lock to promise that: 
   
   ```go
       close(z.reconnectCh)
       z.reconnectCh = make(chan struct{})
   ```




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



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org