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

Add host and container info to DockerInfo API #1091 #2607

Closed
wants to merge 2 commits into from

Conversation

chancez
Copy link
Contributor

@chancez chancez commented Nov 8, 2013

This PR addresses issue #1091 and adds the following information to the DockerInfo API endpoint and the docker info command:

  • Number of running containers
  • Memory usage of running containers
  • Max memory usage of all containers (combined memlimits)
  • Number of Logical CPUs on the server
  • 1, 5, and 15 minute CPU Averages of the server
  • Free RAM on the server
  • Total RAM on the server

It's probably rough around the edges, so I would really appreciate any feedback.

YB
)

func (b ByteSize) String() string {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have a HumanSize function. Can you either use it or remove it? (I like your implementation)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ping @ecnahc515

@jpetazzo
Copy link
Contributor

jpetazzo commented Nov 8, 2013

I like the idea, but here is some feedback, based on our experience when designing and operating the dotCloud PAAS (which is also based on LXC containers):

  • in the beginning, you expose "simple" metrics, like used and free memory, for containers and hosts
  • after a while, you realize that those metrics are broken, because you have to refine used memory between RSS, kernel slabs and other buffers, page cache (and the last one is almost as good as free, but not quite)
  • then you realize that you have the same problem with container memory, when you notice that this tiny Nginx container is "using" 4 GB of RAM for no reason (but when you drill down, you realize that most of it is in inactive page cache memory)
  • ... so you end up with many different definitions of "used" and "free", and in our case, we decided to expose the low level metrics (the things you see in memory.stat) to let the tools above (dashboard, autoscaling...) make the best decision.

I wrote a blog post about container metrics if you want more info about that topic; and for a more "end-user oriented" view, there is also a post on the dotCloud blog.

IMHO, the Docker API should expose things that are unequivocal, I mean non ambiguous, and available on all platforms; i.e.:

  • number of CPUs
  • load average
  • total memory and swap on the server
  • number of running containers
  • total number of containers (including stopped)
  • total memory and cpu quotas for running containers
  • total memory and cpu quotas for all containers (including stopped)

"Memory and CPU quotas" here means whatever was given to Docker with -m and -c.

In addition to that, it would make total sense to expose additional metrics, but in a kind of free format.
For instance, in addition to the "universal" format (i.e. compatible even with other backends, if we add support for OpenVZ, Jails, whatever...), there could be a JSON dict giving extra host info (typically the content of /proc/meminfo) and per-container info (typically the content of memory.stat for the container cgroup).

What do others think? /cc @shykes @vieux @crosbymichael

@chancez
Copy link
Contributor Author

chancez commented Nov 8, 2013

@jpetazzo I felt this way while working on a lot of this, definitely was having trouble deciding what 'used' memory consists of when reading the cgroup info.

I'll be glad to change this to return only non-ambiguous data, and then add additional more detailed (possibly ambiguous) data in another area.

@shykes
Copy link
Contributor

shykes commented Nov 12, 2013

@ecnahc515 could you please make sure public types and methods are documented with a comment on top?

Thanks.

@chancez
Copy link
Contributor Author

chancez commented Nov 14, 2013

@vieux I was going to refactor some of this in response to the suggestions @jpetazzo made. @shykes I'll be sure to do that when I get back on this.

It's been a busy week, but I'll be finishing this up in the next couple days I hope.

@pnasrat
Copy link
Contributor

pnasrat commented Dec 3, 2013

It'd be great if you could use http://golang.org/pkg/expvar/ for the key value pairs, but I can follow up after this goes in to make that happen.

@vieux
Copy link
Contributor

vieux commented Dec 4, 2013

@pnasrat it's in :)

@vieux
Copy link
Contributor

vieux commented Dec 6, 2013

@ecnahc515 could you please rebase

@chancez
Copy link
Contributor Author

chancez commented Dec 6, 2013

Sorry its finals week, been a bit busy. I can rebase it, but I still haven't gotten to fixing the things mentioned. This could be closed until I get the chance to make the requested changes if you prefer, and I could reopen a new one.

@pnasrat
Copy link
Contributor

pnasrat commented Dec 6, 2013

If you don't have time I can base off you're initial pull request and make
the changes.

Paul

On 6 December 2013 01:12, Chance Zibolski notifications@github.com wrote:

Sorry its finals week, been a bit busy. I can rebase it, but I still
haven't gotten to fixing the things mentioned. This could be closed until I
get the chance to make the requested changes if you prefer, and I could
reopen a new one.


Reply to this email directly or view it on GitHubhttps://github.com//pull/2607#issuecomment-29965791
.

@chancez
Copy link
Contributor Author

chancez commented Dec 8, 2013

That would be fine. I'll have to check where this is later as I'd like to work more on making metric data for docker available through the API.

@vieux
Copy link
Contributor

vieux commented Dec 16, 2013

@ecnahc515 could you please rebase this ?

Shows number of running/total containers, system info (CPU, RAM, ect), and
container memory usage.
@creack
Copy link
Contributor

creack commented Jan 3, 2014

@ecnahc515 Can you please rebase and address the comments? Let me know if you need help.

@pnasrat
Copy link
Contributor

pnasrat commented Jan 3, 2014

I'll do this but I'd like to get pull #3310 in first as I'll move the cgroup specific stuff under there.

@creack
Copy link
Contributor

creack commented Jan 13, 2014

ping @pnasrat. #3310 has been merge. Can you confirm you take this?

@pnasrat
Copy link
Contributor

pnasrat commented Jan 13, 2014

I confirm I will do this, the last week has been very busy at the day job

  • I'll have some time this weekend to do this.

On 13 January 2014 13:34, Guillaume J. Charmes notifications@github.comwrote:

ping @pnasrat https://github.com/pnasrat. #3310https://github.com/dotcloud/docker/pull/3310as been merge. Can you confirm you take this?


Reply to this email directly or view it on GitHubhttps://github.com//pull/2607#issuecomment-32196866
.

@vieux
Copy link
Contributor

vieux commented Jan 24, 2014

ping @pnasrat still on this one ?

@pnasrat
Copy link
Contributor

pnasrat commented Jan 24, 2014

Yeah there was some flux but I've made a start at porting this, I've some ideas to make it cleaner and faster though. If I don't get it done in the next week I'll have to come back to after my vacation.

@vieux
Copy link
Contributor

vieux commented Jan 24, 2014

OK, thanks for the update
On Jan 24, 2014 6:08 AM, "Paul Nasrat" notifications@github.com wrote:

Yeah there was some flux but I've made a start at porting this, I've some
ideas to make it cleaner and faster though. If I don't get it done in the
next week I'll have to come back to after my vacation.


Reply to this email directly or view it on GitHubhttps://github.com//pull/2607#issuecomment-33224826
.

@vieux
Copy link
Contributor

vieux commented Feb 17, 2014

@pnasrat how was your trip ? :)

@crosbymichael
Copy link
Contributor

I'm going to close this for now because we havn't had any progress in a while. It can be reopened when ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants