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

Incorrect usage of emit() or incorrect comment #207

Open
carlcsaposs-canonical opened this issue Mar 19, 2024 · 3 comments
Open

Incorrect usage of emit() or incorrect comment #207

carlcsaposs-canonical opened this issue Mar 19, 2024 · 3 comments

Comments

@carlcsaposs-canonical
Copy link
Contributor

This block of code does not behave as the comment describes

# since when an IP change happens, "_on_peer_relation_joined" won't be called,
# we need to alert the leader that it must recompute the node roles for any unit whose
# roles were changed while the current unit was cut-off from the rest of the network
self.on[PeerRelationName].relation_joined.emit(
self.model.get_relation(PeerRelationName)
)

relation-joined will only be emitted on that unit (where

if self.opensearch_config.update_host_if_needed():
evaluates to True)

And that unit will immediately return

def _on_peer_relation_joined(self, event: RelationJoinedEvent):
"""Event received by all units when a new node joins the cluster."""
if not self.unit.is_leader():
return

More info:

Note that the emission of custom events is handled immediately. In other words, custom events are not queued, but rather nested. For example:

1. Main hook handler (emits custom_event_1)
2.   Custom event 1 handler (emits custom_event_2)
3.     Custom event 2 handler
4.   Resume custom event 1 handler
5. Resume main hook handler

https://ops.readthedocs.io/en/latest/#ops.BoundEvent.emit

Copy link

@carlcsaposs-canonical
Copy link
Contributor Author

Potential solution: instead of emitting custom event, write something to unit peer databag (and ensure peer databag is updated [i.e. don't write data that's already there]) to trigger a peer-relation-changed event on all other units

@Mehdi-Bendriss
Copy link
Contributor

Mehdi-Bendriss commented Mar 19, 2024

This looks like a bug indeed. Thanks!

In this case, I don't think we unfortunately can get away from emitting the event, in case the current unit is the elected leader unit - which will then not receive a peer-rel-changed event if the data is stored in its unit data bag..

I believe the fix is a mix of both approaches, something along the lines of:

if self.unit.is_leader():
    self.on[PeerRelationName].relation_joined.emit( 
        self.model.get_relation(PeerRelationName) 
    )
self.peers_data.put(
    Scope.UNIT, "updated_at", datetime.now().timestamp()
)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants