[supervisor] remove JAVA_TOOL_OPTIONS hack #19630
Conversation
|
How can we get a better understanding of the side effects? I.e. how do we know what impact this will have on other customers? |
|
@csweichel I think we will need to test with Java <= 10 and see what happens. It seems it could cause some issues with allocating desired amount of memory, maybe process crash on the startup. |
|
@csweichel do you remember why this was introduced? I can't find any related conversation in issues, PRs or slack. |
|
According to JetBrains about 50% of all Java usage is Java8, i.e. would be affected by this change. The current situation also isn't great. 
|
I don't think it is about IntelliJ itself, we already unset JAVA_TOOL_OPTIONS for IntelliJ backend, because we use default Xmx settings there. I think mostly it will be affected other user loads, like user apps, not IDEs. It maybe alright, if they fail to start then a user has to adjust Xmx |
|
It's not about IntelliJ at all.
How would a user (for whom things probably worked just fine until now), know that? |
Java LS starts crashing in VS Code, some user apps.
I guess we will see something from IDE logs started by us, but any load from user terminals or VS Code Desktop won't be visible to us. I would say we will need to test and check logs first to know what to look for. |
|
Can we feature flag it and rollout to selected customers first? |
Users would know that their stuff is crashing. How would they know what they need to do? |
svenefftinge
force-pushed
the
se/jvm-xmx
branch
from
998bec5
to
009c80e
Compare
| if cfg.ConfigcatEnabled { | ||
| experimentsClientOpts = append(experimentsClientOpts, experiments.WithGitpodProxy(host)) | ||
| } | ||
| exps := experiments.NewClient(experimentsClientOpts...) |
There was a problem hiding this comment.
there should be already exps defined in this file
There was a problem hiding this comment.
There was a problem hiding this comment.
yes, I moved that up
|
/unhold |
| @@ -41,3 +42,7 @@ func SupervisorPersistServerAPIChannelWhenStart(ctx context.Context, client Clie | |||
| func SupervisorUsePublicAPI(ctx context.Context, client Client, attributes Attributes) bool { | |||
| return client.GetBoolValue(ctx, SupervisorUsePublicAPIFlag, false, attributes) | |||
| } | |||
|
|
|||
| func IsSetJavaXmx(ctx context.Context, client Client, attributes Attributes) bool { | |||
| return client.GetBoolValue(ctx, SetJavaXmxFlag, false, attributes) | |||
There was a problem hiding this comment.
@svenefftinge default should be true, for backward compatibility? not sure
There was a problem hiding this comment.
I'd like to go the other way around and see how it's going. Ideally we can get rid of this special hack and users do this themselves.
There was a problem hiding this comment.
ok, it is just if there is a connection hiccups or so it maybe impossible to revert this change via the feature flag
Description
Remove setting Xmx to the containers memory size
Related Issue(s)
Fixes #
How to test
Documentation
Preview status
Gitpod was successfully deployed to your preview environment.
Build Options
Build
Run the build with werft instead of GHA
Run Leeway with
--dont-testPublish
Installer
Add desired feature flags to the end of the line above, space separated
Preview Environment / Integration Tests
If enabled this will build
install/previewIf enabled this will create the environment on GCE infra
Saves cost. Untick this only if you're really sure you need a non-preemtible machine.
Valid options are
all,workspace,webapp,ide,jetbrains,vscode,ssh. If enabled,with-previewandwith-large-vmwill be enabled./hold