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

GC default isn't really a default since it can't be overridden. #372

Open
felixdesouza opened this issue Dec 10, 2018 · 1 comment
Open

Comments

@felixdesouza
Copy link
Contributor

What happened?

We've narrowed it down to #312 which broke an internal product. This product had set its own gc setting (-XX:+UseParNewGC -XX:+UseConcMarkSweepGC - through launcher-custom.yml, and then after upgrading to this via excavator, the default - Throughput ("-XX:+UseParallelOldGC") - (which in this case isn't a default) applied a conflicting GC setting into launcher-static.yml => service would not start.

It's not really a default since it can't be overridden/doesn't respect conflicting overrides. I would have to know to override it by undoing the the -XX:+UserParallelOldGC at which point I should just use the GC flag with the feature => it's a breaking change.

What did you want to happen?

For the overridden gc options to continue to work with a minor version upgrade.

Suggest better behaviour

Instead of writing out the effect of this option to config, make go-java-launcher (or whatever is launching the service) aware of this gc flag. When it launches, it can then deconflict gc options. This plugin can't do the deconflicting, since it's only run as a build step, whereas launcher-custom.yml can be changed at runtime (with options respected upon node bounce), so it's not really the place for it imo.

I'm surprised this hasn't happened sooner, but it is surprising and I would not have expected it in a minor version bump.

@txangel
Copy link

txangel commented Mar 26, 2019

Also perhaps we should embrace G1 (:

@palantir palantir deleted a comment from yyli-palantir Apr 11, 2019
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

No branches or pull requests

2 participants