Skip to content

[#2145] feat(server): The server-side supports choosing Netty's ByteBufAllocator.#2146

Closed
zhengchenyu wants to merge 1 commit intoapache:masterfrom
zhengchenyu:issue-2145
Closed

[#2145] feat(server): The server-side supports choosing Netty's ByteBufAllocator.#2146
zhengchenyu wants to merge 1 commit intoapache:masterfrom
zhengchenyu:issue-2145

Conversation

@zhengchenyu
Copy link
Collaborator

What changes were proposed in this pull request?

The server-side supports choosing Netty's ByteBufAllocator.

Why are the changes needed?

Fix: #2145

Does this PR introduce any user-facing change?

Introduce rss.server.netty.pooled.allocator.enabled to choose allocator.

How was this patch tested?

test in cluster.

@roryqi roryqi requested a review from rickyma September 25, 2024 07:50
@zhengchenyu zhengchenyu marked this pull request as draft September 25, 2024 07:56
@rickyma
Copy link
Contributor

rickyma commented Sep 25, 2024

The Uniffle server does not support PooledByteBufAllocator for now. You can refer to #1534.

@github-actions
Copy link

Test Results

 2 744 files   -  46   2 744 suites   - 46   5h 8m 4s ⏱️ - 16m 25s
 1 019 tests ±  0   1 016 ✅ + 1   1 💤 ±0  2 ❌  - 1 
12 562 runs   - 102  12 541 ✅  - 97  15 💤 ±0  6 ❌  - 5 

For more details on these failures, see this check.

Results for commit 84ab3d4. ± Comparison against base commit 8f4494b.

@zhengchenyu
Copy link
Collaborator Author

zhengchenyu commented Sep 25, 2024

The Uniffle server does not support PooledByteBufAllocator for now. You can refer to #1534.

Thanks for you quick reply! It is just a draft.
I think you mean that if we used pooled allocator, we allocate chunkSize, but DIRECT_MEMORY_COUNTER in netty is not matched with usedMemory in uniffle, then may result to OOM. Am I correct in my understanding?
If so, just close this pr.

@rickyma
Copy link
Contributor

rickyma commented Sep 25, 2024

Yes, you are right.

@zhengchenyu
Copy link
Collaborator Author

Yes, you are right.

Thanks for your reply! I will revert it in our repo, then I'm trying to use mimalloc that you talked about earlier.

@xianjingfeng
Copy link
Member

xianjingfeng commented Apr 16, 2025

I need this feature. I found that in our production environment, memory allocation and deallocation are very slow on the machines with large memory(1.5T). We will reserve sufficient memory, so I think the risk of OOM is relatively low for us. So I think this feature can be used as an option. cc @rickyma @zhengchenyu @jerqi @zuston

@zuston
Copy link
Member

zuston commented Apr 16, 2025

I need this feature. I found that in our production environment, memory allocation and deallocation are very slow on the machines with large memory(1.5T). We will reserve sufficient memory, so I think the risk of OOM is relatively low for us. So I think this feature can be used as an option. cc @rickyma @zhengchenyu @jerqi @zuston

Make sense to me. Let @zhengchenyu take a deep look.

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.

[FEATURE] The server-side supports choosing Netty's ByteBufAllocator

4 participants