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

[refactor] merge pod watchers #3745

Closed
balopat opened this issue Feb 27, 2020 · 2 comments
Closed

[refactor] merge pod watchers #3745

balopat opened this issue Feb 27, 2020 · 2 comments
Labels
area/watch kind/tech-debt kind/todo implementation task/epic for the skaffold team

Comments

@balopat
Copy link
Contributor

balopat commented Feb 27, 2020

#3645 introduces the 3rd podWatcher based control loop in skaffold with the exact same control flow.

I would think a single podWatcher would be less resource intensive (the same "event messages" only need to be sent once for skaffold) on the API server side and on the skaffold side as well (less go routines).

I think, in a separate PR we should refactor first to a "visitor" pattern on a centralized podWatcher and make the logs, portforwards and debug listeners "register" / "unregister" with the podWatcher.

In this visitor framework the implementation of the debug listener only would require line 94-99.

Originally posted by @balopat in #3645

@balopat balopat added the kind/todo implementation task/epic for the skaffold team label Feb 28, 2020
@dgageot dgageot changed the title [refactor] merge watchers [refactor] merge pod watchers May 4, 2020
@dgageot dgageot self-assigned this May 4, 2020
@dgageot dgageot removed their assignment Jun 18, 2020
@briandealwis
Copy link
Member

@nkubala will the deploy refactoring address merging the various pod-watchers into a single instance?

@nkubala
Copy link
Contributor

nkubala commented Aug 30, 2021

I don't think this is an issue anymore. we do share one instance of a pod watcher now - there is one PodSelector per Runner instance which is passed to all sub components. the control loop logic (e.g. the calls to watcher.Select(pod)) needs to happen on a per-component basis, so I think we've done as much optimization as we can on this front.

@nkubala nkubala closed this as completed Aug 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/watch kind/tech-debt kind/todo implementation task/epic for the skaffold team
Projects
None yet
Development

No branches or pull requests

4 participants