Skip to content

Conversation

@rickyma
Copy link
Contributor

@rickyma rickyma commented Mar 23, 2024

What changes were proposed in this pull request?

Fix the inaccurate used_direct_memory_size metric.
Also used_direct_memory_size_by_netty and used_direct_memory_size_by_grpc_netty metrics are added to provide more detailed indicators for locating and analyzing in production.

Why are the changes needed?

Fix #1598.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Tested in our env.

@codecov-commenter
Copy link

codecov-commenter commented Mar 23, 2024

Codecov Report

Attention: Patch coverage is 75.00000% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 54.99%. Comparing base (32d533d) to head (6280ac3).

Files Patch % Lines
...pache/uniffle/server/NettyDirectMemoryTracker.java 62.50% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #1599      +/-   ##
============================================
+ Coverage     54.01%   54.99%   +0.97%     
- Complexity     2863     2865       +2     
============================================
  Files           438      418      -20     
  Lines         24850    22498    -2352     
  Branches       2114     2114              
============================================
- Hits          13423    12372    -1051     
+ Misses        10586     9358    -1228     
+ Partials        841      768      -73     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@jerqi jerqi left a comment

Choose a reason for hiding this comment

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

LGTM, cc @lifeSo

@github-actions
Copy link

Test Results

 2 340 files  ±0   2 340 suites  ±0   4h 32m 0s ⏱️ -1s
   908 tests ±0     907 ✅ ±0   1 💤 ±0  0 ❌ ±0 
10 541 runs  ±0  10 527 ✅ ±0  14 💤 ±0  0 ❌ ±0 

Results for commit 6280ac3. ± Comparison against base commit 32d533d.

@lifeSo
Copy link
Collaborator

lifeSo commented Mar 25, 2024

Great!

@jerqi jerqi merged commit 3a1b4d2 into apache:master Mar 25, 2024
@lifeSo
Copy link
Collaborator

lifeSo commented Mar 25, 2024

@rickyma
I am interested in is there direct memory problem in your prd ?

@rickyma
Copy link
Contributor Author

rickyma commented Mar 25, 2024

@rickyma I am interested in is there direct memory problem in your prd ?

No. I've fixed a lot of serious issues related to off-heap memory when enabling Netty.
There is only one issue #1596 left.
And I find out that gRPC does not work very well when the concurrency is above 11200. There is a huge problem when enabling gRPC. The load of CPU will be a lot higher than using Netty. I'll mention this later in another issue. But I might not fix this, because we will use Netty instead of gRPC. gRPC should be deprecated later on.

@rickyma rickyma deleted the issue-1598 branch May 5, 2024 08:33
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.

[Bug] The used direct memory metric is inaccurate

4 participants