Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure that link_up and link_down notifications are only sent once and if the prior status (metadata.last_status) is different #122

Closed
viniarck opened this issue Mar 16, 2023 · 1 comment
Labels
enhancement New feature or request priority_major Major priority

Comments

@viniarck
Copy link
Member

Although we have solved some redundant kytos/topology.[link_up|link_down] notifications to keep it simple for subscribers to trust that this event is actually changing the status state it's claiming to change on #114, currently, for logical protocols like link liveness and similar (or future) ones, they can still notify redundant state, this typically doesn't cause a major issue, but ultimately it can end up making it harder for subscribers to also ignore redundant events, for instance, if a link.status is UP and a logical protocol like liveness went up then, a kytos/topology.link_up shouldn't be sent again, but if when liveness (or a similar logical protocol) goes up and then the link prior to this was down then it should notify.

Notice that link.status is a dynamic attribute, and currently, we don't have a way to store the prior status state, that's why we need metadata.last_status, we've been using something similar with metadata.last_status_is_active that solved for topology to avoid some of the redundant events when a pair of interface goes up/down, but then we should also further generalize this metadata.last_status_is_active into metadata.last_status and then make sure it works for all protocols #99. In the end, it should also use a threading.Lock something equivalent to this part here, notice that with a lock, it first read the prior state, confirm what it was and then it's ready to update it and publish notification, since logical protocols will have concurrent notifications and they go up and down based on their own state machines or behaviors.

This is a desirable feature to have, currently, it's not creating a major issue, and in the worst case it can cause a redeploy, we may or not ship this on 2023.1 it'll depend how things evolve and if the team still has capacity this. I'll set the priority as major.

@viniarck
Copy link
Member Author

viniarck commented Jul 5, 2023

This landed on #136. It's not using metadata, since it was decided to keep track using a set in memory.

@viniarck viniarck closed this as completed Jul 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request priority_major Major priority
Projects
None yet
Development

No branches or pull requests

1 participant