Hacker News new | ask | show | jobs
Upgradable Read Write Lock for Go (upstash.com)
16 points by sancar 973 days ago
3 comments

iirc RWMutex used to be much slower than standard Mutex so there was no good use case for it over standard Mutext. That might have gotten fixed, don't know.

Secondly, you can use atomics instead of mutexes. They will be faster by about 400% iirc.

Thirdly, you do not need to be concerned for readers, only for writers. Readers can concurrently read and they do not care if value changes. The only concern is writers because the can collide and that is where you need synchronizing. Protecting reads makes sense only in rare cases where you might have a map and it gets set to nil. So obviously reading from nil map will panic. Other than that there is not really much cases to protect readers and add locking overhead just for them.

The benefit of an RWMutex lies in read-often, write-rare, where serialization of the portion under the read lock would significantly limit throughput (i.e., the read portion takes a while and overlaps significantly with other readers).

Whether atomics are applicable depend heavily on the structures and what you are doing under read lock. Atomics are good for reading primitives or pointers that get swapped whole, but not very useful for reason complex structures or traversing arrays/maps. Lock-free atomic structures exist, but are also often slower than their non-atomic counterparts.

Rather than making generic statements about what is slow and what is best, profile, profile, profile!

> iirc RWMutex used to be much slower than standard Mutex so there was no good use case for it over standard Mutext. That might have gotten fixed, don't know.

If you check the implementation RWMutex actually uses a Mutex underneath. The `RLock` is just an atomic increment if you don't have a writer.

The `Lock`(write lock) is normal `Mutex.Lock` + an atomic add.

> Secondly, you can use atomics instead of mutexes. They will be faster by about 400% iirc.

The golang community and the guys that actually implementing golang usually says `share via commuinication` and don't use any locks in the first place. You don't need even atomics in that case.

But in some cases, in production, you can measure and see that locking is better. I can admit that most of the projects can get away with not using locks and follow the guideline.

So I guess, if performance is important, try everything and measure.

> Thirdly, you do not need to be concerned for readers ...

Well, the use case was guarding multiple things under single lock. So the read lock needs to be there because you need a consistent view of the world. Otherwise, the reader see that a field is changed but another is not. And it is a bug for our case .

A simplified concrete example, Under write lock, user can call REDIS MSET command. https://redis.io/commands/mset/

Related part of the commands doc

``` MSET is atomic, so all given keys are set at once. It is not possible for clients to see that some of the keys were updated while others are unchanged. ``

So the readers should see that all updated or none updated. That is why for example one one can need a read lock.

Obviously, aside from the contract we have a lot going on in the background. Some of the data is in memory. some are loading. Some are just written in memory and dirty, therefore next read needs to wait for it to persisted to disk if it is evicted from memory etc....

All of these needs coordination between readers and writers.

This upgradable RW lock looks fundamentally broken in my opinion:

If you use such a lock, it is because you want to write something based on data that you read.

And nothing should change the date after your calculus and before the write.

But if you use 2 of such upgradable RW lock concurrently at the same time: Both will read the original data, and so calculate something based on that, and they will just wait one after another to write. But the second one will not notice and recalculate after the first write, or you will have a deadlock...

In the post they mentioned that there can only be a single upgradeable read to prevent this.
Yes, exactly. Thanks danbulant for reading the blog :)
Have you benchmarked your solution with the plain Write lock? Can share, if you have done so?
Hi, not by itself no.

Since it is part of a bigger implementation, we tested if it has a measurible effect on the usual path and also verified if it is helping where it should help.

In the case, where there is a long running read operation in our Redis server, we saw that other reads not waiting and working at full speed.

In the other cases, like there are only writes against the server, we can not find a performance problem. To be exact, the more cpu usage is not comparable to the latency we already have. The network calls(even in local network) and all other works are already much more bigger bottleneck then the additional CPU cost we introduced here.