Rethink descriptor publisher rate-limiting
The following discussion from !1951 (merged) should be addressed:
-
@Diziet started a discussion: (+1 comment) So, suppose it's 50s since we last uploaded. We reach this point and see that
duration_since_upload
is 50s, which is less thanUPLOAD_RATE_LIM_THRESHOLD
(60s).Then we call
start_rate_limit(60s)
.start_rate_limit
callsruntime.now()
and adds its argument, so scheduling a wakeup 60s from now.We will upload again 110s after the last upload. I think though, that we should do it 60s after.
I think the root cause of this bug is the storage of a separate "we are rate limited" state in the reactor state, and using it to control the upload logic. Whether "we are rate limited" is really just "is the last upload more than
UPLOAD_RATE_LIM_THRESHOLD
ago" - ie, we could recalculate that on each loop iteration.In terms of
PublishStatus
(the status reporting output) I'm not sure "we are rate limited" is a particularly useful status to advertise. I think it's an entirely normal condition.Also we should perhaps randomise this?
OTOH I don't think either of these questions are a blockers for this MR. The code here is a lot nicer, so thanks :-).