On 03/13/2017 06:29 AM, Michal Privoznik wrote:
While connecting to qemu monitor, the first thing we do is for it
s/is for/is wait for/
to show up. However, we are doing it with some timeout to avoid
indefinite waits (e.g. when qemu doesn't create the monitor
socket at all). After beaa447a29 we are using exponential back
off timeout meaning, after the first connection attempt we wait
1ms, then 2ms, then 4 and so on. This allows us to bring down
wait time for small domains where qemu initializes quickly.
However, on the other end of this scale are some domains with
huge amounts of guest memory. Now imagine that we've gotten up to
wait time of 15 seconds. The next one is going to be 30 seconds,
and the one after that whole minute. Well, okay - with current
code we are not going to wait longer than 30 seconds in total,
but this is going to change in the next commit.
The exponential back off is usable only for first few iterations.
Then it needs to be caped (one second was chosen as the limit)
and switch to constant wait time.
Cool. Nice job remembering the subtle fix needed here with potentially large
timeouts!
Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
---
src/util/virtime.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/src/util/virtime.c b/src/util/virtime.c
index aac96918a..650b1d0f9 100644
--- a/src/util/virtime.c
+++ b/src/util/virtime.c
@@ -390,6 +390,9 @@ virTimeBackOffStart(virTimeBackOffVar *var,
return 0;
}
+
+#define VIR_TIME_BACK_OFF_CAP 1000
Maybe just VIT_TIME_BACKOFF_CAP?
+
/**
* virTimeBackOffWait
* @var: Timeout variable (with type virTimeBackOffVar *).
@@ -410,7 +413,9 @@ virTimeBackOffStart(virTimeBackOffVar *var,
* The while loop that runs the body of the code repeatedly, with an
* exponential backoff. It first waits for first milliseconds, then
* runs the body, then waits for 2*first ms, then runs the body again.
- * Then 4*first ms, and so on.
+ * Then 4*first ms, and so on, up until wait time would reach
+ * VIR_TIME_BACK_OFF_CAP (whole second). Then it switches to constant
+ * waiting time of VIR_TIME_BACK_OFF_CAP.
*
* When timeout milliseconds is reached, the while loop ends.
*
@@ -429,8 +434,13 @@ virTimeBackOffWait(virTimeBackOffVar *var)
if (t > var->limit_t)
return 0; /* ends the while loop */
+ /* Compute next wait time. Should go above VIR_TIME_BACK_OFF_CAP
+ * mark, cap it there to avoid long useless sleeps. */
A suggestion for alternate wording of the comment:
+ /* Compute next wait time. Cap at VIR_TIME_BACK_OFF_CAP
+ * to avoid long useless sleeps. */
next = var->next;
- var->next *= 2;
+ if (var->next < VIR_TIME_BACK_OFF_CAP)
+ var->next *= 2;
+ else if (var->next > VIR_TIME_BACK_OFF_CAP)
+ var->next = VIR_TIME_BACK_OFF_CAP;
/* If sleeping would take us beyond the limit, then shorten the
* sleep. This is so we always run the body just before the final
ACK. Your choice on the nits.
Regards,
Jim
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
