Remove gossip connection limit entirely #5486
Conversation
k8s-ci-robot
added
approved
protokube/pkg/gossip/mesh/gossip.go
Outdated
| @@ -41,14 +41,16 @@ type MeshGossiper struct { | |||
|
|
|||
| func NewMeshGossiper(listen string, channelName string, nodeName string, password []byte, seeds gossip.SeedProvider) (*MeshGossiper, error) { | |||
|
|
|||
| connLimit := 64 | |||
| connLimit := 0 // 9 means no limit | |||
There was a problem hiding this comment.
Is this a typo?
9 means no limit?
This simply turns off gossip connection limits, so we shouldn't ever have to manually configure them. Follow on to kubernetes#5077
justinsb
force-pushed
the
no_gossip_limits
branch
from
281a82d to
9b70f75
Compare
| @@ -41,14 +41,16 @@ type MeshGossiper struct { | |||
|
|
|||
| func NewMeshGossiper(listen string, channelName string, nodeName string, password []byte, seeds gossip.SeedProvider) (*MeshGossiper, error) { | |||
|
|
|||
| connLimit := 64 | |||
| connLimit := 0 // 0 means no limit | |||
| gossipDnsConnLimit := os.Getenv("GOSSIP_DNS_CONN_LIMIT") | |||
There was a problem hiding this comment.
Any reason to still have the ENV var at all? Or are we expecting to have user limit the number of peers each node has in case they run into issues and the gossip pool of each node becomes too large?
|
/lgtm I’m never opposed to more config values. Considering it’s changing existing values I think we’re fine. |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: justinsb, mikesplain The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest |
This simply turns off gossip connection limits, so we shouldn't ever have to manually configure them.
Follow on to #5077