RFR: 1659: Fix polling and retries in testinfo bot [v2]

[Erik Joelsson]

On Tue, 8 Nov 2022 10:29:45 GMT, Magnus Ihse Bursie <ihse at openjdk.org> wrote:

>> bots/testinfo/src/main/java/org/openjdk/skara/bots/testinfo/TestInfoBot.java line 63:
>> 
>>> 61:                 .filter(pr -> pr.sourceRepository().isPresent())
>>> 62:                 .map(pr -> (WorkItem) new TestInfoBotWorkItem(pr,
>>> 63:                         delay -> poller.retryPullRequest(pr, now.plus(delay))))
>> 
>> Maybe I'm getting lost here, but this feels wrong to me. The `now` is computed as the now of the call to `getPeriodicItems()`, but it is used in the lambda for the `retry` operation of `TestInfoBotWorkItem`. But that retry should happen `delay` time after it is evaluated, right? 
>> 
>> It might not have much practical impact, but it just looks odd to me, like mixing different time streams.
>
> To expand a bit: the `now.plus(delay)` will be evaluated in the `run` method of the work item, but then the result will be offset not from the time of `run` but from the time of the call to this method. What would happen if it, for some reason, would take more than 2 minutes between these two events, and we'd get a time in the past. Will it be retried immediately, or will it cause trouble with the poller? 
> 
> And also it feel conceptually wrong; the retry time feels like it should be specified from when the decision is made for the next check.

You are right, that is odd, it really should be X minutes after it was evaluated, not after the `getPeriodicItems` call. When bots are busy, the time difference can easily be much longer than 2 minutes, and in that case, effectively ignoring the delay is not helping in reducing the load.

The poller should handle past time fine though, it will basically be equivalent to a retry without a time component.

-------------

PR: https://git.openjdk.org/skara/pull/1411