On 03/08/2023 16:53, Peter Xu wrote:
> @@ -2694,7 +2694,17 @@ static void migration_update_counters(MigrationState
> *s,
> transferred = current_bytes - s->iteration_initial_bytes;
> time_spent = current_time - s->iteration_start_time;
> bandwidth = (double)transferred / time_spent;
> - s->threshold_size = bandwidth * migrate_downtime_limit();
> + if (migrate_max_switchover_bandwidth()) {
> + /*
> + * If the user specified an available bandwidth, let's trust the
> + * user so that can be more accurate than what we estimated.
> + */
> + avail_bw = migrate_max_switchover_bandwidth();
> + } else {
> + /* If the user doesn't specify bandwidth, we use the estimated */
> + avail_bw = bandwidth;
> + }
> + s->threshold_size = avail_bw * migrate_downtime_limit();
>
[ sorry for giving review comments in piecemeal :/ ]
There might be something odd with the calculation. It would be right if
downtime_limit was in seconds. But we are multipling a value that is in
bytes/sec with a time unit that is in miliseconds. When avail_bw is set to
switchover_bandwidth, it sounds to me this should be a:
/* bytes/msec; @max-switchover-bandwidth is per-seconds */
avail_bw = migrate_max_switchover_bandwidth() / 1000.0;
Otherwise it looks like that we end up overestimating how much we can still send
during switchover? If this is correct and I am not missing some assumption, then
same is applicable to the threshold_size calculation in general without
switchover-bandwidth but likely in a different way:
/* bytes/msec; but @bandwidth is calculated in 100msec quantas */
avail_bw = bandwidth / 100.0;
There's a very good chance I'm missing details, so apologies beforehand on
wasting your time if I didn't pick up on it through the code.
Joao
