Commit Graph
72 Commits
Author SHA1 Message Date
Matthias Radestock 9aa4313d37 refactor 2016-01-06 17:38:40 +00:00
Matthias Radestock 969dc4e527 get rid of GossipSender gc
With the per-broadcast-source GossipSenders gone, we can get rid of
GossipSender gc...

Instead of maintaining a Connection->GossipSender map in the
GossipChannels - which we then need to gc when connections get closed
- we keep gossip_channel_name->GossipSender maps in LocalConnections.

Access to the map is guarded by a separate lock, rather than re-using
the LocalConnection lock. This reduces lock contention and risk of
deadlock.

In the GossipSenders, we replace the Stop() method with a signalling
channel, i.e. senders stop when that channel gets closed. Handily, the
LocalConnection.finished channel fits that description. So when a
LocalConnection's run loop terminates, any existing and future
associated GossipSenders will end up terminating too.
2016-01-06 17:38:40 +00:00
Matthias Radestock 99b7fbbc71 optimisation: stop GossipSender on send error 2016-01-05 17:18:41 +00:00
Matthias Radestock 486eb740b5 optimisation: abort GossipSender.deliver when stopped 2016-01-05 17:18:41 +00:00
Matthias Radestock c6a73b8a75 don't stall broadcasting when there are blocked connections
Previously broadcasts were being handled by one GossipSender per
broadcast source (and channel), which sends the broadcast to all next
hops, and hence can stall when a single destination is stalled.

Here we get rid of these per broadcast source GossipSenders. Instead
broadcasts are sent to per-connection GossipSenders for all next hops.

For this we use the existing GossipSenders we have set up for ordinary
gossip. In order to deal with broadcasts they need one GossipData cell
per broadcast source, since only broadcasts from the same source can
be Merge()ed. So we add a PeerName->GossipData map of cells, in
addition to the existing cell for ordinary gossip.

The GossipSender goroutine picks one of the cells at a time, Encode()s
the contents and sends it. It prefers the ordinary gossip cell over
the broadcast cells since typically ordinary gossip is more important.

To reduce coupling, the GossipSenders don't actually know how to
construct protocol messages. They just invoke a couple of functions
for that - one for ordinary gossip and one for broadcast - which are
supplied by the GossipChannel.

There are two downsides to this change:

1. broadcasts get encoded per connection, rather than just once

2. we can potentially end up with O(n_peers^2) cells, each containing
accumulated (via GossipData.Merge()) broadcasts. For this to happen,
the peer must

- deal with gossip from most nodes. This doesn't happen in
  (near)complete connection topologies. Hypercube topologies are
  probably the worst case uniform topology. And a star topology is the
  worst case for a single (the centre) peer.

- have backlogged connections to most of its neighbours, without
  those connections being completely stalled (since that would cause
  heartbeat timeouts to terminate them).

Furthermore, for this to matter in practise, the accumulated broadcast
GossipData must be sizeable:

- For topology gossip, GossipData is just a set of PeerNames. Which
  takes very little space and is bounded, since there is a finite
  number of peers. And, if we could get rid of workaround for #1793
  (cbaa92dcb), then topology broadcasts would only ever contain
  information about the source peer, so the GossipData would contain
  just one PeerName.

- IPAM only employs broadcast during initialisation and shutdown.

- DNS broadcasts contain DNS entries for containers on the source
  peer. Each entry will typically be 100-200 bytes. The accumulated
  broadcast GossipData from a peer will contain entries for all the
  peer's DNS entries, worst case. This includes tombstones,
  i.e. entries for containers that have died. If there is churn,
  i.e. DNS entries being added and removed continuously, and the churn
  rate exceeds the rate at which we can forward those entries, then
  the accumulated broadcast GossipData can grow unbounded. Note that
  this is the case on master too; the difference here is that we can
  have up to n_peers copies of that GossipData.
2016-01-05 17:18:19 +00:00
Bryan Boreham 262f8915a8 Merge pull request #1826 from weaveworks/cleaner-gossip-sender
clean up GossipSender
2016-01-05 16:36:26 +00:00
Matthias Radestock 75d9e95b15 appease linter 2016-01-02 12:35:10 +00:00
Matthias Radestock 58de14ff65 optimisation: don't re-assign our ShortID if nothing got gc'ed 2015-12-30 14:07:23 +00:00
Matthias Radestock 4d304146f3 refactor: inlining 2015-12-30 12:29:23 +00:00
Matthias Radestock e9520215a0 refactor: introduce a helper
which has a very narrow purpose, and can use 'defer' to release the
lock, which is nice.
2015-12-30 12:09:19 +00:00
Matthias Radestock a2b192c472 we no longer need the GossipChannel lock around GossipSender.Send 2015-12-30 12:09:19 +00:00
Matthias Radestock 52575915d0 clean up GossipSender and make it more self-contained
In particular don't require Send() to be serialised by the caller.

We split up the triple(!) purpose 'cell' channel into:

1) a 'cell' member, holding pending GossipData or nil
2) a 'more' channel, for signalling that 'cell' has been filled
3) a 'stop' channel, for requesting termination
2015-12-30 12:09:19 +00:00
Matthias Radestock 0ecc0ee02e tiny refactor 2015-12-30 12:07:56 +00:00
Matthias Radestock 8101dea338 remove some noisy logging 2015-12-24 21:25:25 +00:00
Matthias Radestock 373f8a4463 oops 2015-12-24 21:23:35 +00:00
Matthias Radestock 8846274c42 optimisation: terminate connection actorLoop asap on error 2015-12-24 14:25:25 +00:00
Matthias Radestock c2356ebcde optimisation: stop sending gossip on dead connections
An error in SendProtocolMsg indicates that the connection is
permanently broken. So there is no point continuing to attempt to send
gossip on it.

The connection will eventually be removed from LocalPeer, and any
associated GossipSenders will be gc'ed, but that doesn't happen
straight away, so this change is about not doing unnecessary work in
the meantime.
2015-12-24 12:27:33 +00:00
Matthias Radestock 33812f7666 tiny refactor: rename 2015-12-23 18:44:09 +00:00
Matthias Radestock f58e3bf2a9 refactor: relocate an optimisation -> reduce duplication 2015-12-23 18:27:59 +00:00
Matthias Radestock 5473995305 remove maxFeasibleMessageLen check
since the root cause got fixed by 3cd199e85
2015-12-23 15:29:35 +00:00
Bryan Boreham 1796582032 Merge pull request #1773 from weaveworks/1761-lazy-route-calculation-1
lazy route calculation; LGTM
2015-12-23 12:27:02 +00:00
Matthias Radestock 39e29fd40b refactor(ish): cleaner / more idiomatic GossipSender flush 2015-12-23 10:57:36 +00:00
Matthias Radestock cb6176fd8a tweak ProtocolSender API
make SendProtocolMsg return an error; this isn't used at any call
sites (yet) but is TRTTD.
2015-12-21 18:24:22 +00:00
Matthias Radestock b93ce1cea5 refactor: no need for GossipSender.sent 2015-12-21 18:09:50 +00:00
Matthias Radestock 3be28523c9 cosmetic 2015-12-21 17:56:38 +00:00
Matthias Radestock 592335092c refactor: tweak GossipChannel API 2015-12-21 17:42:20 +00:00
Matthias Radestock 6c4e9fccd3 cosmetic 2015-12-21 17:28:22 +00:00
Matthias Radestock 6903b8d458 refactoring for consistency 2015-12-21 17:18:12 +00:00
Bryan Boreham cbaa92dcb2 Revert "don't broadcast-gossip other peer when adding a connection"
while we consider how to fix it properly.

This reverts commit cbb2ad018d.
2015-12-16 13:46:37 +00:00
Matthias Radestock 81df06b3f4 fix DeleteTestGossipConnection
This is a one-sided operation, so we shouldn't be touching the
remote's Peers.

Also fix the associated test... When both 1 and 2 have lost their
connection to 3, they no longer have a route to 3 and hence gc it.

And because they no longer have a route, gossip from them won't get
through to 3.
2015-12-16 13:18:05 +00:00
Matthias Radestock 5401037743 refactor: inline mockGossipConnection.Connect
Also change Start() order in AddTestGossipConnection to be more
pleasing to the eye; the order isn't actually important.
2015-12-16 12:45:12 +00:00
Matthias Radestock 9d9768ee6f fix incorrect peer initialisation on test gossip connections
We were creating peers with NewPeerFrom, which copies the Version (but
not connections). That's not what happens for a real connection, where
remote peers are always created with an initial version of 0.

The mistake results in brokenness: when we subsequently receive a
topology update for that peer with the same version then we ignore it,
even though the update contains connection information which is new to
us.

Related, it is conceptually wrong to call FetchWithDefault on the
remote router. That is not what happens for a real connection - the
FetchWithDefault occurs in the symmetric counterpart.
2015-12-16 12:33:07 +00:00
Adam Harrison 4e41d4d451 Add trusted subnets to weave status
`weave status` now displays the list of specified trusted subnets, and
`weave staus connections` shows the encryption state of individual
connections.
2015-12-14 12:13:16 +00:00
Adam Harrison 059cc65b55 Fallback to encrypted sleeve on a per-connection basis
If overlay encryption is required between peers, the fast datapath
forwarder returns an error rather than aborting the process, allowing
the overlay switch to fall back gracefully to an encrypted sleeve
overlay connection.
2015-12-14 12:13:11 +00:00
Adam Harrison d434b50dfc Include trust level in features exchange
Add a 'Trusted' feature to the protocol exchange which indicates whether
the remote considers us to be reachable via a secure network; overlay
encryption will be disabled in the event that both peers agree. For
backwards compatibility purposes a default value of untrusted is assumed
if the feature is missing.
2015-12-14 12:12:25 +00:00
Matthias Radestock b1cb0c298d refactor 2015-12-10 14:37:30 +00:00
Matthias Radestock d7835eabfb lazy route calculation
Instead of calculating all routes in one go, we calculate them when
needed.

Except for unicast routes, and broadcast routes from ourself. We still
calculate those eagerly because they are needed to route locally
captured/fdp-missed packets, and performing route calculcations in
that critical path increases the chances of dropping packets due to
not keeping up.

Well, technically only the established & symmetric routes are needed
for the data plane, but we might as well calculate the equivalent
routes for the control plane - it keeps the code simple and uniform.

Calculating all routes is O(n_peers^2), so this change represents a
huge saving in situations where not all of them are needed, which is
quite common, especially when the topology is evolving rapidly.

One downside of this new approach is that we can no longer suppress
OnChange callbacks when the data plane routes remain unchanged. This
means we will be purging the FDP flows more frequently than before,
e.g. when only the control plane topology changed, e.g. new
connections appeared that haven't been marked as 'established'
yet. It's a small price to pay, fixable if we really care, and in any
case making FDP flow invalidation more fine-grained would be a better
approach.

Fixes #1761.
2015-12-10 14:20:59 +00:00
Matthias Radestock ab902b45aa refactor: introduce a helper to reduce duplication 2015-12-10 10:56:18 +00:00
Matthias Radestock ed14561afd refactor: rename 2015-12-10 10:54:20 +00:00
Matthias Radestock 86d2db5cc2 better emulation of connections in gossip tests
real Connections do not process gossip until after AddConnection.
mockChannelConnection wasn't behaving in that way, which resulted in
some sporadic test failures since it would cause some gossip to get
processed too early, which in turn would trigger gc of some of the
just-received peers, which we would then find missing in the test
assertions.

To fix this we introduce a latch on mockChannelConnection which closes
when the remote end is ready to receive gossip. Effectively this
emulates the entering of LocalConnection.receiveTCP after the call to
AddConnection completes.

Unfortunately an open latch will cause sendPendingGossip to block. We
get around that by abandoning checks in "half-connected" connection
states.

Fixes #1765.
2015-12-10 10:54:10 +00:00
Matthias Radestock 1374e4c4de ditch RunWithTimeout test helper
It breaks stack traces on failure.

We can just rely on running `go test -timeout ...` instead, which we
already do in tools/test.
2015-12-09 13:20:57 +00:00
Matthias Radestock 0ef57a102f cosmetic: remove unnecessary comment 2015-12-09 12:37:53 +00:00
Matthias Radestock be1dc8ef96 Merge branch '1.3' 2015-12-09 10:34:52 +00:00
Matthias Radestock 18fc6ccc03 remove redundant code in test 2015-12-09 10:32:14 +00:00
Matthias Radestock 7ec46983aa Merge branch '1.3' 2015-12-08 23:08:35 +00:00
Matthias Radestock eddcf93942 oops 2015-12-08 23:08:19 +00:00
Matthias Radestock 42807f7b8a Merge branch '1.3' 2015-12-08 23:07:06 +00:00
Matthias Radestock 0b521272ae prevent peer duplication in tests
and correct the test assertions accordingly - they make more sense
now.

Fixes #1766.
2015-12-08 23:06:02 +00:00
Matthias Radestock cbb2ad018d don't broadcast-gossip other peer when adding a connection
This is more efficient since broadcasting about other peers is
strictly redundant. Any change to the peers will be broadcast by
those peers anyway. And SendAllGossipDown takes care of informing new
peers about others on first connect, and periodic gossip take care of
missed broadcasts.

The only reason we did broadcast about the remote peer is to reduce
the liklihood of UnknownPeerError and the associated throwing away of
gossip. This was always a bit of kludge, and is no longer necessary
since UnknownPeerError is gone.

Now peers only ever broadcast information about themselves. Which,
unlike information about other peers, is always authorative and most
up-to-date. Which is nice.
2015-12-01 13:10:06 +00:00
Matthias Radestock 7e15eabe65 remove UnknownPeerError
when encountering an unknown peer in received gossip, rather than
ignoring the entire gossip we create a placeholder Peer. This has
version 0 and uid 0, which means it will be replaced by any complete
peer information subsequently received via gossip.
2015-11-30 16:17:16 +00:00