dcrdata icon indicating copy to clipboard operation
dcrdata copied to clipboard

remove custom trylock in favor of stock sync.Mutex

Open chappjc opened this issue 4 years ago • 1 comments

Go 1.18 supplements Mutex with a TryLock method. For the on-demand concurrent client accesses to the cache-guarded DB, use this directly instead of our dumb homebrew trylock mutex when Go 1.18 is in use.

IMPORTANT NOTES:

  • This change does NOT adopt a new "try lock" pattern. This involves legacy code that had already established this with custom mutex type that has the same new method provided by the sync.Mutex in Go 1.18, so this just uses it.
  • While I am aware that proper uses of TryLock are few and far between, the synchronization need here involves controlling access to two data sources, a cache and a DB, where a simple CAS operation is insufficient. In particular, if there is no cached data or if it is stale (by height), a DB query must be launched on-demand, but there must only be one querying goroutine. That part is solvable with a CAS. However, subsequent callers concurrent with the DB updating goroutine (who failed to get the lock or swap) may return stale data, but if there was no cached data they must wait for the DB updater goroutine. While other solutions are readily conceivable for this, the pattern if !TryLock() { if haveCachedData { /* return stale data */ } else { Lock() /* wait */ } } solves this quite handily. This has been functioning well in production for quite some time.

The use case in pseudo code:

// First try the cache for fresh data.
cachedData, stale := checkCache(height)
if !stale {
	return cachedData
}

// Stale data.  Attempt to gain updater status.
if mtx.TryLock() {
	// Query the DB, update cache, and return fresh data.
	...
	return freshData
}
// Another goroutine is running a db query to get the updated data.

if cachedData != nil { // return stale data from cache if we have any
	return cachedData
}

// No stale data available, wait on the DB updater with a full `Lock`
mtx.Lock() // waiting, could also RLock
cachedData, _ := checkCache(height)
return cachedData

An alternative approach that does not involve locking to wait on the data, but rather a channel to receive from, used elsewhere in the project: https://github.com/decred/dcrdata/blob/429ab905142074af46a78c1e90accb222dddf129/db/cache/addresscache.go#L56-L65

Also update CI to use go 1.18-beta1.

chappjc avatar Dec 15 '21 05:12 chappjc

@dajohi wdyt?

ukane-philemon avatar Aug 26 '25 09:08 ukane-philemon