Retry timeout

[eagr]

closes #660

Is the retry delay being used (200ms) good enough for the retry for job timeout?

Btw, how serious are you with the SchrodingersCandidate? I got zero problem with the name. :)) @mrcnski @s0me0ne-unkn0wn

[paritytech-cicd-pr]

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: test-linux-stable 2/3
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/4429721

[mrcnski]

Btw, how serious are you with the SchrodingersCandidate? I got zero problem with the name. :)) @mrcnski @s0me0ne-unkn0wn

I love that you asked this. 😂 I vote "yes", only question is, should we include the umlaut?

[mrcnski]

Could be reworded as it's potentially confusing. It is only definitely-invalid in backing where we don't retry the possibly-invalid candidates. In backing there's no time for a retry (and it's stricter).

[mrcnski]

Hmm, actually I'm not sure if we should retry, at least with the current timeout which is DEFAULT_APPROVAL_EXECUTION_TIMEOUT = 12. That's already a long time given for execution.

One idea is to allow a retry but cut that timeout in half. The idea is that if execution is taking 6 seconds (which is already a long time) then something went wrong (i.e. maybe the worker/job communication broke down).

And speaking of communication, the host/worker comms (note, not worker/job comm) have a really long timeout (like factor of 4 of the default timeout). We shouldn't retry if that fails because there'd just be no time left. But that long timeout needs to remain long, because it's wall clock time, whereas in the job we timeout based on CPU clock time. (That probably sounds really confusing, so see paritytech/polkadot#4217.)

But getting back to cutting the approval timeout in half, I wonder if opens up some attacks, like candidates getting through backing but timing out on some validators with slower machines, leading to disputes. 6 seconds would already be 3x the backing timeout, but I can see it happening. cc @eskimor

[eagr]

You have a change of mind now or did I misunderstood your comment?

12s is a really long time. How often does a timeout happen on a valid candidate in practice? If it's extremely rare, I suppose we shouldn't. In that case, I'm happy to close this as it no longer makes sense to switch the category.

[mrcnski]

You understood correctly, I just forgot that we had bumped the approval timeout to 12s.

How often does a timeout happen on a valid candidate in practice?

Not sure, I suppose my concerns are more theoretical in nature. Like if there is a validator that is 3x slower than the rest, it can potentially be exploited to create disputes. I think in practice it's more likely that an approval timeout happens due to some local conditions, like some deadlock, comm bug, etc... though still not very likely.

Maybe some compromise is possible, like 8s + a retry? Not sure, @eskimor would have a better idea.

[eskimor]

Definitely no to shortening the timeout. We have measured fluctuations on the same machine up to a factor of 3. An additional factor of 2 does not seem to be too conservative, given that validators don't run identical machines.

Whether a retry on timeout makes sense is an interesting question:

1. We would be too slow to vote and thus approval voting would escalate because of our no-show. This is an issue, as without the retry we would have raised a dispute - which is a full escalation, but in that case at least someone will pay the bill.
2. Retrying increases the chances of us voting valid on a candidate. If a super majority finds this borderline candidate to be invalid, we just increased our chances of getting slashed significantly. Although if everybody does this, then the chances of a super majority finding the candidate invalid also gets smaller. On top of this with Slash approval voters on approving invalid blocks - dynamically #635 in place, collectively it does not matter much if 1 or 10 validators voted valid on an invalid candidate.

2 is not much of a reason, but 1 means it is free to trigger an escalation (and break sharding). So retry on timeout is not an option ... in fact if we had the time, I don't see why we would ever retry instead of just increasing the timeout to begin with. What would be the value of retrying, instead of increasing the timeout? If you restart - any work already done (even if little) would go to waste. While with a longer timeout you keep it and have a better success chance, given the same time constraints.

[eskimor]

12s is a really long time. How often does a timeout happen on a valid candidate in practice?

It is.

If it's extremely rare

We at least hope so. That's the whole point of that long timeout. We actually should increase the noshow timeout because of above reasoning, as execution time alone is not the full story.

For handling dishonest backers, we need time disputes.

[mrcnski]

We have measured fluctuations on the same machine up to a factor of 3.

Wow, a 3x fluctuation on the same machine? This must be wall clock time? The timeouts in question are CPU time. But still, I agree that we should have a large margin for differently-performing validators.

in fact if we had the time, I don't see why we would ever retry instead of just increasing the timeout to begin with. What would be the value of retrying, instead of increasing the timeout?

If execution is taking that long, then it could be due to some deadlock etc. Waiting a second and then retrying could avoid some local nondeterminism. Then again, I can't think of any specific internal scenarios that would cause this to occur, except for high load on the machine, in which case retrying could make things worse.

Maybe, there is some way for the node to query whether the execution job is at least making progress as opposed to spinning in a loop. But I might be getting into the weeds here.

[eagr]

Thanks for the explanation and pointers! @eskimor Those are so informative. Curious, what difference does it make if we no-show for longer time, if any? Say we double the timeout.

You could just close this if there's no reason to proceed. :) @mrcnski

[burdges]

I've forgotten our last discussion, but I think..

We designed time disputes aka overtime billing so that candidate validation timeouts could be increased to 30s or 1 minute, which reduces our risks from execution times differing. We've maybe deprioritized actually doing this until polkavm reaches some maturity. If polkavm becomes twice as fast then maybe we'll have deterministic metering via the jan's floating point overflow trick. It's possible jan's floating point overflow trick work for metering wasmtime too. We'll restart the overtime billing plan though if the floating point trick becomes dubious.

[mrcnski]

If polkavm becomes twice as fast

Looks like it just happened.

[mrcnski]

Closing. Thanks for the hustle @eagr and apologies for the spurious suggestion! Will also close the related issue as there's nothing left to do.

[mrcnski]

Maybe, there is some way for the node to query whether the execution job is at least making progress as opposed to spinning in a loop.

I suppose it can be done in PVM by checking if gas is going up. Maybe overly paranoid. I wouldn't do it unless timeouts become a source of issues.