France-ioi / AlgoreaBackend

Backend for the new Algorea platform
MIT License
1 stars 2 forks source link

Fix test TestResultStore_Propagate_Concurrent on CI #1067

Open GeoffreyHuck opened 4 months ago

GeoffreyHuck commented 4 months ago

It has been commented out for now because it makes me lose a lot of time.

Details in commit message: https://github.com/France-ioi/AlgoreaBackend/pull/1066/commits/e8ae6a1ea19d9077dd765b49147178738ed5f8ad

GeoffreyHuck commented 4 months ago

What the test does

It fails on CI because it reaches the timeout (it is way slower on CI, maybe because the CPU there only have 2 cores, or maybe just because it's an older machine):

image

Current behavior

The result propagation takes a lock with SELECT GET_LOCK("listener_propagate", 10 seconds), with a timeout of 10s, to make sure it's never running concurrently. This is reasonable because the way it's written, it really can't be run concurrently (maybe that could be changed but it might be difficult).

What we want

I don't think the current behavior is really what we want.

  1. It's possible that the results propagation takes > 10s to execute. If we follow this, shouldn't we want a timeout > 10s?

  2. What we want, when a results propagation is running and another request hits the lock, is to make sure the results propagation is called again after it is finished (to propagate the results that were just marked to be propagated, but not propagated during the propagation that is currently running). If a THIRD request or more hits the lock at the same time, we can safely ignore it because what's currently marked to be propagated will be taken care of during the propagation started by the SECOND request. So theoretically, having more than one process waiting at a time is useless.

Problem: implementing such a strategy is a little awkward. It would be something like this with a two locks strategy:

if we acquire LOCK_1 right now:
  PROPAGATION()
  RELEASE(LOCK_1)
else if we acquire LOCK_2 right now:
  ACQUIRE_WITH_WAIT(LOCK_1)
  RELEASE(LOCK_2)
  PROPAGATION()
  RELEASE(LOCK_1)
else:
  EXIT()

WIP: needs a bit more thoughts