Accept raft role listeners at raft partition server

[Zelldon]

This feature allows the user to add raft role change listeners to the RaftPartitionGroup.

Would be nice if we could have that, we have the use case where we need to know who is the leader and listen for then role changes (whether we becoming the leader or step down). We want to start processing on leader nodes and stop the processing on followers.

[coveralls]

Coverage decreased (-0.1%) to 53.884% when pulling 38bc2b7 on Zelldon:raft-listener into 670e460 on atomix:master.

[Zelldon]

Hey @johnou, thanks for approving this PR. I rebased to master and fixed the latest checkstyle errors in the tests.

[Zelldon]

Anything else? 🙂

[kuujo]

My fat fingers posted a partial comment...

I think this is a good first step to fulfilling the feature request in #945.

The Partition interface currently provides a generic members/primary/backups list. In the case of Raft, primary and backups are the Raft leader and followers. Ideally we should add a generic event type to the Partition interface.

I need to take a closer look at this in the morning. Maybe we can add a generic PartitionEvent and only implement ListenerService for RaftPartition for now, then add the event to the other partition types separately.

[Zelldon]

Ideally we should add a generic event type to the Partition interface.

Agree. This PR was the easiest way for me to implement, since I was also not sure if is necessary on the other implementations (like primary backup).

Maybe we can add a generic PartitionEvent and only implement ListenerService for RaftPartition for now, then add the event to the other partition types separately.

Yes would be nice if we can have that.

Greets
Chris :)

[kuujo]

How should we go about it? I can add the necessary interfaces and then we can just update this PR to implement them. Does that make sense?

[Zelldon]

Yes why not 🙂 👍

[kuujo]

Okay should be good to go. Just need to additionally implement ListenerService<PartitionEvent, PartitionEventListener> in RaftPartition and modify the implementation a bit. Then I'll do the same for the other Partition implementations and eventually extend ListenerService in the Partition interface itself.

[Zelldon]

Ok rebased the branch. Locally I tested it with impl the ListenerService on RaftPartition. The Problem I had was on RaftContext, where I have to add and call the Listeners on #transition. To call the listeners we need a PartitionEvent, so we need to map the RaftRole changes to the PartitionEvent#Type and get the followers etc. So far so good, but how I get the PartitionId 😅

[kuujo]

Hmm... that's a good question let me look at it

[kuujo]

RaftPartition contains the PartitionId in it, so if the event is generated there, where the ListenerService is implemented, you have access to the PartitionId.

I just realized something, though. Really it seems the PartitionEvent should be produced in response to a client event, not a server event. Not all RaftPartitions will have local server instances, but they will all have local client instances. If the client is producing the events then that ensures all nodes see all events.

Maybe in the case of RaftPartition we should keep a role event and implement the PartitionEvent separately.

[Zelldon]

RaftPartition contains the PartitionId in it, so if the event is generated there, where the ListenerService is implemented, you have access to the PartitionId.

Ah thanks have overseen that.

Maybe in the case of RaftPartition we should keep a role event and implement the PartitionEvent separately.

So we will leave the PR as it is?

[Zelldon]

Hey @kuujo

what do you think? Can we merge this? Or should I improve anything else?

Greets,
Chris

[johnou]

@kuujo poke

[Zelldon]

Hey guys, any news on this?