improve the deduplication logic

[garypen]

Close examination revealed that some judicious tweaking of the locking and spawning would improve the behaviour.

In performance testing this performs the same as current (dev) or using the deduplicate crate. I can't reproduce the hanging problem with dev with this set of changes.

The rationale for the changes are as follows:

• We need to spawn the sentinel before we insert the waiter entry or failure after inserting, but before spawning a sentinel, will cause issues.
• Notification of waiters from the "first" entry must be synchronised under the "wait_map" lock, or else that becomes racy and subscribers may be missed. That notification must be immediately preceded by the removal of the wait_map entry.

fixes: #1984

[Geal]

that sounds very reasonable. So the issue you were seeing would have been that getting data from the storage at line 65 waits indefinitely or the task stops, and so the sender would remain?
Or was it the race between removing the entry from the waitmap and sending the value?

[garypen]

that sounds very reasonable. So the issue you were seeing would have been that getting data from the storage at line 65 waits indefinitely or the task stops, and so the sender would remain? Or was it the race between removing the entry from the waitmap and sending the value?

Moving the sentinel before the insert guarantees that we can't have an entry in the entrymap that lives longer than we desire. That's the really important change, since otherwise we might have inserted an entry that will never be removed. With such a thing in our wait_map, then new clients just subscribe and sit there waiting for nothing.

We absolutely must remove the waiters (under lock) before send, but we could perhaps drop the lock just before the send and still be safe. I didn't want to take that risk, since performance is already very good and we know this area is complex to work with.

UPDATE: I tried dropping the lock before the send and it appears to work fine. Performance isn't any better than when we hold the lock over the send, so I'll leave things as they are for now.