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

Fix Multiple GC declared error when running Druid Cluster #17078

Merged
merged 3 commits into from
Sep 20, 2024

Conversation

GWphua
Copy link
Contributor

@GWphua GWphua commented Sep 16, 2024

Description

This problem surfaced when I am attempting to get MM-less Druid running on ZGC with Kubernetes.

After declaring -XX:+UseZGC under JAVA_OPTS for my services, I faced the following error for only the Coordinator, Overlord and Router:
"Error occurred during initialization of VM, Multiple garbage collectors selected".

It turns out that the declaration of the G1GC in the cluster configuration causes the error to surface. I also find it weird that this G1GC configuration is only in the Coordinator/Overlord and Router configuration. By removing the G1GC configuration, I am able to run my cluster with ZGC.

Since G1GC is the default option, there will be no change to Druid's default GC with this removal.

Fixed the multiple GC declared error.

Release note


Allow Druid to run non-G1 Garbage Collectors with JAVA_OPTS.

This PR has:

  • been self-reviewed.
  • been tested in a test Druid cluster.

@GWphua GWphua changed the title Remove UseG1GC in cluster/auto to prevent multiple GC declared Error Remove UseG1GC in JVM Config for cluster/auto Sep 16, 2024
@GWphua GWphua changed the title Remove UseG1GC in JVM Config for cluster/auto Fix Multiple GC declared error when running Druid Cluster Sep 17, 2024
@kgyrtkirk
Copy link
Member

I think the default configs should not set such a thing - unless its really beneficial.
I haven't seen any measurements/etc why this was set initially in the commit logs

Its mentioned in https://druid.apache.org/docs/latest/operations/basic-cluster-tuning/ ; it seems like it was added as a suggestion in this PR.

I looked at -XX flags being set in the files below examples:

  • -XX:+ExitOnOutOfMemoryError looks reasonable :)
  • -XX:MaxDirectMemorySize seems okay as well
  • -XX:+UseG1GC (this)

I agree that the GC algo selection should be removed - as its important to ship something which works out of the box with the least amount of issues.

@kgyrtkirk
Copy link
Member

would you like to remove G1GC from other files under examples ?

@GWphua
Copy link
Contributor Author

GWphua commented Sep 18, 2024

@kgyrtkirk Thanks for the quick reply. I have removed the declaration in all files under examples.

@kgyrtkirk
Copy link
Member

note: there is no need to force push the branch as merge will be done with a squash (don't rebase your downstream branch)

Comment on lines -4 to -7
-XX:+UseG1GC
-XX:MaxDirectMemorySize=128m
-XX:+ExitOnOutOfMemoryError
-XX:+UseG1GC
Copy link
Member

Choose a reason for hiding this comment

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

lol :)

@GWphua
Copy link
Contributor Author

GWphua commented Sep 19, 2024

hi @kgyrtkirk

Thanks for approving my PR, can I trouble you to merge this PR now that the CI has passed? 😄

@kgyrtkirk kgyrtkirk merged commit a4c971c into apache:master Sep 20, 2024
90 checks passed
@kgyrtkirk
Copy link
Member

Thank you @GWphua for fixing this!

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.

2 participants