[BGPCEP-904] Graph-impl is performing cross-datastore transactions Created: 23/Apr/20  Updated: 03/Aug/20  Resolved: 03/Aug/20

Status: Resolved
Project: bgpcep
Component/s: General
Affects Version/s: None
Fix Version/s: Aluminium

Type: Bug Priority: High
Reporter: Robert Varga Assignee: Olivier Dugeon
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified


 Description   

ConnectedGraphServer is populating both Operational and Configuration datastores in a single transaction.

This leads to sub-optimal performance and is deprecated funcitonality, which is going away in the next release.

 



 Comments   
Comment by Olivier Dugeon [ 23/Apr/20 ]

Good catch. Indeed, ConnectedGraphServer initializes both Operational and Configuration datastore. Initialize Configuration is wrongly placed. I'll split initialization in two separate transactions.

Comment by Robert Varga [ 23/Apr/20 ]

I wonder, what is the intent in actually writing to CONFIG here? I mean that's persisted by default, so should it really be initialized each startup...

Comment by Olivier Dugeon [ 23/Apr/20 ]

Graph in Config Datastore is here to give the possibility to manually provide a topology or complete an automatic topology discovery (e.g. through BGP-LS). But Path Algorithms are working on the connected graph which is computed from the Operational Graph. So, GraphListener is here to listen to Graph Configuration and populate Graph Operational like LinkstateGraphBuilder done with Linkstate Routes.

If Graph Configuration is not initialized, I'm afraid that GraphListener could not start correctly. I'm thinking to initialize Graph Configuration in GraphListener class, but, this is more complex as simply split the Graph initialization in two separate transactions.

Now, regarding persistence, I could add some trigger to initialize the Graph Configuration only if it's not exist if it makes sense.

Comment by Robert Varga [ 23/Apr/20 ]

Why would the GraphListener not start? I mean it should be just fine with no config being present, right?

Also, the init actually actively overwrite config datastore – so even if there was something there, you'll end up wiping it on start – that does not feel like config at all. As for the BGP-LS discovery, etc. that certainly should not go through config – it very much feels liek you really want to have multiple instances in oper and each instance having a semantic meaning. The user-supplied configuration (which would be persisted) would live in config, but code components would never actively write to it and instead move it to oper – this is very much what BGP already does anyway ...

Comment by Olivier Dugeon [ 23/Apr/20 ]

Why would the GraphListener not start? I mean it should be just fine with no config being present, right?

I try to launch the Graph without the creation of initial GraphModel top node in config datastore, but in this case, you are unable to add some graph in config. I need at least to create the graph-model top node in the config datastore. Now, I agree that it is certainly the role of GraphListnener to do that instead of ConnectedGraphServer.

Also, the init actually actively overwrite config datastore – so even if there was something there, you'll end up wiping it on start – that does not feel like config at all.

I just would create the graph-model top node in the config datastore like I do for the operational datastore. Perhaps we could do this differently with some xml file. But well, It is just the top node with no graph inside. Just let me know what it is the better design.

As for the BGP-LS discovery, etc. that certainly should not go through config – it very much feels liek you really want to have multiple instances in oper and each instance having a semantic meaning. The user-supplied configuration (which would be persisted) would live in config, but code components would never actively write to it and instead move it to oper – this is very much what BGP already does anyway ...

As mention, Graph model in configuration is just here to give the possibility to provide a graph with the REST API instead of through BGP-LS. This allows an operator to provide a network topology that it collect through another tools which not speaks BGP-LS. This could be also helpful to provide static topology or complement a topology acquired through BGP-LS. In all cases, the active topology on which Path Computation Algorithms oper is the Connected Graph Model issue from the Graph Model in operational datastore.

Just let me know where is the best place to create the graph-model top node for the Configuration datastore

Comment by Olivier Dugeon [ 23/Apr/20 ]

So, I finally simply drop initialization of GraphModel in Configuration datastore

Generated at Wed Feb 07 19:14:27 UTC 2024 using Jira 8.20.10#820010-sha1:ace47f9899e9ee25d7157d59aa17ab06aee30d3d.