go-txdb icon indicating copy to clipboard operation
go-txdb copied to clipboard

Published 20 hours ago • verified publisher icon DATA-DOG

fix beginTxOnce() context cancel

Open daisuke0925m opened this issue 1 year ago • 6 comments

we received this error.

transaction has already been committed or rolled back errors

fix: https://github.com/DATA-DOG/go-txdb/issues/43#issuecomment-1625076132

daisuke0925m avatar Mar 18 '24 09:03 daisuke0925m

I'm investigating this closer today, and trying to reproduce it locally, to verify that this is the optimal solution, bit I'm not able to trigger the error, using either your test, or using my own techniques.

Can you provide any additional insight into the circumstances under which this problem surfaces?

Also, which database driver are you using?

flimzy avatar Mar 27 '24 15:03 flimzy

@flimzy

I can reproduce this by running TestShouldRunWithHeavyWork locally without modifying beginTxOnce() in db_go18.go. https://github.com/DATA-DOG/go-txdb/pull/64/files#diff-b0b479cb93a7eee6d8905ffc4e94df874afad18b4a97edbf3e2a1402a20db634R57-R63

Also, I am using mysql in my actual development environment.

daisuke0925m avatar Mar 27 '24 23:03 daisuke0925m

@flimzy @daisuke0925m I also ran the test. I could not reproduce failure with the original test that @daisuke0925m wrote, but successfully reproduced after modifying 10000 to 1000000 in this line. https://github.com/DATA-DOG/go-txdb/pull/64/files#diff-842765a20a0a937a8eb5267b53fb11b7493cc6c2a6a8871e1da2b684702c5bc9R841

Warashi avatar Mar 27 '24 23:03 Warashi

We have a test suite that was plagued by this error in GHA and this change seems to fix it.

I also never was able to reproduce it on my laptop, you need a machine a sluggish as a CI runner to get there apparently.

iamnoah avatar Apr 26 '24 21:04 iamnoah

I'm on holiday now, but when I return, I intend to get back into this and merge it, or an alternative fix, based on my investigation.

flimzy avatar Apr 29 '24 10:04 flimzy

Just wanted to bump. Been using this for 3 weeks now and haven't seen the issue resurface or any other problems.

iamnoah avatar May 20 '24 14:05 iamnoah

My apologies to everyone for my delay in getting back to this. I was able to re-produce the problem locally with the hints above. And while I don't think the underlying structure of how go-txdb handles contexts is ideal (I think I've spotted at least one other possible race condition), this does appear to be an improvement in behavior at least, so worth merging. I may try to do additional refactoring later.

flimzy avatar May 23 '24 08:05 flimzy