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 support for blkio.weight_device #13959

Merged
merged 1 commit into from
Nov 12, 2015

Conversation

Mashimiao
Copy link
Contributor

Signed-off-by: Ma Shimiao [email protected]

@Mashimiao
Copy link
Contributor Author

ping @LK4D4, @icecrime , @jamtur01, @calavera

@Mashimiao Mashimiao force-pushed the add-support-blkio_weight_device branch 3 times, most recently from 5a03dc4 to 383af16 Compare July 2, 2015 00:41
@jessfraz
Copy link
Contributor

jessfraz commented Jul 7, 2015

can you add an integration-cli test, other wise i see nothing wrong here

@@ -1128,6 +1128,13 @@ func (s *DockerSuite) TestRunWithBlkioInvalidWeight(c *check.C) {
}
}

func (s *DockerSuite) TestRunWithBlkioInvalidWeightDevice(c *check.C) {
Copy link
Contributor

Choose a reason for hiding this comment

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

oops my b i see this now

Copy link
Contributor

Choose a reason for hiding this comment

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

but we could use a valid one too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @jfrazelle I planted to add a valid one. But it seems it's not easy to find out what block device does the janky have.

@icecrime
Copy link
Contributor

icecrime commented Jul 8, 2015

@Mashimiao Thanks for the contrib! Please see the comments above.

@Mashimiao Mashimiao force-pushed the add-support-blkio_weight_device branch 7 times, most recently from 6cd23b8 to f51f5f1 Compare July 8, 2015 11:07
@@ -83,6 +84,9 @@ The initial status of the container created with **docker create** is 'created'.
**--blkio-weight**=0
Block IO weight (relative weight) accepts a weight value between 10 and 1000.

**--blkio-weight-device**=[]
Block IO weight (relative weight for device).
Copy link
Member

Choose a reason for hiding this comment

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

can you add (device:weight)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

@Mashimiao Mashimiao force-pushed the add-support-blkio_weight_device branch from f03555e to ecef677 Compare November 6, 2015 09:32
@@ -392,6 +394,7 @@ Return low-level information on the container `id`
"HostConfig": {
"Binds": null,
"BlkioWeight": 0,
"BlkioWeightDevice": [{}],
Copy link
Member

Choose a reason for hiding this comment

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

Not your fault, but it looks like the rest of this example uses tabs for indentation 😞

Can you change to tabs to match the other lines? (just for this instance, because we normally use spaces) ☺️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@Mashimiao Mashimiao force-pushed the add-support-blkio_weight_device branch from ecef677 to 682f892 Compare November 9, 2015 02:59
@@ -623,6 +623,7 @@ container:
| `--cpuset-mems=""` | Memory nodes (MEMs) in which to allow execution (0-3, 0,1). Only effective on NUMA systems. |
Copy link
Contributor

Choose a reason for hiding this comment

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

@Mashimiao Thank you for the contribution. You missed some tics in there and there are also some slightly awkward structs. Please move the note which states a limitation to the top of the section.

---> https://gist.github.com/moxiegirl/a7bc16b00dbdb0c45bac

Block IO bandwidth (Blkio) constraint

By default, all containers get the same proportion of block IO bandwidth
(blkio). This proportion is 500. To modify this proportion, change the
container's blkio weight relative to the weighting of all other running
containers using the --blkio-weight flag.

Note: The blkio weight setting is only available for direct IO. Buffered IO
is not currently supported.

The --blkio-weight flag takes a weight between 10 to 1000.
For example, the commands below create two containers with different blkio
weight:

    $ docker run -ti --name c1 --blkio-weight 300 ubuntu:14.04 /bin/bash
    $ docker run -ti --name c2 --blkio-weight 600 ubuntu:14.04 /bin/bash

If you do block IO in the two containers at the same time, for example:

    $ time dd if=/mnt/zerofile of=test.out bs=1M count=1024 oflag=direct

You'll find that the proportion of time is the same as the proportion of blkio
weights of the two containers.

The --blkio-weight-device="DEVICE_NAME:WEIGHT" flag sets a specific device
weight. The DEVICE_NAME:WEIGHT is a string containing a colon-separated device
name and weight. For example, to set /dev/sda device weight to 200:

$ docker run -it \
    --blkio-weight-device "/dev/sda:200" \
    ubuntu

If you specify both the --blkio-weight and --blkio-weight-device, Docker
uses the --blkio-weight as the default weight and uses --blkio-weight-device
to override with a different weight on a specific device. The following example uses a default weight of 300 and overrides this default
on /dev/sda setting with a weight of 200:

$ docker run -it \
    --blkio-weight 300 \
    --blkio-weight-device "/dev/sda:200" \
    ubuntu

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, fixed

Copy link
Member

Choose a reason for hiding this comment

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

@Mashimiao did you push your changes? I see the note is still at the bottom (havent checked other changes)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@thaJeztah sorry, I forgot to push my changes

Copy link
Member

Choose a reason for hiding this comment

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

A pfew almost thought I needed new glasses 👓 haha

Copy link
Member

Choose a reason for hiding this comment

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

erm, but I still don't see the changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@thaJeztah Sorry, forgot to modify. Already fixed

@Mashimiao Mashimiao force-pushed the add-support-blkio_weight_device branch 2 times, most recently from d3b8572 to 4a1a8e5 Compare November 11, 2015 01:24
@Mashimiao Mashimiao force-pushed the add-support-blkio_weight_device branch from 4a1a8e5 to 0fbfa14 Compare November 11, 2015 15:06
@thaJeztah
Copy link
Member

Thanks @Mashimiao!

docs LGTM

ping @moxiegirl PTAL

@moxiegirl
Copy link
Contributor

LGTM thank you @Mashimiao for your patience. @thaJeztah as you like sir!

@thaJeztah
Copy link
Member

restarting the userns and windows builds for good measure

@vdemeester
Copy link
Member

@thaJeztah it's green, merging 😉

vdemeester added a commit that referenced this pull request Nov 12, 2015
@vdemeester vdemeester merged commit 812a1c1 into moby:master Nov 12, 2015
@thaJeztah thaJeztah added this to the 1.10 milestone Dec 4, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.