Running franzgo code with synctest #1250
Description
Hello,
We use franz go extensively in our codebase, and in our tests. We are also starting to use synctest in a bigger and bigger fraction of our tests.
When using synctest and relying on sync.Mutex, the way the go maintainers chose to not make a Mutex part of the synctest bubble can be a problem. A goroutine which is blocked on a Mutex is not "durably blocked" and so it is not enough to make time go forward in a synctest bubble. This problem is explained here and the maintainers go in a little bit more details here
Most of the time, Mutexes are held for a short amount of time to linearize access to a resource, but sometimes something like this can happen
goroutine 1:
mu.Lock()
defer mu.Unlock()
time.Sleep(xxx)
goroutine2:
mu.Lock()
defer mu.Unlock()
time.Sleep(yyy)
and if this happens the test will be blocked, as time cannot flow forward because there are goroutines that are not durably blocked from the point of view of the synctest bubble.
In practice, this happens with one of our test where the noCommitDuringJoinAndSync mutex is held both by the loopCommit goroutine that is waiting for a network call to come back (and this call is batched and so incurs a very small time.Sleep() ) and by the joinAndSync() goroutine that is blocked for the same reason.
I cannot guarantee that this is the only such mutex in the franzgo codebase, but in our testing, this is the only case we have found that blocked our synctests.
I would like to know if you would be open to refactoring the code to either use a channel-based mutex class or refactoring a little bit around this to use a channel rather than a lock. I could provide a pull request.
Another solution is to create a type alias (xsync.Mutex) that is aliased:
- to
sync.Mutexif no go tags is provided at compile time - to a channel-based mutex implementation if you provide the right go tag at compile time.
^ We have done this in our codebase and it works well for our tests. Here again, if you are open to this solution, I could provide you with a PR that implements this.
I am also available to discuss this, I don't want to impose a solution of course, I would just like to write an even bigger portion of our tests with franzgo and synctest, without having to kgo.DisableAutoCommit() or to fork.