[MDSAL-819] Deprecate Clustered(DOM)DataTreeChangeListener Created: 27/Mar/23  Updated: 20/Jan/24  Resolved: 19/Jan/24

Status: Resolved
Project: mdsal
Component/s: Binding API, DOM API
Affects Version/s: None
Fix Version/s: 13.0.0

Type: Improvement Priority: Medium
Reporter: Robert Varga Assignee: Ruslan Kashapov
Resolution: Done Votes: 0
Labels: pt
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Relates
relates to AAA-258 aaa-encrypt-service fails to start on... Resolved

 Description   

The distinction through a dedicated sub-class is quite weird and is a source of bugs (like AAA-258) and quite ugly hacks (in implementations).
Remove ClusteredDataTreeChangeListener interface and instead of that let the users specify the policy during registration.



 Comments   
Comment by Ruslan Kashapov [ 18/Sep/23 ]

The only place where ClusteredDataTreeChangeListener is actually differentiated from "non-clustered" DataTreeChangeListener is sal-distributed-datastore component of Controller. DataTreeChangeListenerProxy converts the listener interface difference into a boolean value registerOnAllInstances which then used in subsequent logic (akka message RegisterDataTreeChangeListener processed by DataTreeChangeListenerSupport#onMessage).

Specifying the policy on listener registration actually means moving the mentioned boolean property (defined using marking interface) from out of listener. So the new boolean property and listener instance now need to be delivered to same consumer independently of each other. 

Changing registration listener API is expected to be significant. Following basic interfaces and all the ancestors need to be changed in order to add new parameter to registerDataTreeChangeListener() API method :

  • DataTreeChangeService (mdsal-binding-api, extended by DataBroker)
  • DOMDataTreeChangeService (mdsal-dom-api)
  • DOMStoreTreeChangePublisher (mdsal-dom-spi)

All the affected implementations will require an update no mater the new parameter is taken into account (sal-distributed-datastore) or just ignored (all the other cases).

From other hand the condition of either listener should be enabled for notifications on each node or only on a leader is mainly defined by listener purpose (implementation):

  • if it serves the states synchronization (like cache or configuration update) for local services then it's expected to be enabled on each node
  • if it generates new data/events and multiple active instances (doing same) could cause conflicts and/or resource waste then the listener expected to be active on single node

It means the single/all nodes activation mode property actually defined by (belongs to) listener, no reason to extract it out. It's easier and more safe (reliable) just to convert extra listener interface into actual property of a listener with clear naming/description of the option purpose/usage – see https://git.opendaylight.org/gerrit/c/mdsal/+/107868

While this approach serves removal of extra listener interface and makes the clustered case  usage more clear, this is mainly a cosmetic change, no performance impact is expected. Some may find this update unnecessary bc using of marking interface already works as expected. The further changes need clarification either to continue the update or leave as is.

Comment by Robert Varga [ 18/Sep/23 ]

Thank you for exhaustive analysis. The task is still relevant as it was originally defined.

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