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]> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
