Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

May leak memory when running controllers #729

Closed
kdada opened this issue Dec 16, 2019 · 3 comments
Closed

May leak memory when running controllers #729

kdada opened this issue Dec 16, 2019 · 3 comments
Labels
priority/awaiting-more-evidence Lowest priority. Possibly useful, but not yet enough support to actually get it done.

Comments

@kdada
Copy link
Contributor

kdada commented Dec 16, 2019

I just raise a question and does not test it.

For #424.

func (cm *controllerManager) Start(stop <-chan struct{}) error {
	...
	//  This method will start informers.
	go cm.startNonLeaderElectionRunnables()
	// lead election
	if cm.resourceLock != nil {
		err := cm.startLeaderElection()
		if err != nil {
			return err
		}
	} else {
		go cm.startLeaderElectionRunnables()
	}
	...
}

In this case, candidate controllers also can receive object events even it does not start. All events are in controller queue and no customers.

  1. It will keep deleted objects(reconcile.Request) in the queue(Of course, this case is not serious )
  2. But if users redefine the event with a pointer (or other structs without equivalence), It may cause visible leakage.
@DirectXMan12
Copy link
Contributor

I'm not sure I'm following what you're staying here

DirectXMan12 pushed a commit that referenced this issue Jan 31, 2020
📖 Update controller file path
@answer1991
Copy link
Contributor

@kdada

commit #0fdf465bc21be27b20c5b480a1aced84a3347d43 fixed your issue

@vincepri
Copy link
Member

/priority awaiting-more-evidence

@k8s-ci-robot k8s-ci-robot added the priority/awaiting-more-evidence Lowest priority. Possibly useful, but not yet enough support to actually get it done. label Feb 15, 2020
@kdada kdada closed this as completed Feb 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority/awaiting-more-evidence Lowest priority. Possibly useful, but not yet enough support to actually get it done.
Projects
None yet
Development

No branches or pull requests

5 participants