Less coarse-grained locking for bindings add/remove operations

[hairyhum]

Proposed Changes

All binding operations aquire special locks for destination and source.
This is done to make sure bindings cannot be added or removed while
being cleaned up.

This means that adding and removing bindings for the same records may
conflict.

To avoid conflict the locks aquired during add/remove are now read locks.

This should improve performance in case of concurrent bindings creation
for the same resource.

Types of Changes

What types of changes does your code introduce to this project?
Put an x in the boxes that apply

• Optimisation
• Bug fix (non-breaking change which fixes issue #NNNN)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause an observable behavior change in existing systems)
• Documentation improvements (corrections, new content, etc)
• Cosmetic change (whitespace, formatting, etc)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating
the PR. If you're unsure about any of them, don't hesitate to ask on the
mailing list. We're here to help! This is simply a reminder of what we are
going to look for before merging your code.

• I have read the CONTRIBUTING.md document
• I have signed the CA (see https://cla.pivotal.io/sign/rabbitmq)
• All tests pass locally with my changes
• I have added tests that prove my fix is effective or that my feature works
• I have added necessary documentation (if appropriate)
• Any dependent changes have been merged and published in related repositories

Further Comments

This change needs some functional performance testing. It's suppose to improve the situation when multiple connections try to bind their queues (e.g. exclusive queues) to the same exchange concurrently.

[hairyhum]

Test: binding a queue to exchange 10 times in each process with different routing keys.

Tested on the node, calling rabbit_binding:add for the same source and
destination with different keys.

Time in microseconds, collected by timer:tc

		Without the change	With the change
100 processes	1000 bindings	~420 ms	~350 ms
500 processes	5000 bindings	~1450 ms	~650 ms
1000 processes	10000 bindings	~2780 ms	~1080 ms

[michaelklishin]

This looks good but during our sync-up we learned that it is not yet ready for QA.

[hairyhum]

More testing:

		Without the change	With the change
1 node
Durable
10 processes	100 bindings	~260 ms	~210 ms
100 processes	1000 bindings	~400 ms	~380 ms
1000 processes	10000 bindings	~2100 ms	~1380 ms
Transient
10 processes	100 bindings	~30 ms	~10 ms
100 processes	1000 bindings	~250 ms	~75 ms
1000 processes	10000 bindings	~2000 ms	~700 ms
2 nodes
Durable
10 processes	100 bindings	~320 ms	~230 ms
100 processes	1000 bindings	~1230 ms	~460 ms
1000 processes	10000 bindings	~11700 ms	~3310 ms
Transient
10 processes	100 bindings	~100 ms	~23 ms
100 processes	1000 bindings	~860 ms	~200 ms
1000 processes	10000 bindings	~8300 ms	~2100 ms

As expected, with more nodes the difference is more visible.
I'm getting convinced that the change is good enough performance-wise.

[michaelklishin]

Let's wait for a bit and perhaps backport after 3.7.13 GA ships next week.

[michaelklishin]

Good job, @hairyhum 👍👍

[michaelklishin]

References #1513, #1690, #1589, #1715.

[michaelklishin]

3.7.x feels most appropriate before we backport.

[michaelklishin]

Backported to v3.7.x.