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

No way to use ScanIterator concurrently - consider dropping mutex or changing signature

Open ash2k opened this issue 1 year ago • 1 comments

Expected Behavior

ScanIterator doc says it's safe for concurrent use.

Current Behavior

If one goroutine calls Next() then it cannot call Val() safely because another goroutine might have called Next() at that moment, moving to the next value. In that case a value will be missed, obviously leading to problems.

Possible Solution

Either remove mutex and doc, stating it's safe for concurrent use OR change the interface, allowing for safe concurrent iteration.

I think it'd be much better to just remove the mutex and let the called wrap it in a mutex if they need to. Even if NextVal() (string, error) is introduced, there is no safe way to e.g. iterate over a key-value map, such as HScan result. Perhaps NextPair() (string, string, error) can be introduced? Or maybe a new PairIterator? That'd be useful in any case, but I think mutex should be removed regardless - most users don't need to pay the lock/unlock cost.

I think it's a great time to make this breaking change (well, anyone who is using the current API concurrently is already broken) before v9 is released.

ash2k avatar Jul 23 '22 11:07 ash2k

You seems to be right - let's remove the mutex.

vmihailenco avatar Aug 02 '22 06:08 vmihailenco