-
Notifications
You must be signed in to change notification settings - Fork 276
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
Development
: Add safeguards against split Hazelcast clusters
#8473
Conversation
Development
: (WIP) Add safeguards for split Hazelcast clustersDevelopment
: (WIP) Add safeguards against split Hazelcast clusters
Development
: (WIP) Add safeguards against split Hazelcast clustersDevelopment
: Add safeguards against split Hazelcast clusters
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.
Code changes look good to me 👍 great work!
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.
Tested on TS3 (cluster was split, but healed after a while) and LocalCI staging (I encountered two build queue states that resolved eventually)
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.
Actionable comments posted: 1
Out of diff range and nitpick comments (4)
src/main/java/de/tum/in/www1/artemis/config/CacheConfiguration.java (4)
Line range hint
158-252
: The configuration of the Hazelcast instance is comprehensive, covering various scenarios like local and production environments. However, consider documenting the implications of disabling multicast and auto-detection, as well as the choice of TCP/IP config, to ensure clarity for future maintainers.
Line range hint
253-261
: The configuration of the Hazelcast queue for local CI environments is appropriate. Consider adding more detailed comments explaining the choice of backup count and the role of the priority comparator class for better understanding.
Line range hint
262-267
: The serializer configuration for Path objects is crucial for caching files in the Hazelcast cluster. Consider adding comments to explain its importance and the choice of serializer implementation for future maintainers.
Line range hint
268-270
: The configuration of the key generator using Git and Build properties is a good practice for distinguishing cache entries. Consider documenting the rationale behind using prefixed keys to enhance clarity for future maintainers.
ef3d0f3
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.
Reapprove
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.
Code looks good to me 👍
Checklist
General
Server
I added multiple integration tests (Spring) related to the features (with a high test coverage).Not possible via JUnitMotivation and Context
Currently when starting Artemis it can happen that the Hazelcast cluster is not created properly and some nodes form separate cluster of varying sizes (1+). There is currently no way to fix this, except to restart the affected nodes. The prevention for this so far was to just start the nodes one-by-one with a delay of 20 seconds. This made deployment tedious.
Description
I added 2 changes to address this:
As a small side-quest I also added the possibility for nodes to have different Hazelcast ports between one another. This could theoretically replace the old local dev cluster handling, though I left it in place for now.
Steps for Testing
Scenario 1
Prerequisites:
Scenario 2
Prerequisites:
Testserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Review Progress
Performance Review
Code Review
Manual Tests
Screenshots
How the health panel should look like:
Summary by CodeRabbit
New Features
Style
BuildJobContainerService
to improve readability.