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: the launchers memory should not effect the node memory #1221

Merged
merged 3 commits into from
Jul 9, 2023

Conversation

0utplay
Copy link
Member

@0utplay 0utplay commented May 9, 2023

Motivation

The way our launcher works is that all jvm options of the launcher get passed down to the started node instance. Therefore increasing the memory in one of the start files would result in the memory increasing for two different processes.

  1. The launcher process it self start is started by the start file
  2. The memory of the new node process started by the launcher

So that means that the memory consumption is doubled.

Modification

Made sure to remove the -xmx and -xms options of the launcher before passing the remaining options to the node and introduced a new option in the launcher.cnl.
The new option cloudnet.node.memory is the new way to set the memory of the node (in megabytes).

Result

The memory is not occupied twice and the node memory is not dependant on the launcher.

@0utplay 0utplay added v: 4.X This pull should be included in the 4.0 release t: fix A pull request introducing a fix for a bug. in: launcher An issue/pull request releated to the common launcher code labels May 9, 2023
@0utplay 0utplay added this to the 4.0.0-RC9 milestone May 9, 2023
@0utplay 0utplay requested a review from derklaro May 9, 2023 14:04
@0utplay 0utplay self-assigned this May 9, 2023
@0utplay 0utplay requested a review from derklaro May 16, 2023 09:01
@github-actions
Copy link

github-actions bot commented May 16, 2023

Test Results

  47 files  ±0    47 suites  ±0   1m 14s ⏱️ +2s
322 tests ±0  322 ✔️ ±0  0 💤 ±0  0 ±0 
652 runs  ±0  652 ✔️ ±0  0 💤 ±0  0 ±0 

Results for commit 679e1c7. ± Comparison against base commit daa3096.

♻️ This comment has been updated with latest results.

@derklaro derklaro merged commit 3c10d5b into nightly Jul 9, 2023
@derklaro derklaro deleted the feat-split-node-launcher-memory branch July 9, 2023 10:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: launcher An issue/pull request releated to the common launcher code t: fix A pull request introducing a fix for a bug. v: 4.X This pull should be included in the 4.0 release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants