MDSAL best practices (NETVIRT-1318)

[NETVIRT-1339] Convert elanmanager to datastore-constrained transactions Created: 25/Jun/18  Updated: 03/Oct/18  Resolved: 03/Oct/18

Status: Resolved
Project: netvirt
Component/s: elanmanager
Affects Version/s: None
Fix Version/s: Fluorine-SR1, Neon

Type: Sub-task Priority: Medium
Reporter: Stephen Kitt Assignee: Manu B
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified


 Comments   
Comment by Vishal Thapar [ 02/Jul/18 ]

ezghodh If you're not actively working on this please unassign yourself so someone else can work on this.

Comment by Manu B [ 04/Jul/18 ]

Review added doesn't use DS constraint txn at all the places. It is difficult to add where same transaction is used for both config and oper. Similarly it is difficult to use for places where blocking calls are used.

Comment by Stephen Kitt [ 04/Jul/18 ]

Could you give an example where blocking calls make it difficult to use?

Comment by Manu B [ 04/Jul/18 ]

For eg: deleteElanInterfaceForwardingTablesList in ElanForwardingEntriesHandler.java is called from 2 places( one from deleteElanInterfaceForwardingEntries and updateElanInterfaceForwardingTablesList from same file).  It is easy to move to DS constraint tx in deleteElanInterfaceForwardingEntries  but difficult at updateElanInterfaceForwardingTablesList. This is because updateElanInterfaceForwardingTablesList is called from update method of ElanInterfaceManager.java which doesn't use txRunner(due to blocked call).

This resulted in not using DS constraint txn in both deleteElanInterfaceForwardingEntries and updateElanInterfaceForwardingTablesList.

Please let me know if there is any better way to use it.

 

Comment by Stephen Kitt [ 04/Jul/18 ]

The better way is to use an existing transaction in `ElanInterfaceManager::update`, without blocking. (Blocking shouldn’t be necessary if all the dependencies are in the same transaction.)

Comment by Manu B [ 05/Jul/18 ]

Do we need to take up removal of blocking calls in the current activity? It could have impact in the functionalities. There are 5 usage of ElanUtils::waitForTransactionToComplete.(blocking calls)
Some are related to data dependencies and there are already work going on txn chaining those areas. Other usages seems to be added for race conditions.

Please let me know your views.

Comment by Stephen Kitt [ 05/Jul/18 ]

As always, if it makes the patch unwieldy, it’s fine to split the patch up. Especially if work is being done in other patches to address these issues, it makes sense to push an initial patch converting what can easily be converted, and then follow up with another patch once the other patches have been merged. (But this bug shouldn’t be marked as “done” until everything is done.)

Note that converting code to use a single transaction which is passed down avoids a number of these issues: changes made in a transaction are visible inside that transaction, which means some waits can be removed. There should also be fewer race conditions as a result of this work.

Comment by Michael Vorburger [ 25/Jul/18 ]

with https://git.opendaylight.org/gerrit/#/c/74026/ not yet, let's re-use this issue, for clear status.

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