[GODRIVER-2109] Race condition in staleness checks if connection has not finished connecting Created: 28/Jul/21 Updated: 28/Oct/23 Resolved: 02/Sep/21 |
|
| Status: | Closed |
| Project: | Go Driver |
| Component/s: | None |
| Affects Version/s: | 1.7.0 |
| Fix Version/s: | 1.7.2, 1.6.2 |
| Type: | Bug | Priority: | Critical - P2 |
| Reporter: | Divjot Arora (Inactive) | Assignee: | Matt Dale |
| Resolution: | Fixed | Votes: | 2 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||||||||||
| Cloud Backport: | Needed | ||||||||||||||||||||||||
| Description |
|
We're seeing a race condition in tests that set MinPoolSize when upgrading to 1.7.0. Relevant pieces of the race detector output:
My understanding is that a connection resource will be added to the resource pool in the foreground while the actual establishment is done in the background. When resourcePool#Get is called, it may iterate over this resource and check if it's expired even though it hasn't finished connecting. This can probably be mitigated by a mutex/atomic, but it brings up a larger question about whether we should even be checking connections for expiration if we know they're not done connecting yet. |
| Comments |
| Comment by Matt Dale [ 07/Sep/21 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
james@getadmiral.com thanks for the retraction suggestion! I forgot to include that in the v1.7.2 release, but I've created GODRIVER-2151 to track retracting the affected Go module versions as part of the v1.7.3 release. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Matt Dale [ 02/Sep/21 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Fix for this is available in Go Driver v1.6.2 and v1.7.2 releases. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Matt Dale [ 02/Sep/21 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Does this need a backport for the cloud-1.7.1 branch? | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Githook User [ 01/Sep/21 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Author: {'name': 'Matt Dale', 'email': '9760375+matthewdale@users.noreply.github.com', 'username': 'matthewdale'}Message: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Githook User [ 01/Sep/21 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Author: {'name': 'Matt Dale', 'email': '9760375+matthewdale@users.noreply.github.com', 'username': 'matthewdale'}Message: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Matt Dale [ 01/Sep/21 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Fix PR #728 is merged. We're planning to do an expedited release, so it should be available sometime this week. I'm starting the release process now and will update this ticket when the release is ready. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Githook User [ 01/Sep/21 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Author: {'name': 'Matt Dale', 'email': '9760375+matthewdale@users.noreply.github.com', 'username': 'matthewdale'}Message: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Peter Smith [ 30/Aug/21 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Amazing, thanks for the update matt.dale 🙏 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Matt Dale [ 30/Aug/21 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
adinoyi.omuya and peter.smith, this fix is currently scheduled for release with Go Driver v1.7.2 on Tuesday, Sept 7. I'm starting the implementation today, so I'll post any timeline updates to this ticket. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Peter Smith [ 30/Aug/21 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Hi matt.dale 👋 We have a customer in POC right now affected by this Go Driver race condition, in our last sync call, I said I 'd get them an update on the resolution progress. Would you be able to give me something I can share to keep them in the loop? Really appreciate anything 🙏 Peter | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by James Hartig [ 27/Aug/21 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Can you retract the affected versions so people don't unexpectedly update to a newer release and run into this? We had no idea about this until we updated our modules and now were seeing these race complaints in all of our repos. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Adinoyi Omuya [ 23/Aug/21 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
This is impacting completion of some additional work on the ADL side - e.g. MHOUSE-3261 - do y'all have a rough estimate for when this might be fixed? | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Alexej Kubarev [ 17/Aug/21 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Same data race arises on the first operation on connection in a pool, even with minPoolSize=1 (so basically single connection that receives an operation) with v1.7.1 driver. While the triggering methods are different (e.g CreateIndex) the race condition happens in the same place. So guards around that should be put in place. Reproduction steps1. Use a simple application, such as this:
2. Run with go run -race main.go 3. Observe data race printout due to the first issues command (Ping in this case)
|