Closed kenshaw closed 4 months ago
Hi,
Would expect to be able to configure this on several levels or just one global level. There are basically 3 options
Not giving it too much thought, I would expect 1 to be some default value (maybe what it currently is, but set-able by the user), and maybe allow an override in CDIConfig for a per storage class level. I suspect if you have several different storage classes they would want different settings. I would think 3 is overkill
We recently (yesterday) merged a PR that allows one to specify the file system overhead on a global and a per SC level. With a structured storage class map in CDIConfig. I think it would make sense to leverage that and extend the struct with a cache mode field which can then be passed to the various import/clone/upload pods on a per storage class basis.
I'd like to consider whether this can be a default rather than a tunable. Syncing once at the end should be safe and faster than doing it multiple times in the process of the write, which is what we're probably doing right now.
Should probably explicitly ping @aglitke to read this thread as he changed the behaviour, if he hadn't seen it yet.
so, in the case of NFS, it is always safe to not synchronize the cache, not even with a sync command at the end. NFS guarantees close-to-open consistency, so any opens done after dd closes the fd (at exit) will see the new content. I don't see why other remote file systems wouldn't promise at least that.
A regular file system on top of a remote block (XFS on ceph RBD, iSCSI, etc.) needs some extra help, because the file system layer doesn't have the information that it is remote. While a sync
only flushes all pending writes, this is sufficient because we are purely writing.
Since this is a pure RWX+Block problem, I thought about making it conditional, but a single sync
at the end is cheap enough it's probably not worth the hassle of making everything conditional.
Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale
.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close
.
/lifecycle stale
/lifecycle frozen
Do you still have performance issues after #1919?
I have not tested this; and not likely to in the next few days. As per the original issue (and as #1919 demonstrates), I still think this should be a user-configurable option as various data storage systems are going to have significantly different performance and require separate configuration/tuning on a per-cluster basis.
Created JIRA card https://issues.redhat.com/browse/CNV-26195 to handle this issue.
We don't want to expose the cache mode API to end users because understanding the implications of this setting is not trivial. We have since switched the cache mode to writeback and have a PR open to fall back to cache=none only if we detect it is needed. At this time I think the combination of these two things should be enough. I am going to close this issue since it is pretty old. Please reopen if you would like to continue discussing.
Is this a BUG REPORT or FEATURE REQUEST?:
/kind enhancement
What happened:
The
cdi-importer
command (from code inpkg/image/qemu.go
) hard codesqemu-img import
's cache option to-t none
. On a bare metal cluster, using NFSv4.1, this causesqemu-img
to wait for verification of the written data. For large imported images, this can take an extraordinary (and frustrating!) amount of time, wildly disproportionate to the expected speed of the underlying networks/nodes/storage (10Gbit/EPYC 32core/ZFS raid5, capable 500+ MB/s throughput).As per a discussion on the K8s #virtualization channel with Mike Henriksen, the option was hardcoded/added via this PR. Per Mike, this was in order to prevent issues with the imported images not being fully written prior to the termination of the importer pod.
In my (humble) opinion, this should be configurable via the CDI deployment, as the CDI user should be able to choose the risk profile they want their cluster to operate with.
Using the bare metal cluster my company has, I did a couple of tests to demonstrate the difference in speed between the various options. From a running importer instance, I grabbed the generated
qemu-img
run command, which was the following:(note that the [REDACTED] URL above is a local HTTP mirror on the same 10Gbit network)
Then, using this pod definition:
I manually executed (via
kubectl exec -it cdi-test -- /bin/bash
) the following, to show the difference in speed between the different options:What you expected to happen:
Allow configuration of
qemu-img convert
's-t
option via the CDI deployment.Ideally, it should have the following options available (copied from here):
How to reproduce it (as minimally and precisely as possible):
Do a standard installation of CDI using NFS persistent volumes, and time the import of an image.
Anything else we need to know?:
I plan on writing a PR for this in the next few days.
Environment:
kubectl get deployments cdi-deployment -o yaml
): v1.23.5kubectl version
): v1.19.2