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. It won’t diverge, but it also won’t really converge. 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. But I do think there’s quite a bit of difference in how the job behaves still... I don’t know. :-/ Max
signature.asc
Description: OpenPGP digital signature