You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2020/11/11 03:35:02 UTC

[GitHub] [apisix-dashboard] starsz opened a new issue #779: bug: EtcdV3Storage.Watch doesn't close the watchResponse chan

starsz opened a new issue #779:
URL: https://github.com/apache/apisix-dashboard/issues/779


   Please answer these questions before submitting your issue.
   
   - Why do you submit this issue?
   - [ ] Question or discussion
   - [x] Bug
   - [ ] Requirements
   - [ ] Feature or performance improvement
   - [ ] Other
   
   ___
   ### Bug
   - Which version of Apache APISIX Dashboard, OS, and Browser?
   APISIX Dashboard branch v2.0
   
   - What happened?
   If possible, provide a way to reproduce the error.
   
   Hi, I think we need to close `ch` in EtcdV3Storage.Watch function.
   If not, the groutine in GenericStore.Init() can't break the for. This called `goroutine leak` 
   
   Now:
   https://github.com/apache/apisix-dashboard/blob/e08f3abcec25811ff74d2959088d374750105f72/api/internal/core/storage/etcd.go#L108-L138
   
   Advice:
   
   ```
   func (s *EtcdV3Storage) Watch(ctx context.Context, key string) <-chan WatchResponse { 
    	eventChan := Client.Watch(ctx, key, clientv3.WithPrefix()) 
    	ch := make(chan WatchResponse, 1) 
    	go func() { 
    		for event := range eventChan { 
    			output := WatchResponse{ 
    				Canceled: event.Canceled, 
    			} 
     
    			for i := range event.Events { 
    				e := Event{ 
    					Key:   string(event.Events[i].Kv.Key), 
    					Value: string(event.Events[i].Kv.Value), 
    				} 
    				switch event.Events[i].Type { 
    				case clientv3.EventTypePut: 
    					e.Type = EventTypePut 
    				case clientv3.EventTypeDelete: 
    					e.Type = EventTypeDelete 
    				} 
    				output.Events = append(output.Events, e) 
    			} 
    			if output.Canceled { 
    				output.Error = fmt.Errorf("channel canceled") 
    			} 
    			ch <- output 
    		} 
                 
                   close(ch)
    	}() 
     
    	return ch 
    } 
   ```
   So that, the goroutine created in GenericStore.Init can exit when utils.CloseAll() is called.
   Although this problem is not very serious, we still need to fix it. 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



[GitHub] [apisix-dashboard] nic-chen commented on issue #779: bug: EtcdV3Storage.Watch doesn't close the watchResponse chan

Posted by GitBox <gi...@apache.org>.
nic-chen commented on issue #779:
URL: https://github.com/apache/apisix-dashboard/issues/779#issuecomment-726422957


   sure


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



[GitHub] [apisix-dashboard] membphis commented on issue #779: bug: EtcdV3Storage.Watch doesn't close the watchResponse chan

Posted by GitBox <gi...@apache.org>.
membphis commented on issue #779:
URL: https://github.com/apache/apisix-dashboard/issues/779#issuecomment-726196161


   @nic-chen can we close this PR?


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



[GitHub] [apisix-dashboard] nic-chen commented on issue #779: bug: EtcdV3Storage.Watch doesn't close the watchResponse chan

Posted by GitBox <gi...@apache.org>.
nic-chen commented on issue #779:
URL: https://github.com/apache/apisix-dashboard/issues/779#issuecomment-725734490


   LGTM. welcome PR !


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



[GitHub] [apisix-dashboard] nic-chen closed issue #779: bug: EtcdV3Storage.Watch doesn't close the watchResponse chan

Posted by GitBox <gi...@apache.org>.
nic-chen closed issue #779:
URL: https://github.com/apache/apisix-dashboard/issues/779


   


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



[GitHub] [apisix-dashboard] membphis edited a comment on issue #779: bug: EtcdV3Storage.Watch doesn't close the watchResponse chan

Posted by GitBox <gi...@apache.org>.
membphis edited a comment on issue #779:
URL: https://github.com/apache/apisix-dashboard/issues/779#issuecomment-726196161


   @nic-chen can we close this issue?


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