-
Notifications
You must be signed in to change notification settings - Fork 33
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
Showing cluster state convergence #124
Conversation
This allows to show convergence: - when no convergence is reached yet, the heartbeat blinks in red - when a gossip was received in which all members have seen the latest state, the heartbeat blinks 3 times in green - it then reverts to blinking in white Some notes: - classes moved to package `akka.cluster.pi` or else we can't follow the (internal) event - gossip is slowed down so that it is possible to see things happen - this is only applied to exercise 2 so far, since it might be distracting if it is shown all the time (and because I didn't want to introduce a PR with changes to package structure in all exercises) I'll post a video about how this looks like online shortly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@manuelbernhardt
This looks pretty cool! Will give this a try to see it in action. I noticed that you renamed the package from org.neopixel
to akka.cluster.pi
which is something that should have been done a long time ago. This is a historic artefact from the very start of the project when I was taking the first steps to control the Adafruit Neopixel LED strips.
@@ -36,6 +36,13 @@ akka { | |||
"akka://pi-"${cluster-node-configuration.cluster-id}"-system@"${cluster-node-configuration.seed-node-2}":2550", | |||
"akka://pi-"${cluster-node-configuration.cluster-id}"-system@"${cluster-node-configuration.seed-node-3}":2550", | |||
"akka://pi-"${cluster-node-configuration.cluster-id}"-system@"${cluster-node-configuration.seed-node-4}":2550"] | |||
|
|||
gossip-interval = 2s | |||
gossip-time-to-live = 4s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@manuelbernhardt
Note: It's probably a good idea to add a comment stating that this parameter tweaking is done to slow down the convergence process for demo purposes and that this shouldn't be done unless one knows very well why. WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@manuelbernhardt
Tried it out and it works really nice!
- I would add some general explanation about the idea of slowing down the gossip communication in the exercise README. By the way, I ran the same code using the default params, and then the LED stay in the red blinking state. That shouldn't happen, isn't it?
- Good point to change the package for the
ClusterStatusTracker
: this should be applied to all exercises. I will create an issue for that to follow-up separately.
@eloots thanks! If you set |
Thanks! That clarifies matters. I'll go ahead and merge. Also, as you mentioned, maybe keep this feature limited to this exercise? |
This allows to show convergence:
Some notes:
akka.cluster.pi
or else we can't follow the (internal) eventI'll post a video about how this looks like online shortly.