On 12/13/21 20:56, Dumitru Ceara wrote: > On 12/13/21 20:07, Ilya Maximets wrote: >> On 12/13/21 19:45, Han Zhou wrote: >>> >>> >>> On Mon, Dec 13, 2021 at 10:37 AM Ilya Maximets <[email protected] >>> <mailto:[email protected]>> wrote: >>>> >>>> On 12/13/21 19:09, Han Zhou wrote: >>>>> >>>>> >>>>> On Mon, Dec 13, 2021 at 7:43 AM Ilya Maximets <[email protected] >>>>> <mailto:[email protected]> <mailto:[email protected] >>>>> <mailto:[email protected]>>> wrote: >>>>>> >>>>>> Snapshots are scheduled for every 10-20 minutes. It's a random value >>>>>> in this interval for each server. Once the time is up, but the maximum >>>>>> time (24 hours) not reached yet, ovsdb will start checking if the log >>>>>> grew a lot on every iteration. Once the growth is detected, compaction >>>>>> is triggered. >>>>>> >>>>>> OTOH, it's very common for an OVSDB cluster to not have the log growing >>>>>> very fast. If the log didn't grow 2x in 20 minutes, the randomness of >>>>>> the initial scheduled time is gone and all the servers are checking if >>>>>> they need to create snapshot on every iteration. And since all of them >>>>>> are part of the same cluster, their logs are growing with the same >>>>>> speed. Once the critical mass is reached, all the servers will start >>>>>> creating snapshots at the same time. If the database is big enough, >>>>>> that might leave the cluster unresponsive for an extended period of >>>>>> time (e.g. 10-15 seconds for OVN_Southbound database in a larger scale >>>>>> OVN deployment) until the compaction completed. >>>>>> >>>>>> Fix that by re-scheduling a quick retry if the minimal time already >>>>>> passed. Effectively, this will work as a randomized 1-2 min delay >>>>>> between checks, so the servers will not synchronize. >>>>>> >>>>>> Scheduling function updated to not change the upper limit on quick >>>>>> reschedules to avoid delaying the snapshot creation indefinitely. >>>>>> Currently quick re-schedules are only used for the error cases, and >>>>>> there is always a 'slow' re-schedule after the successful compaction. >>>>>> So, the change of a scheduling function doesn't change the current >>>>>> behavior much. >>>>> >>>>> Thanks Ilya! >>>>> To understand the behavior change at the error scenario: >>>>> If there is not much updates to the DB, it keeps rescheduling the next >>>>> snapshot >>>>> with quick=true, which updates "min" but not "max", so in the end "max" >>>>> is small >>>>> than "min" and snapshot happens. Now if the snapshot fails, the next >>>>> snapshot is >>>>> scheduled with quick=ture (because of the error), but the "max" will not >>>>> be updated, >>>>> so it means the next snapshot will happen immediately and if it fails >>>>> again this >>>>> repeats - would this cause a busy loop? It seems a lot of scenarios can >>>>> return >>>>> errors during snapshot, so I think we'd better be careful. >>>> >>>> I agree that we need to be careful. >>>> >>>> Regarding this particular case, we will still wait for 1-2 min before >>>> re-trying, because 'should_snapshot' always checks the 'min' first and >>>> returns if we didn't surpass it. 'max' checked only if 'min' check >>>> passed. >>> >>> Ok, I see. Sorry that I missed this part when executing the code in my >>> brain :o) >> >> No worries. :) >> >>> >>>> >>>> With the current code (without this patch applied), on error the >>>> 'max' will be updated, but that will only matter if the previous >>>> failed snapshot attempt was due to hitting the 'max' condition. >>>> >>>> Assuming that the database didn't grow a lot and 24 hours passed. >>>> With the current implementation: >>>> 1. ovsdb will try to snapshot >>>> 2. fail >>>> 3. re-schedule a quick retry updating min and max >>>> 4. wait 1-2 min (now < min). >>>> 5. should_snapshot will return 'false' since it didn't grow a lot >>>> and the updated max is still almost 24 hours ahead. >>>> 6. checking on every iteration until it grew a lot or 24 hours passed. >>>> 7. if it didn't grow a lot and 24 hours passed, goto 1. >>>> >>>> So, the actual next attempt after the failure in this case will be >>>> after 24 hours. >>>> If the reason for a failed snapshot was the db size, 'should_snapshot' >>>> will return 'true' on the step 5. So it will re-schedule and try to >>>> snapshot every 1-2 min until it succeds. >>>> >>>> With the patch applied we will re-schedule and re-try to create a >>>> snapshot every 1-2 min regardless of the reason why we wanted to >>>> create it in the first place. Because now < max will keep being >>>> false until the snapshot created successfully. >>>> >>>> That's the only difference I can think of. >>>> >>>> It should be OK to re-try every 1-2 min. What do you think? >>>> >>> >>> Yes, I think it is even better than before. My only concern was busy-loop >>> but I was wrong. So: >>> >>> Acked-by: Han Zhou <[email protected] <mailto:[email protected]>> >> >> Thanks! I want to run a couple of tests with ovn-heater before >> applying the change. Will reply once this is done. >> > > Looks good to me too, thanks! > > Acked-by: Dumitru Ceara <[email protected]> >
Thanks, Han and Dumitru! The tests looked fine, so I applied the patch. Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
