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

Upgrade dependency hazelcast from 3.12.5 to 5.3.7 #94

Merged
merged 6 commits into from
Sep 6, 2024

Conversation

iarwen
Copy link

@iarwen iarwen commented Jun 20, 2024

Upgrade dependency hazelcast from 3.12.5 to 5.3.7, tested with 3 nodes on k8s.

@iarwen iarwen mentioned this pull request Jun 20, 2024
Copy link
Member

@guusdk guusdk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for taking the time to implement and test this upgrade. It is much appreciated!

The change generally looks good, but I've left a number of remarks (most of them are pretty minor).

Apart from these remarks: should we provide documentation on how to upgrade from one version to another version? Is there something that administrators need to be aware of?

classes/hazelcast-cache-config.xml Outdated Show resolved Hide resolved
src/web/system-clustering-node.jsp Outdated Show resolved Hide resolved
src/web/system-clustering-node.jsp Outdated Show resolved Hide resolved
src/web/system-clustering-node.jsp Outdated Show resolved Hide resolved
classes/hazelcast-cache-config.xml Outdated Show resolved Hide resolved
classes/hazelcast-cache-config.xml Show resolved Hide resolved
classes/hazelcast-cache-config.xml Show resolved Hide resolved
classes/hazelcast-cache-config.xml Show resolved Hide resolved
pom.xml Show resolved Hide resolved
@iarwen iarwen requested a review from guusdk June 24, 2024 02:23
@iarwen
Copy link
Author

iarwen commented Jun 24, 2024

Any other discussion for this PR? still wait for your suggestions @guusdk @akrherz.

@guusdk guusdk requested a review from GregDThomas June 25, 2024 09:18
@guusdk
Copy link
Member

guusdk commented Jun 25, 2024

@iarwen thanks for your changes!

I have the following remaining concerns:

  • I've unresolved two discussions, for you to create a new PR for the related changes.
  • Why did you update to 5.3.7 and not the latest release? Can we upgrade to the latest release?
  • Do we need explicit instructions/documentations for people to upgrade from an old version to a new version of the plugin. Do we need to revise the existing plugin documentation?

I've also pinged @GregDThomas to see if they're interested in reviewing this, given that they have a lot of experience with the plugin.

Copy link
Contributor

@GregDThomas GregDThomas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @guusdk's comments, added a couple of my own in the area of the config.

classes/hazelcast-local-config.template.xml Outdated Show resolved Hide resolved
classes/hazelcast-local-config.template.xml Outdated Show resolved Hide resolved
classes/hazelcast-local-config.template.xml Show resolved Hide resolved
@iarwen
Copy link
Author

iarwen commented Jun 28, 2024

@iarwen thanks for your changes!

I have the following remaining concerns:

  • I've unresolved two discussions, for you to create a new PR for the related changes.
  • Why did you update to 5.3.7 and not the latest release? Can we upgrade to the latest release?
  • Do we need explicit instructions/documentations for people to upgrade from an old version to a new version of the plugin. Do we need to revise the existing plugin documentation?

I've also pinged @GregDThomas to see if they're interested in reviewing this, given that they have a lot of experience with the plugin.

  • Updated the unresolved discussions and marked as resolved, bugfix PR had created Fix memory usage status #95.
  • The latest version 5.4.0 need the minimum supported Java version is 17. https://github.com/hazelcast/hazelcast/releases/tag/v5.4.0, that not match the minimum version of openfire.
  • The existing plugin documentation looks good and enough except some link to the 3.12.5 document, if user use their old config file hazelcast-local-config.xml and use this 5.3.7 version to start up cluster, will occur some warning and information, but will not prevent the cluster startup. The logs looks like:
    2024.06.28 13:44:23 WARN [PluginMonitorTask-2]: com.hazelcast.cp.CPSubsystem - [10.42.0.165]:5701 [openfire-cluster-by-hazelcast] [5.3.7] CP Subsystem is not enabled. CP data structures will operate in UNSAFE mode! Please note that UNSAFE mode will not provide strong consistency guarantees.

I had updated the readme.html file.

@iarwen iarwen requested a review from GregDThomas June 28, 2024 15:00
Copy link
Contributor

@GregDThomas GregDThomas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One trivial comment, but happy that my changes are dealt with.

@arelkar
Copy link

arelkar commented Aug 22, 2024

Hi @GregDThomas @guusdk @iarwen,
What are your plans to merge this PR and release openfire-hazelcast plugin 3.x.x version? Can you share tentative timelines?

There are bunch of CVEs reported for hazelcast, looking forward to new version.
https://mvnrepository.com/artifact/com.hazelcast/hazelcast/3.12.5
Eg: https://nvd.nist.gov/vuln/detail/CVE-2023-33264

Appreciate your response. Thank you.

@guusdk guusdk merged commit e00c345 into igniterealtime:main Sep 6, 2024
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

Successfully merging this pull request may close these issues.

4 participants