[OVSDB-364] A tons of unnecessary read operations in the switch update processing control flow Created: 19/Aug/16  Updated: 19/Oct/17  Resolved: 23/Aug/16

Status: Resolved
Project: ovsdb
Component/s: Southbound.Open_vSwitch
Affects Version/s: unspecified
Fix Version/s: None

Type: Bug
Reporter: Hideyuki Tai Assignee: Unassigned
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified
Environment:

Operating System: All
Platform: All

External issue ID: 6475

 Description   

The performance of the OVSDB Southbound Plugin to process Update Notification from Open vSwitch is really slow. One of reasons of the bad performance is that the plugin does a lot of unnecessary read operations on MD-SAL. The run() method of the TransactionInvokerImpl class calls a lot of the unnecessary read operations. This has a lot of bad impact, especially when we run several OpenDaylight nodes as a cluster in which an OpenDaylight node sometimes needs to communicate with another node (shard leader) to get data during the read operation.

We are thinking this bad performance probably causes the NETVIRT-5 which is a critical bug for NetVirt.
Therefore, we need to fix this problem. Especially, in this bug report, I would like to focus on removing the unnecessary read operations during processing Update Notification.

 Comments   
Comment by Hideyuki Tai [ 19/Aug/16 ]

I think the master, stable/boron, and stable/beryllium branches have this issue.

Comment by Hideyuki Tai [ 19/Aug/16 ]

I've noticed the OVSDB Southbound Plugin ALWAYS calls SouthboundUtil.readNode() SEVERAL TIMES on processing ONE Update Notification regardless on the contents of the Update Notification.
That's because some classes which extends AbstractTransactionCommand always call SouthboundUtil.readNode() in any conditions.

For example, please check the OvsdbQueueRemovedCommand.java.

[OvsdbQueueRemovedCommand.java]

49 @Override
50 public void execute(ReadWriteTransaction transaction) {
51 final InstanceIdentifier<Node> nodeIId = getOvsdbConnectionInstance().getInstanceIdentifier();
52 final Optional<Node> ovsdbNode = SouthboundUtil.readNode(transaction, nodeIId);
53 if (ovsdbNode.isPresent()) {

Here, this execute() method calls the SouthboundUtil.readNode() method without any condition check.

However, I think this method doesn't need the node information when it doesn't have any data to process, so this method should check condition first as follows.

[OvsdbQueueRemovedCommand.java (Modification proposed)]

47 @Override
48 public void execute(ReadWriteTransaction transaction) {
49 if (removedQueueRows == null || removedQueueRows.isEmpty() )

{ 50 return 51 }

52
53 final InstanceIdentifier<Node> nodeIId = getOvsdbConnectionInstance().getInstanceIdentifier();
54 final Optional<Node> ovsdbNode = SouthboundUtil.readNode(transaction, nodeIId);

Please note that every time run() method of the TransactionInvokerImpl class calls execute() method of OvsdbOperationalCommandAggregator class, execute() method of OvsdbQueueRemovedCommand class is always called by OvsdbOperationalCommandAggregator class.
I mean the OVSDB Southbound Plugin ALWAYS calls the exeucte() of the OvsdbQueueRemovedCommand class and calls the SouthboundUtil.readNode() every time the plugin receives Update Notification from Open vSwitch regardless on the contents of the Update Notification, even if Update Notification doesn't have any updates regarding to Queue table.

I'm writing a patch for this.
https://git.opendaylight.org/gerrit/#/c/44374/
I plan to modify other classes which extends AbstractTransactionCommand in the above patch.

Comment by Hideyuki Tai [ 22/Aug/16 ]

To make it easy for users to know the performance issue happens due to Update Notification from OVS, I've pushed a patch to log events regarding to the performance issue.
https://git.opendaylight.org/gerrit/#/c/44520/

And, the log messages which the above patch introduces also helps developers to improve the scalability, and identify the root cause of issues such as NETVIRT-5.

Comment by Hideyuki Tai [ 23/Aug/16 ]

There are patches which removed unnecessary read operations:

https://git.opendaylight.org/gerrit/#/c/44526/ (master)
https://git.opendaylight.org/gerrit/#/c/44374/ (stable/boron)
https://git.opendaylight.org/gerrit/#/c/44525/ (stable/beryllium)

Generated at Wed Feb 07 20:36:12 UTC 2024 using Jira 8.20.10#820010-sha1:ace47f9899e9ee25d7157d59aa17ab06aee30d3d.