-
Notifications
You must be signed in to change notification settings - Fork 527
feat: re-enable container runtime dir relocation #3393
Conversation
…ir (Azure#3072) (Azure#3179)"""" This reverts commit 5af21fe.
Manually tested with docker + containerd since it's been a while since this was originally applied, still looks good though. I would like to avoid special case logic for handling copy to temp disk at provision time. If our target is ephemeral OS disk or using data disk for docker, both of those will use a cached VHD. This avoids increasing provisioning latency. For temp disk, we will not make it the default and so will not increase latency by default. We will only allow it as an opt-in toggle in the short/medium-term, especially for targeted SKUs like N series. These users are clear on the cost and have already been paying the runtime cost of pulling images. When these SKUs support ephemeral OS disk or AKS supports using a data disk for docker root, we would de-prioritize temp disk and ask those same customers to use ephemeral OS + data disk, which will not have the same latency issues. In short I don't think it's worth it to detect if the user placed it on "/mnt" vs a data disk because we would remove that logic in the future anyway, and prefer for it not to be used. |
I did some testing around provisioning time and using this option with a VHD (the AKS scenario). I tested provisioning time with docker on temp disk (this should trigger network pull at provision time) Standard_D8s_v3 for all tests roundtrip create/delete, 1024gb, default
roundtrip create/delete, 1024gb, reroot
100gb, default
100gb, reroot
1024gb, default
1024gb, reroot
copy from 100 GB OS disk (16 GB of data copied, this throttles the OS disk and hangs for a long time)
Seems we have an enormous cache, but pulling a few images to temp disk on fast VMs is basically negligible, and we even gain some improvements by offloading from the OS disk in the small case. For the N-series SKUs we are primarily targeting with this change, this is promising. added |
/lgtm |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: alexeldeib, xuto2 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This reverts commit 5af21fe.
Reason for Change:
Issue Fixed:
Requirements:
Notes:
not sure if there have been material changes in between that should affect this, the revert was fairly clean. doing some manual testing.