DIS-1449 add resource limits to executor config and spawn opts

[pretentious7]

No description provided.

[paulgb]

Looks good! A couple minor comments.

[paulgb]

where does mins come from as the unit? From the man page for ulimit it looks like CPU time is secs, but I can't find anything that talks about if/how docker transforms it https://ss64.com/bash/ulimit.html

[pretentious7]

I pulled that from the earlier code I think, but it was wrong

thanks for the catch.

[paulgb]

Do you know what happens if cpu_quota is Some(_) but cpu_period is None?

If Docker ignores it, I would rather that we use the default period so that someone who supplies cpu_quota gets the behavior they expect.

I am assuming that cpu_period has no effect if cpu_quota is not provided (I can't find confirmation in the docker docs). If that's not the case, we probably want to use the default only if cpu_quota is provided.

[pretentious7]

good question

after doing

docker run --cpu-quota=10000 -it --rm bash

I get:

so yea it just uses the default, but in anticipation of surprising behaviour I agree we should just pass in the default anyhow.

[pretentious7]

actually, probably less surprising if we just leave it

[paulgb]

I agree with leaving the default behaviour to Docker. Since it uses the Docker default if it is None, can we get rid of DockerCpuPeriod::default and use None instead?

[pretentious7]

That is what it's doing now. Or do you mean get rid of the default impl? I use that to calculate quota from the relative value if the period isn't specified.

[paulgb]

ahh right, nevermind

[paulgb]

LGTM!