kytos-ng / pathfinder

Kytos main path finder Network Application (NApp)
https://kytos-ng.github.io/api/pathfinder.html
MIT License
0 stars 7 forks source link

Possible error when handling ownership link metadata #53

Closed italovalcy closed 1 year ago

italovalcy commented 1 year ago

Hi,

When trying to use ownership link metadata attribute, I got the following error:

2023-07-03 14:40:04,644 - INFO [werkzeug] [_internal.py:225:_log] (Thread-122085) 127.0.0.1 - - [03/Jul/2023 14:40:04] "POST /api/kytos/topology/v3/links/048a97e782ee6e336b111248c97e0d523fa44a2c317d068203075369746c8f98/metadata HTTP/1.1" 201 -
2023-07-03 14:40:23,747 - INFO [werkzeug] [_internal.py:225:_log] (Thread-122104) 127.0.0.1 - - [03/Jul/2023 14:40:23] "POST /api/kytos/topology/v3/links/b554ee5eafb54898cc54d723afa68001dd838e9e69d266c68cc1b7e7ff03036d/metadata HTTP/1.1" 201 -
2023-07-03 14:40:23,781 - WARNING [kytos.napps.kytos/pathfinder] [main.py:268:update_links_metadata_changed] (thread_pool_app_57) Unexpected KeyError '00:00:00:00:00:16:00:02:30' on event kytos/topology.links.metadata.added. pathfinder will reconciliate the topology

The request was basically:

# curl -X POST -H 'Content-type: application/json' http://127.0.0.1:8181/api/kytos/topology/v3/links/048a97e782ee6e336b111248c97e0d523fa44a2c317d068203075369746c8f98/metadata -d '{"ownership": "Monet"}'
"Operation successful"
# curl -X POST -H 'Content-type: application/json' http://127.0.0.1:8181/api/kytos/topology/v3/links/b554ee5eafb54898cc54d723afa68001dd838e9e69d266c68cc1b7e7ff03036d/metadata -d '{"ownership": "Monet"}'
"Operation successful"

# curl -s http://127.0.0.1:8181/api/kytos/topology/v3/links | jq -r '.links[] | .id + " " + .metadata.ownership'
048a97e782ee6e336b111248c97e0d523fa44a2c317d068203075369746c8f98 Monet
b554ee5eafb54898cc54d723afa68001dd838e9e69d266c68cc1b7e7ff03036d Monet
viniarck commented 1 year ago

@italovalcy,

Analyzing the code and the race conditions possibilities, I managed to simulate an equivalent problem, by forcing a preemption just so on_links_metadata_changed would execute before update_topology:

kytos $> 2023-07-06 15:21:30,020 - INFO [uvicorn.access] [httptools_impl.py:506:send] (MainThread) 127.0.0.1:60350 - "POST /api/kytos/topology/v3/links/c8b55359990f89a5849813dc348d30e9e
1f991bad1dcb7f82112bd35429d9b07/metadata HTTP/1.1" 201
2023-07-06 15:21:30,021 - WARNING [kytos.napps.kytos/pathfinder] [main.py:186:update_links_metadata_changed] (thread_pool_app_15) Unexpected KeyError '00:00:00:00:00:00:00:01:4' on even
t kytos/topology.links.metadata.added. pathfinder will reconciliate the topology
kytos $>                                                                                                                                                                                 

In retrospect, due to also threaded access race, although we have a lock, still how on_links_metadata_changed is performing writes it's assuming that the graph would've been properly updated before, so in addition to using a lock we'd need more logic to control the order of the access. Taking a step back, I think the most appropriate solution would be what I proposed on issue #54 (which also would fix another bug that has surfaced), in summary, only rely on kytos.topology.[updated|topology_loaded|current] to have a single handler responsible for the update, instead of having kytos/topology.links.metadata.(added|removed) updating too, simplifying the threaded access logic, it was not worth splitting this. If you have any other better and simpler idea let me know.