Review Board 1.7.22

Create a dedicated connection for each federation bridge (route)

Review Request #3331 - Created Dec. 29, 2011 and discarded

Kenneth Giusti
aconway, gordon, mick, tross
Proposed set of changes that will create a new connection for each bridge that is created on a link.
Unit tests.
Ship it!
Posted (Jan. 4, 2012, 3:53 p.m.)
Only minor comments
I'd prefer returning strings by value rather than const ref. Returning a ref is a race condition if the string referenced is potentially deleted in a different thread. Probably not the case here but I'd prefer to keep a simple convention of return by value. The cost of an atomic counter inc/dec is pretty small and will be paid anyway if the caller assigns the return value to another string.
Why no log message here? Bridge connecting seems like a significant event.
I agree that this is OK.
Where (which class & thread) do we detect failure of a link connection to call this? Will it be called multiple times if multiple connections fail?
Should be updateAddressLH.
Posted (Jan. 11, 2012, 5:19 p.m.)
Having a connection per bridge would potentially allow a great deal of simplification, by collapsing the link and bridge concepts into a single entity. This change seems to add complexity. It looks neat enough, I'm not complaining about the changes. However I would like to consider whether this is an opportunity for some cleanup of an overly complex and fragile/brittle component. Thoughts?
  1. You're correct - changing the behavior to "one connection per route" would open up the opportunity to simplify the use model (and the code).  Hadn't considered that, unfortunately, since my original goal for these changes was to improve the performance and scale of inter-broker links.   Given the current threading architecture of the broker, this approach boils down to allocating more connections to these inter-broker flows.
    But the early feedback on this patch, and the approach in general (1 connection/route) was that it is too restrictive with respect to the allocation of connections (a limited resource in itself).  A more flexible approach would grant the user full control over the number of connections between two brokers (aka 'links'), and control over the distribution of sessions (aka 'bridges') across them.  I'm finishing up a (smaller) set of patches that does exactly that - extend the existing model to permit multiple Links to the same destination - in lieu of these proposed changes.