-
Notifications
You must be signed in to change notification settings - Fork 529
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
Minimize expensive API calls #245
Open
stokito
wants to merge
13
commits into
bcicen:master
Choose a base branch
from
stokito:refresh_periodically
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Inspect call is expensive and slow but almost all fields we can extract directly from List Containers. The only one missing info for us is ENV VAR which can be retrieved only from inspect
Destroy event may be lost so users may see non existing containers. To cleanup them we have to refresh all containers periodically.
This will be more lightweight API call than inspect
When new container created event received then register the new container with basic fields: name, image. But we need to request other fields: IPs, ports, etc. To get them add the new container's id to list needsRefresh. Usually few containers are created almost simultaneously so to minimize API calls we try to make a batch update. We'll make a short delay to collect all new containers ids and only then request refresh.
# Conflicts: # connector/docker.go
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The PR is intended to improve performance and minimize expensive API calls.
Docker API has two methods:
ContainersList()
and more detailedInspect()
.The ContainersList() has almost all needed fields except of ENV variables that are shown only in Inspect() response.
So the basic idea of the PR is: what if we'll try to get all fields from ContainersList() but when user opens Single View additionally make Inspect() call to get ENV variables. This also allows us to fetch here other things e.g. #226
Currently ctop works like:
name
, registers a container and calls a separate Inspect() or each container to get all other fields.created
event and callsInspect()
to retrieve all fields and only then registers a container.But in fact the only one container field shown in main view is container's name. All other fields are shown only in Single view.
And the
created
event already contains container's name and image. So we can just register a container directly from event.But in future ctop may receive a new functionality to show other fields directly in main view (e.g. show
image
column). So let's keep the fetching of full info but use cheap ContainersList().Also the PR has additional improvements: Refresh periodically each 5 minutes to remove non existing containers and bulk refresh.