On 04.10.19 17:04, Vladimir Sementsov-Ogievskiy wrote: > 04.10.2019 17:48, Max Reitz wrote: >> On 04.10.19 15:22, Vladimir Sementsov-Ogievskiy wrote: >>> 04.10.2019 15:59, Max Reitz wrote: >>>> On 03.10.19 11:34, Vladimir Sementsov-Ogievskiy wrote: >>>>> 02.10.2019 18:52, Max Reitz wrote: >>>>>> On 02.10.19 17:06, Vladimir Sementsov-Ogievskiy wrote: >>>>>>> 02.10.2019 18:03, Vladimir Sementsov-Ogievskiy wrote: >>>>>>>> 02.10.2019 17:57, Max Reitz wrote: >>>>>>>>> On 12.09.19 17:13, Vladimir Sementsov-Ogievskiy wrote: >>>>>>>>>> Prior 9adc1cb49af8d do_sync_target_write had a bug: it reset >>>>>>>>>> aligned-up >>>>>>>>>> region in the dirty bitmap, which means that we may not copy some >>>>>>>>>> bytes >>>>>>>>>> and assume them copied, which actually leads to producing corrupted >>>>>>>>>> target. >>>>>>>>>> >>>>>>>>>> So 9adc1cb49af8d forced dirty bitmap granularity to be >>>>>>>>>> request_alignment for mirror-top filter, so we are not working with >>>>>>>>>> unaligned requests. However forcing large alignment obviously >>>>>>>>>> decreases >>>>>>>>>> performance of unaligned requests. >>>>>>>>>> >>>>>>>>>> This commit provides another solution for the problem: if unaligned >>>>>>>>>> padding is already dirty, we can safely ignore it, as >>>>>>>>>> 1. It's dirty, it will be copied by mirror_iteration anyway >>>>>>>>>> 2. It's dirty, so skipping it now we don't increase dirtiness of the >>>>>>>>>> bitmap and therefore don't damage "synchronicity" of the >>>>>>>>>> write-blocking mirror. >>>>>>>>> >>>>>>>>> But that’s not what active mirror is for. The point of active mirror >>>>>>>>> is >>>>>>>>> that it must converge because every guest write will contribute >>>>>>>>> towards >>>>>>>>> that goal. >>>>>>>>> >>>>>>>>> If you skip active mirroring for unaligned guest writes, they will not >>>>>>>>> contribute towards converging, but in fact lead to the opposite. >>>>>>>>> >>>>>>>> >>>>>>>> The will not contribute only if region is already dirty. Actually, >>>>>>>> after >>>>>>>> first iteration of mirroring (copying the whole disk), all following >>>>>>>> writes >>>>>>>> will contribute, so the whole process must converge. It is a bit >>>>>>>> similar with >>>>>>>> running one mirror loop in normal mode, and then enable write-blocking. >>>>>>>> >>>>>>> >>>>>>> >>>>>>> In other words, we don't need "all guest writes contribute" to converge, >>>>>>> "all guest writes don't create new dirty bits" is enough, as we have >>>>>>> parallel >>>>>>> mirror iteration which contiguously handles dirty bits. >>>>>> >>>>>> Hm, in a sense. But it does mean that guest writes will not contribute >>>>>> to convergence. >>>>>> >>>>>> And that’s against the current definition of write-blocking, which does >>>>>> state that “when data is written to the source, write it (synchronously) >>>>>> to the target as well”. >>>>>> >>>>> >>>>> Hmm, understand. But IMHO our proposed behavior is better in general. >>>>> Do you think it's a problem to change spec now? >>>>> If yes, I'll resend with an option >>>> >>>> Well, the thing is that I’d find it weird if write-blocking wasn’t >>>> blocking in all cases. And in my opinion, it makes more sense for >>>> active mirror if all writes actively contributed to convergence. >>>> >>> >>> Why? What is the benefit in it? >>> What is "all writes actively contributed to convergence" for user? >> >> One thing I wonder about is whether it’s really guaranteed that the >> background job will run under full I/O load, and how often it runs. >> >> I fear that with your model, the background job might starve and the >> mirror may take a very long time. > > Hmmm. I think mirror job is in same context as guest writes, and goes > through same IO api.. Why it will starve? (I understand that my words > are not an evidence...).
I thought that maybe if the disk is read to write and write all the time, there’d be no time for the mirror coroutine. >> It won’t diverge, but it also won’t >> really converge. > > But same will be with current behavior: guest is not guaranteed to write > to all parts of disk. And in most scenarios it doesn't. So, if mirror job > starve because of huge guest IO load, we will not converge anyway. > > So, background process is necessary thing for converge anyway. Good point. That convinces me. >> The advantage of letting all writes block is that even under full I/O >> load, the mirror job will progress at a steady pace. >> >>> I think for user there may be the following criteria: >>> >>> 1. guaranteed converge, with any guest write load. >>> Both current and my proposed variants are OK. >>> >>> 2. Less impact on guest. >>> Obviously my proposed variant is better >>> >>> 3. Total time of mirroring >>> Seems, current may be a bit better, but I don't think that unaligned >>> tails gives significant impact. >>> >>> === >>> >>> So, assume I want [1]+[2]. And possibly >>> 2.2: Even less impact on guest: ignore not only unaligned tails if they are >>> already dirty, but full synchronous mirror operation if area is already >>> dirty. >>> >>> How should I call this? Should it be separate mode, or option for >>> write-blocking? >> >> I don’t know whether it makes sense to add a separate mode or a separate >> option just for this difference. I don’t think anyone would choose the >> non-default option. > > At least Virtuozzo will choose :) But if Virtuozzo knows exactly what to choose, then we can just implement only that behavior. Max
signature.asc
Description: OpenPGP digital signature