Xmpp update generation should use multiple Tasks

Bug #1607132 reported by Nischal Sheth
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Juniper Openstack
Status tracked in Trunk
R3.2
Fix Committed
Wishlist
Nischal Sheth
Trunk
Fix Committed
Wishlist
Nischal Sheth

Bug Description

Existing SchedulingGroupManager design allows multiple SchedulingGroups to
work in parallel. Each SchedulingGroup produces and sends updates for the
RibOuts and Peers that belong to it. A SchedulingGroup is created/updated
such that all for all Peers, all RibOuts they advertise are part of the
same SchedulingGroup and for all RibOuts, all their member Peers are part
of the same SchedulingGroup. This design avoids contention when writing to
a Peer.

The current design works ok for eBGP Peers as we create multiple RibOuts
for the same BgpTable, based on the peer type, asn and other parameters.
This results in creation of a SchedulingGroup for each set of peers that
share a peer type ASN etc. These SchedulingGroups can work in parallel.
However, this doesn't help with iBGP peers since they all end up using
the same RibOut.

Current design does not work well for xmpp as we create a single RibOut
per table for xmpp. With large number of xmpp peers and a large number of
tables, we effectively send up with a single SchedulingGroup for all xmpp
peers.

Producing xmpp updates is more expensive than producing bgp updates due to
use of xml. This is exacerbated by xmpp update generation being limited to
a single SchedulingGroup. The changes implemented as part of bug 1591399
optimized single threaded xmpp updates.

Further improvements will likely require use of Tasks running in parallel.
This can be done in one of 2 ways:

1. Create multiple xmpp RibOuts for each BgpTable and then pick one of the
RibOuts when an xmpp peer subscribes to a table. If the RibOut is picked based on a hash of the peer name/ip, we get multiple SchedulingGroups,
which can then produce and send updates in parallel. This is relatively
simple to implement. However, results were quite poor when we tried this
in the past. Note that creating multiple xmpp RibOuts for all BgpTables
consumes a fair amount of extra memory and increases export processing
overhead since each route needs to be evaluated N times, once per RibOut.

2. Modify SchedulingGroupManager/SchedulingGroup design such that updates
for different table partitions can be generated in parallel by using one
Task per partition. This will create some contention when writing bgp/xmpp
update messages to a peer since multiple Tasks can do a write at the same
time. However, the contention should be pretty low with a large number of
peers.

Nischal Sheth (nsheth)
description: updated
Nischal Sheth (nsheth)
description: updated
description: updated
Revision history for this message
OpenContrail Admin (ci-admin-f) wrote : [Review update] master

Review in progress for https://review.opencontrail.org/23744
Submitter: Nischal Sheth (<email address hidden>)

Revision history for this message
OpenContrail Admin (ci-admin-f) wrote : A change has been merged
Download full text (4.0 KiB)

Reviewed: https://review.opencontrail.org/23744
Committed: http://github.org/Juniper/contrail-controller/commit/d05ca505177eb37e405e862a4f1a7aae9fce1bac
Submitter: Zuul
Branch: master

commit d05ca505177eb37e405e862a4f1a7aae9fce1bac
Author: Nischal Sheth <email address hidden>
Date: Thu Aug 25 17:00:40 2016 -0700

Redesign update generation code to improve performance

The SchedulingGroupManager design allows multiple SchedulingGroups to
work in parallel. Each SchedulingGroup produces and sends updates for
the RibOuts and Peers that belong to it. A SchedulingGroup is created
and updated such that all for all Peers, all RibOuts they advertise are
part of the same SchedulingGroup and for all RibOuts, all their member
Peers are part of the same SchedulingGroup.

The above design works well when there are many scheduling groups since
each of them can work in parallel. However, it does not work well for
xmpp as we create a single RibOut per table for xmpp. If there's a large
number of xmpp peers and a large number of tables, we effectively end
up with a single SchedulingGroup for all xmpp peers. This becomes the
main bottleneck in highly scaled scenarios with a large number of routes
and vrouters.

Implement a new design that creates a BgpSenderPartition per database
partition. A BgpSenderPartition effectively has the same functionality
as a SchedulingGroup. The main difference is that a BgpSenderPartition
contains all RibOuts and Peers - it's not created, updated and deleted
based on Peers joining/leaving RibOuts.

Additionally, shard the RibOutUpdates (and hence the UpdateQueues that
are contained within) for each RibOut instead of using a single set of
UpdateQueues for each RibOut.

The new design allows multiple BgpSenderPartitions to work in parallel
on the corresponding RibOutUpdates shard.

There are some downsides to the new design:

1. Update packing based on common attributes (applicable only for bgp)
is now limited to the scope of a shard instead of being done across the
board.

2. It can result in some lock contention if multiple BgpSenderPartitions
attempt to write to the same peer. This will not happen too often with a
large number of peers.

3. The amount of state compression is reduced to some extent since each
BgpSenderPartition independently discovers that a peer is blocked when
writing to the peer.

The benefits of being able to build and send updates for each shard in
parallel outweigh the downsides. Further, this design also allows (still
to be implemented) the possibility of not using any locks when producing
and consuming updates to/from UpdateQueues by making the producer and
consumer Tasks for any shard mutually exclusive.

This patch implements the following changes:

o Move code from scheduling_group.[cc|h] to bgp_update_sender.[cc|h] and
surgically remove all the SchedulingGroup split and merge related logic.

o The simplified SchedulingGroupManager is now called BgpUpdateSender.

o The simplified SchedulingGroup is now called BgpSenderPartition.

o Allocate N RibOutUpdates per RibOut based on number of DB partitions.

o Modify BgpExport to use the RibOutUpdates for the appropriate shard.

o Make RibOutUpdates::Qu...

Read more...

Revision history for this message
OpenContrail Admin (ci-admin-f) wrote : [Review update] master

Review in progress for https://review.opencontrail.org/24008
Submitter: Nischal Sheth (<email address hidden>)

Revision history for this message
OpenContrail Admin (ci-admin-f) wrote : A change has been merged

Reviewed: https://review.opencontrail.org/24008
Committed: http://github.org/Juniper/contrail-controller/commit/799c4d3d7e1d24050a86f32f5bfc47ad8ef451f9
Submitter: Zuul
Branch: master

commit 799c4d3d7e1d24050a86f32f5bfc47ad8ef451f9
Author: Nischal Sheth <email address hidden>
Date: Wed Sep 7 14:40:22 2016 -0700

Reduce memory allocation overhead in update generation code

Use static vectors of string and xml_document, 1 per BgpSenderPartition,
instead of creating and destroying them dynamically as BgpXmppMessages
are created and destroyed.

The vectors have an entry for each shard. This reduces memory allocation
overhead by allowing BgpXmppMessage allocated from a BgpSenderPartition
to use the string and xml_document associated with it's shard instead of
creating and destroying them repeatedly.

Note that pugixml library allocates memory for a document in increments
of 32KB pages and then manages smaller allocations (nodes or attributes)
using these pages. Pre-creating the xml_documents ensures that pugixml
does only a single 32KB allocation per document and then recycles the
same memory when building the tree for each route/item. Without this
optimization, we would allocate/free a 32KB page for each route/item.

Similarly, using a static vector of strings allows the same string to be
re-used by the BgpSenderPartition for a given shard instead of growing
the capacity of the string as routes are added to a BgpXmppMessage and
then destroying the string along with the BgpXmppMessage.

Change-Id: I8fb5d79c3d015c330bbc6409b8cd6180b2025479
Partial-Bug: 1607132

Revision history for this message
OpenContrail Admin (ci-admin-f) wrote : [Review update] master

Review in progress for https://review.opencontrail.org/24057
Submitter: Nischal Sheth (<email address hidden>)

Revision history for this message
OpenContrail Admin (ci-admin-f) wrote : A change has been merged

Reviewed: https://review.opencontrail.org/24057
Committed: http://github.org/Juniper/contrail-controller/commit/34560aa08a80d97d72376c0d49bd4af33810c60c
Submitter: Zuul
Branch: master

commit 34560aa08a80d97d72376c0d49bd4af33810c60c
Author: Nischal Sheth <email address hidden>
Date: Fri Sep 9 10:50:42 2016 -0700

Reuse xmpp message string for tracing

Change-Id: I10977b61fab1ae3b02ea6785bf5f6996df8e6b24
Partial-Bug: 1607132

Revision history for this message
OpenContrail Admin (ci-admin-f) wrote : [Review update] master

Review in progress for https://review.opencontrail.org/24078
Submitter: Nischal Sheth (<email address hidden>)

Revision history for this message
OpenContrail Admin (ci-admin-f) wrote :

Review in progress for https://review.opencontrail.org/24079
Submitter: Nischal Sheth (<email address hidden>)

Revision history for this message
OpenContrail Admin (ci-admin-f) wrote : A change has been merged

Reviewed: https://review.opencontrail.org/24078
Committed: http://github.org/Juniper/contrail-controller/commit/df1add65abb0a7c03f781596db21b31a8c25067c
Submitter: Zuul
Branch: master

commit df1add65abb0a7c03f781596db21b31a8c25067c
Author: Nischal Sheth <email address hidden>
Date: Mon Sep 12 09:39:31 2016 -0700

Fix breakage in bgp_xmpp_wready_test

The call to XmppChannelMuxMock::Send was being bypassed because
XmppChannelMuxMock did not define a Send with msg_str parameter.

Change-Id: Ie60239223e31dcd34fc62e93cda1b4f3b74fb64a
Partial-Bug: 1607132

Revision history for this message
OpenContrail Admin (ci-admin-f) wrote :

Reviewed: https://review.opencontrail.org/24079
Committed: http://github.org/Juniper/contrail-controller/commit/471cf88237b8b89653a376eeee3252cb3cae1097
Submitter: Zuul
Branch: master

commit 471cf88237b8b89653a376eeee3252cb3cae1097
Author: Nischal Sheth <email address hidden>
Date: Thu Sep 1 11:27:35 2016 -0700

Eliminate use of locks in update generation code

Make db::DBTable and bgp::SendUpdate mutually exclusive for a given
partition. Since the RibOutUpdates for a given partition only has a
single producer/consumer (db::DBTable/bgp::SendUpdate respectively),
we can remove the use of locks in UpdateQueue and RibUpdateMonitor.

In theory, we can get rid of RibUpdateMonitor altogether, but we're
leaving it alone for now.

Change-Id: I854ec4b2f6dbe94af4460c1254e5e5d5c9eae187
Partial-Bug: 1607132

Revision history for this message
OpenContrail Admin (ci-admin-f) wrote : [Review update] master

Review in progress for https://review.opencontrail.org/24285
Submitter: Nischal Sheth (<email address hidden>)

Revision history for this message
OpenContrail Admin (ci-admin-f) wrote : A change has been merged

Reviewed: https://review.opencontrail.org/24285
Committed: http://github.org/Juniper/contrail-controller/commit/156c97fc059f7d78fae435d23d89b7ceda17d784
Submitter: Zuul
Branch: master

commit 156c97fc059f7d78fae435d23d89b7ceda17d784
Author: Nischal Sheth <email address hidden>
Date: Mon Sep 19 10:00:43 2016 -0700

Allocate static vectors of bgp and xmpp messages

Allocate one bgp and xmpp message per DB partition and reuse
them instead of allocating and freeing messages repeatedly.
There's no issue with conccurrent access since only a single
bgp/xmpp message is used per DB partition at any given time.

Change-Id: I354440a7207283c1932a7a12ac502042cb96c64c
Partial-Bug: 1607132

Revision history for this message
OpenContrail Admin (ci-admin-f) wrote : [Review update] master

Review in progress for https://review.opencontrail.org/24310
Submitter: Ananth Suryanarayana (<email address hidden>)

Revision history for this message
OpenContrail Admin (ci-admin-f) wrote : A change has been merged

Reviewed: https://review.opencontrail.org/24310
Committed: http://github.org/Juniper/contrail-controller/commit/3bfdef473c1e9b72ce2cc3e21d20f9349b924d8a
Submitter: Zuul
Branch: master

commit 3bfdef473c1e9b72ce2cc3e21d20f9349b924d8a
Author: Ananth Suryanarayana <email address hidden>
Date: Tue Sep 20 12:26:32 2016 -0700

Fix xmpp unit tests linker issue

Change-Id: Ia2eff64da938ffe68ab180b21388564750226349
Partial-Bug: 1607132

Revision history for this message
OpenContrail Admin (ci-admin-f) wrote : [Review update] master

Review in progress for https://review.opencontrail.org/24328
Submitter: Nischal Sheth (<email address hidden>)

Revision history for this message
OpenContrail Admin (ci-admin-f) wrote :

Review in progress for https://review.opencontrail.org/24345
Submitter: Ananth Suryanarayana (<email address hidden>)

Revision history for this message
OpenContrail Admin (ci-admin-f) wrote :

Review in progress for https://review.opencontrail.org/24351
Submitter: Nischal Sheth (<email address hidden>)

Revision history for this message
OpenContrail Admin (ci-admin-f) wrote : A change has been merged

Reviewed: https://review.opencontrail.org/24328
Committed: http://github.org/Juniper/contrail-controller/commit/39937ea483087e701f609312af8f0ac7baa5bb86
Submitter: Zuul
Branch: master

commit 39937ea483087e701f609312af8f0ac7baa5bb86
Author: Nischal Sheth <email address hidden>
Date: Tue Sep 20 14:17:38 2016 -0700

Minor optimization to tracing in XmppConnection::Send

Change-Id: I003ab02554bdc782f8d17221de0eb57ace434db7
Partial-Bug: 1607132

Revision history for this message
OpenContrail Admin (ci-admin-f) wrote :

Reviewed: https://review.opencontrail.org/24351
Committed: http://github.org/Juniper/contrail-controller/commit/7d4cd116724c8b8fe78070b0c243425687dcfb14
Submitter: Zuul
Branch: master

commit 7d4cd116724c8b8fe78070b0c243425687dcfb14
Author: Nischal Sheth <email address hidden>
Date: Wed Sep 21 10:51:30 2016 -0700

Fix linking issue for ifmap and xmpp unit tests

Change-Id: Ie1db2869dca76d6eacd3ad050865f20fee0b5e6c
Partial-Bug: 1607132

Revision history for this message
OpenContrail Admin (ci-admin-f) wrote : [Review update] master

Review in progress for https://review.opencontrail.org/24364
Submitter: Nischal Sheth (<email address hidden>)

Revision history for this message
OpenContrail Admin (ci-admin-f) wrote : A change has been merged

Reviewed: https://review.opencontrail.org/24364
Committed: http://github.org/Juniper/contrail-controller/commit/b3661b9b8c5c58b30468b573977753b51f06eb18
Submitter: Zuul
Branch: master

commit b3661b9b8c5c58b30468b573977753b51f06eb18
Author: Nischal Sheth <email address hidden>
Date: Wed Sep 21 09:36:22 2016 -0700

Minor optimizations to speed up xmpp update encoding

o Change signature of IPeer::ToString and IPeer::ToUVEKey methods
o Add TcpSession::remote_addr_str() method

Change-Id: Ia0e8d7f88f432812559bad8d9d4d9526dd32cbc7
Partial-Bug: 1607132

Revision history for this message
OpenContrail Admin (ci-admin-f) wrote : [Review update] master

Review in progress for https://review.opencontrail.org/24397
Submitter: Nischal Sheth (<email address hidden>)

Revision history for this message
OpenContrail Admin (ci-admin-f) wrote : A change has been merged

Reviewed: https://review.opencontrail.org/24397
Committed: http://github.org/Juniper/contrail-controller/commit/7ac378ca20240dd4addc87fbd4a776a96fd60026
Submitter: Zuul
Branch: master

commit 7ac378ca20240dd4addc87fbd4a776a96fd60026
Author: Nischal Sheth <email address hidden>
Date: Thu Sep 22 11:59:40 2016 -0700

More minor tweaks to speed up xmpp updates

o Do not use integerToString to encode afi and safi
o Use fill and copy instead of string::replace because the latter
seems to allocate a new string internally to allow the possibility
of overlap between input and output strings

Change-Id: I4c90eff9f12588441a9c0610e21823723849d27d
Partial-Bug: 1607132

Revision history for this message
OpenContrail Admin (ci-admin-f) wrote : [Review update] master

Review in progress for https://review.opencontrail.org/24439
Submitter: Nischal Sheth (<email address hidden>)

Revision history for this message
OpenContrail Admin (ci-admin-f) wrote : A change has been merged

Reviewed: https://review.opencontrail.org/24439
Committed: http://github.org/Juniper/contrail-controller/commit/e50bfc997cec417fe0937874f3e21ff10ed3cf89
Submitter: Zuul
Branch: master

commit e50bfc997cec417fe0937874f3e21ff10ed3cf89
Author: Nischal Sheth <email address hidden>
Date: Sat Sep 24 20:58:35 2016 -0700

Use DB::PartitionCount() to set bgp::SendUpdate Task policy

Use DB::PartitionCount() to set bgp::SendUpdate policy instead of using
the hardware thread count. DB partition count is usually initialized to
be same as the hardware thread count, but could be set to a different
value in the future.

Change-Id: I1038c1d9b8bfb90dce23ae546d73245d98052642
Partial-Bug: 1607132

Revision history for this message
OpenContrail Admin (ci-admin-f) wrote : [Review update] master

Review in progress for https://review.opencontrail.org/24470
Submitter: Nischal Sheth (<email address hidden>)

Revision history for this message
OpenContrail Admin (ci-admin-f) wrote : A change has been merged

Reviewed: https://review.opencontrail.org/24470
Committed: http://github.org/Juniper/contrail-controller/commit/d1c29477eceba3e89bb232ba104f2104288dcf43
Submitter: Zuul
Branch: master

commit d1c29477eceba3e89bb232ba104f2104288dcf43
Author: Nischal Sheth <email address hidden>
Date: Mon Sep 26 20:54:16 2016 -0700

Make bgp::SendReadyTask and db::DBTable mutually exclusive

Make bgp::SendReadyTask and db::DBTable Tasks mutually exclusive so that
BgpSenderPartition::MaybeStartWorker does not get called concurrently.
Else, it can get called from BgpSenderPartition::SendReadyCallback and
BgpExport::Join and run into a variety of concurrency issues since we do
not use a mutex to protect the WorkQueue in BgpSenderPartition.

Change-Id: If95243565ffa33bb689c5d29d335a52dcb6c7ee6
Partial-Bug: 1607132

Revision history for this message
OpenContrail Admin (ci-admin-f) wrote : [Review update] master

Review in progress for https://review.opencontrail.org/24544
Submitter: Nischal Sheth (<email address hidden>)

Revision history for this message
OpenContrail Admin (ci-admin-f) wrote : A change has been merged

Reviewed: https://review.opencontrail.org/24544
Committed: http://github.org/Juniper/contrail-controller/commit/cbc08df5112ad400948c47d3f471bc9973858a6d
Submitter: Zuul
Branch: master

commit cbc08df5112ad400948c47d3f471bc9973858a6d
Author: Nischal Sheth <email address hidden>
Date: Wed Sep 28 11:42:21 2016 -0700

Improve UpdateQueue performance by using a vector instead of map

Improve UpdateQueue performance by using a vector instead of map to
maintain the bit index/position to UpdateMarker mapping. This helps
reduce the amount of time spent in GetMarker, Marker[Split|Merge].

Change-Id: I492569b90a85266ec02cfd2b917e9f8a339a5b20
Partial-Bug: 1607132

Revision history for this message
OpenContrail Admin (ci-admin-f) wrote : [Review update] master

Review in progress for https://review.opencontrail.org/24831
Submitter: Nischal Sheth (<email address hidden>)

Revision history for this message
OpenContrail Admin (ci-admin-f) wrote :

Review in progress for https://review.opencontrail.org/25139
Submitter: Nischal Sheth (<email address hidden>)

Revision history for this message
OpenContrail Admin (ci-admin-f) wrote :

Review in progress for https://review.opencontrail.org/24831
Submitter: Nischal Sheth (<email address hidden>)

Revision history for this message
OpenContrail Admin (ci-admin-f) wrote :

Review in progress for https://review.opencontrail.org/25139
Submitter: Nischal Sheth (<email address hidden>)

Revision history for this message
OpenContrail Admin (ci-admin-f) wrote :

Review in progress for https://review.opencontrail.org/24831
Submitter: Nischal Sheth (<email address hidden>)

Revision history for this message
OpenContrail Admin (ci-admin-f) wrote : [Review update] R3.2

Review in progress for https://review.opencontrail.org/25187
Submitter: Nischal Sheth (<email address hidden>)

Revision history for this message
OpenContrail Admin (ci-admin-f) wrote :

Review in progress for https://review.opencontrail.org/25188
Submitter: Nischal Sheth (<email address hidden>)

Revision history for this message
OpenContrail Admin (ci-admin-f) wrote : A change has been merged

Reviewed: https://review.opencontrail.org/25187
Committed: http://github.org/Juniper/contrail-controller/commit/ac0f819b9eea74ca54701ad11d3a731bb8811466
Submitter: Zuul
Branch: R3.2

commit ac0f819b9eea74ca54701ad11d3a731bb8811466
Author: Nischal Sheth <email address hidden>
Date: Wed Oct 12 15:41:44 2016 -0700

Make BgpSenderPartition::SetQueueActive more efficient

Have RibOutUpdates::TailDequeue fill in the bitset of unsync peers
based on the tail marker instead of building it by going through
all peers for the RibOut in BgpUpdateSender::SetQueueActive.

The new approach is more efficient because we only really need to
update the unsync peers based on the tail marker. Going through all
the unsync peers in the RibOut is unnecessary since the rest of the
peers will already have their queue active state updated previously
i.e. when they got blocked.

Consider a scenario where a RibOut has member peers 0-7 and the tail
marker for the RibOut has peers 0-5. Peers 6-7 got blocked earlier
and have their own marker(s). Further, assume that peers 4-5 are out
of sync i.e. they got blocked while doing TailDequeue for some other
RibOut.

If a TailDequeue is performed for the RibOut, the old code would go
through all the peers to build the sync and unsync bitsets, perform
TailDequeue, and then mark the RibOut as active for all unsync peers
4-7. In the new scheme, TailDequeue would return a bitset with peers
4-5 as unsync peers and BgpSenderPartition::SetQueueActive will only
mark the RibOut as active for peers 4-5. State for peers 6-7 does not
need to be updated since that would have been done at the time they
got blocked when doing TailDequeue for the same RibOut.

Also fix UTs in bgp_update_sender_test appropriately.

Change-Id: I9b567832e87a553af1bdd178bf29b13263526834
Partial-Bug: 1607132

Revision history for this message
OpenContrail Admin (ci-admin-f) wrote :

Reviewed: https://review.opencontrail.org/24831
Committed: http://github.org/Juniper/contrail-controller/commit/f8ed1ccf942c2a64ddf86989e6706e6653451ad1
Submitter: Zuul
Branch: master

commit f8ed1ccf942c2a64ddf86989e6706e6653451ad1
Author: Nischal Sheth <email address hidden>
Date: Wed Oct 12 15:41:44 2016 -0700

Make BgpSenderPartition::SetQueueActive more efficient

Have RibOutUpdates::TailDequeue fill in the bitset of unsync peers
based on the tail marker instead of building it by going through
all peers for the RibOut in BgpUpdateSender::SetQueueActive.

The new approach is more efficient because we only really need to
update the unsync peers based on the tail marker. Going through all
the unsync peers in the RibOut is unnecessary since the rest of the
peers will already have their queue active state updated previously
i.e. when they got blocked.

Consider a scenario where a RibOut has member peers 0-7 and the tail
marker for the RibOut has peers 0-5. Peers 6-7 got blocked earlier
and have their own marker(s). Further, assume that peers 4-5 are out
of sync i.e. they got blocked while doing TailDequeue for some other
RibOut.

If a TailDequeue is performed for the RibOut, the old code would go
through all the peers to build the sync and unsync bitsets, perform
TailDequeue, and then mark the RibOut as active for all unsync peers
4-7. In the new scheme, TailDequeue would return a bitset with peers
4-5 as unsync peers and BgpSenderPartition::SetQueueActive will only
mark the RibOut as active for peers 4-5. State for peers 6-7 does not
need to be updated since that would have been done at the time they
got blocked when doing TailDequeue for the same RibOut.

Also fix UTs in bgp_update_sender_test appropriately.

Change-Id: I9b567832e87a553af1bdd178bf29b13263526834
Partial-Bug: 1607132

Revision history for this message
OpenContrail Admin (ci-admin-f) wrote :

Reviewed: https://review.opencontrail.org/25139
Committed: http://github.org/Juniper/contrail-controller/commit/2020048b0810a94f44898a9dd1a9f3e489e92e72
Submitter: Zuul
Branch: master

commit 2020048b0810a94f44898a9dd1a9f3e489e92e72
Author: Nischal Sheth <email address hidden>
Date: Thu Oct 13 10:14:19 2016 -0700

Make BgpSenderPartition::UpdatePeerQueue more efficient

The old implementation used to go through all peers for a RibOut
and build a bitset of send ready peers to pass to PeerDequeue.
The bitset is used to decide which peers to merge with the start
marker when we encounter other markers while doing peer dequeue.

The new implementation determines which peers are send ready more
dynamically i.e. as we encounter other markers during peer dequeue.
This avoids unnecessary work in cases where we do not encounter
many markers. This could be because some markers are behind the
start marker for the peer or because the all peers in the start
marker get blocked or because some peers are in the tail marker.
Further, the new implementation determines the send ready state
directly from the RibOut instead of the BgpUpdateSender since the
RibOut state is more authoritative.

Also treat send_ready state in PeerState as advisory and send_ready
in the IPeerUpdate as authoritative. This if justified because the
IPeerUpdate could become blocked while processing updates from some
other BgpSenderPartition.

Also fix a few update related UTs appropriately.

Change-Id: Ib40d0a0d71c48f56e1af30962cc6f2459be4140a
Partial-Bug: 1607132

Revision history for this message
OpenContrail Admin (ci-admin-f) wrote :

Reviewed: https://review.opencontrail.org/25188
Committed: http://github.org/Juniper/contrail-controller/commit/08a04e6d9cda5f22825556b43ddba18c57bd0b72
Submitter: Zuul
Branch: R3.2

commit 08a04e6d9cda5f22825556b43ddba18c57bd0b72
Author: Nischal Sheth <email address hidden>
Date: Thu Oct 13 10:14:19 2016 -0700

Make BgpSenderPartition::UpdatePeerQueue more efficient

The old implementation used to go through all peers for a RibOut
and build a bitset of send ready peers to pass to PeerDequeue.
The bitset is used to decide which peers to merge with the start
marker when we encounter other markers while doing peer dequeue.

The new implementation determines which peers are send ready more
dynamically i.e. as we encounter other markers during peer dequeue.
This avoids unnecessary work in cases where we do not encounter
many markers. This could be because some markers are behind the
start marker for the peer or because the all peers in the start
marker get blocked or because some peers are in the tail marker.
Further, the new implementation determines the send ready state
directly from the RibOut instead of the BgpUpdateSender since the
RibOut state is more authoritative.

Also treat send_ready state in PeerState as advisory and send_ready
in the IPeerUpdate as authoritative. This if justified because the
IPeerUpdate could become blocked while processing updates from some
other BgpSenderPartition.

Also fix a few update related UTs appropriately.

Change-Id: Ib40d0a0d71c48f56e1af30962cc6f2459be4140a
Partial-Bug: 1607132

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.