Skip to content

NIFI-12095 Increase default Xmx to 1g#7762

Closed
nandorsoma wants to merge 2 commits intoapache:mainfrom
nandorsoma:NIFI-12095
Closed

NIFI-12095 Increase default Xmx to 1g#7762
nandorsoma wants to merge 2 commits intoapache:mainfrom
nandorsoma:NIFI-12095

Conversation

@nandorsoma
Copy link
Contributor

Summary

NIFI-12095

Tracking

Please complete the following tracking steps prior to pull request creation.

Issue Tracking

Pull Request Tracking

  • Pull Request title starts with Apache NiFi Jira issue number, such as NIFI-00000
  • Pull Request commit message starts with Apache NiFi Jira issue number, as such NIFI-00000

Pull Request Formatting

  • Pull Request based on current revision of the main branch
  • Pull Request refers to a feature branch with one commit containing changes

Verification

Please indicate the verification steps performed prior to pull request creation.

Build

  • Build completed using mvn clean install -P contrib-check
    • JDK 21

Licensing

  • New dependencies are compatible with the Apache License 2.0 according to the License Policy
  • New dependencies are documented in applicable LICENSE and NOTICE files

Documentation

  • Documentation formatting appears as expected in rendered files

@nandorsoma nandorsoma marked this pull request as ready for review September 20, 2023 09:08
Copy link
Contributor

@exceptionfactory exceptionfactory left a comment

Choose a reason for hiding this comment

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

Thanks for proposing this change @nandorsoma. Increasing the default heap size seems reasonable. As noted in the details, the minimum heap size should be changed to match the maximum.

@@ -26,7 +26,7 @@
<properties>
<!--Wrapper Properties -->
<nifi.jvm.heap.init>512m</nifi.jvm.heap.init>
Copy link
Contributor

Choose a reason for hiding this comment

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

The minimum heap size should be changed to match the maximum heap size.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review @exceptionfactory. Just out of curiosity, what is the reason for that? I thought a rule of thumb is to initially have the minimum as the 50% of the maximum heap size.

Copy link
Contributor

Choose a reason for hiding this comment

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

Setting the minimum heap size to the same as the maximum makes the overall process memory usage more predictable. There are other factors that contribute to total process memory usage, but heap is usually the primary contributor. Starting out with the same amount of heap as the maximum avoids unexpected surprises when the heap grows to handle cached objects.

@joewitt
Copy link
Contributor

joewitt commented Sep 20, 2023

We're not trying to share with other services generally. We want the 1GB or whatever is set. But if users want the ability to let it flex they could do so. For now it is better in our applications case to just take what we want from the get go. I'm not sure it matters much in practice these days frankly.

@timeabarna
Copy link
Contributor

LGTM, merging to main and support/nifi-1.x

@joewitt
Copy link
Contributor

joewitt commented Sep 22, 2023

I left a comment on the JIRA. I only plan to merge to main. Don't see any reason to change the default at this point on the 1.x line.

@asfgit asfgit closed this in 9ae6921 Sep 22, 2023
arpadboda pushed a commit that referenced this pull request Sep 29, 2023
Signed-off-by: Joseph Witt <joewitt@apache.org>
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