Skip to content

Conversation

@hezhangjian
Copy link
Member

Motivation

Our DirectMemoryUtils has huge limit, it can't work well with other jvm. The Netty PlatformDependent.maxDirectMemory(); is more generic.

Changes

Use PlatformDependent.maxDirectMemory(); instead of DirectMemoryUtils

@hezhangjian
Copy link
Member Author

Copy link
Contributor

@dlg99 dlg99 left a comment

Choose a reason for hiding this comment

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

I don't think this is the right change. As I understand you have a situation when direct memory usage results in problems for other processes, possibly linux's OOM killer kicking in.

https://netty.io/4.1/xref/io/netty/util/internal/PlatformDependent.html#L153 describes how PlatformDependent.maxDirectMemory is controlled and used in netty. It doesn't limit direct memory for the JVM, -XX:MaxDirectMemorySize jvm parameter does.
The idea is to set -XX:MaxDirectMemorySize for the JVM and possibly limit (up to that max) for netty,

I suggest taking a look at the shell scripts for BK for examples of -XX:MaxDirectMemorySize usage and setting it to the appropriate value.
Please let me know if I misunderstood something.
.

@hezhangjian
Copy link
Member Author

@dlg99 Hi, We have a bookkeeper server, which is conifugred as "-Xmx6G -XDirectMemory4G". And we are using huawei jdk11. DirectMemoryUtils can't reed the sun.misc.VM. maxDirectMemory, fall back to the heap memory, which is 6G beyond the real -XDirectMemory.
Instead of enhace DirectMemoryUtils, we can use PlatformDependent.maxDirectMemory()

@dlg99
Copy link
Contributor

dlg99 commented Jan 20, 2022

Just switching to PlatformDependent.maxDirectMemory() feels like a misuse of it plus it might not work for some people if they e.g. set io.netty.maxDirectMemory greater than zero.
I'd try something like a configurable strategy for the DirectMemoryUtils to specify how to read the max memory (via Oracle JD specific methods, via Huawei's, via config parameter, etc)

@hezhangjian
Copy link
Member Author

@dlg99 You are right. set io.netty.maxDirectMemory greater than zero might not work.
So we can enhance the DirectMemoryUtils with netty's code(They are Apache LICENSE too)
PTAL, thanks

@hezhangjian
Copy link
Member Author

rerun failure checks

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

Did you copy paste the code from Netty?

@hezhangjian
Copy link
Member Author

Did you copy paste the code from Netty?

Yes, I did it because that these are more genernal, and netty is under apache license.

@eolivelli
Copy link
Contributor

What about sending a PR to Netty in order to expose such value?
Netty project is very open to contributions, I sent some PRs in the past.
They also have a very frequent release cycle.

I would prefer to not copy paste, because in Netty they are maintaining that code and adapt it to new Java versions, and we will keep this version forever

@hezhangjian
Copy link
Member Author

@eolivelli Thanks for your advice, I will try

@hezhangjian
Copy link
Member Author

@eolivelli @dlg99 netty/netty#12118 is merged. I will reopen this PR when we update to netty next version.

@hezhangjian
Copy link
Member Author

@eolivelli @dlg99 @merlimat @pkumar-singh PTAL again. Netty has exposed a util method for user to get maxDirectoryMemory. See netty/netty#12118

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

Great!

@hezhangjian
Copy link
Member Author

@eolivelli @dlg99 @merlimat @pkumar-singh @nicoloboschi
Please help merge this PR when you have time, thanks :)

@nicoloboschi nicoloboschi added this to the 4.16.0 milestone Apr 6, 2022
@nicoloboschi nicoloboschi merged commit d4eba62 into apache:master Apr 6, 2022
@hezhangjian hezhangjian deleted the direct-memory branch April 6, 2022 08:34
hezhangjian pushed a commit to apache/pulsar that referenced this pull request May 15, 2022
### Motivation
- `PlatformDependent.maxDirectMemory()` can be inaccurate if `io.netty.maxDirectMemory` are setted
- Bookkeeper's `DirectMemoryUtils` is not worked within some jdk releases.
In netty 4.1.75, they introduced a new method `PlatformDependent.estimateMaxDirectMemory` to help users get maxDirectMemory. Since netty's this method works well on many jdk releases, use this to replace below two.
PS: `DirectMemoryUtils` has been removed from bookkeeper in apache/bookkeeper#2989
### Modifications
- use `PlatformDependent.estimateMaxDirectMemory` instead
hezhangjian pushed a commit that referenced this pull request Jun 30, 2022
### Motivation
#2989

### Modification
Use estimateMaxDirectMemory instead of maxDirectMemory
Ghatage pushed a commit to sijie/bookkeeper that referenced this pull request Jul 12, 2024
### Motivation

Our `DirectMemoryUtils` has huge limit, it can't work well with other jvm. The Netty `PlatformDependent.maxDirectMemory();` is more generic.

### Changes
Use `PlatformDependent.maxDirectMemory();` instead of `DirectMemoryUtils`

Reviewers: Enrico Olivelli <[email protected]>, Andrey Yegorov <None>, Matteo Merli <[email protected]>, Nicolò Boschi <[email protected]>

This closes apache#2989 from Shoothzj/direct-memory
Ghatage pushed a commit to sijie/bookkeeper that referenced this pull request Jul 12, 2024
### Motivation
apache#2989

### Modification
Use estimateMaxDirectMemory instead of maxDirectMemory
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants