You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@uniffle.apache.org by "xianjingfeng (via GitHub)" <gi...@apache.org> on 2025/12/23 07:59:24 UTC
[PR] [#2701] fix(server): release the memory of duplicate blocks [uniffle]
xianjingfeng opened a new pull request, #2702:
URL: https://github.com/apache/uniffle/pull/2702
<!--
1. Title: [#<issue>] <type>(<scope>): <subject>
Examples:
- "[#123] feat(operator): support xxx"
- "[#123] feat(server,coordinator): support xxx"
- "[#123] feat(common): support xxx"
- "[MINOR] improvement(script): fix style"
- "[MINOR] improvement(ci): add some examples and notice"
- "[#233] fix: check null before access result in xxx"
- "[#233][FOLLOWUP] improvement(dashboard): display something xxx"
- "[MINOR] refactor: fix typo in variable name"
- "[MINOR] docs: fix typo in README"
- "[#255] test: fix flaky test NameOfTheTest"
Reference: https://www.conventionalcommits.org/en/v1.0.0/
2. Contributor guidelines:
https://github.com/apache/incubator-uniffle/blob/master/CONTRIBUTING.md
3. If the PR is unfinished, please mark this PR as draft.
-->
### What changes were proposed in this pull request?
Release the memory of duplicate blocks.
### Why are the changes needed?
<!--
(Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, describe the bug.)
-->
Fix: #2701
### Does this PR introduce _any_ user-facing change?
<!--
(Please list the user-facing changes introduced by your change, including
1. Change in user-facing APIs.
2. Addition or removal of property keys.)
-->
No.
### How was this patch tested?
<!--
(Please test your changes, and provide instructions on how to test it:
1. If you add a feature or fix a bug, add a test to cover your changes.
2. If you fix a flaky test, repeat it for many times to prove it works.)
-->
UT
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
Re: [PR] [#2701] fix(server): release the memory of duplicate blocks [uniffle]
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #2702:
URL: https://github.com/apache/uniffle/pull/2702#discussion_r2642320260
##########
server/src/main/java/org/apache/uniffle/server/buffer/ShuffleBufferManager.java:
##########
@@ -290,6 +290,10 @@ public StatusCode cacheShuffleData(
}
if (!isPreAllocated) {
updateUsedMemory(size);
+ } else {
Review Comment:
Could you explain more about this case? How to trigger this code path? Could u add some comments?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
Re: [PR] [#2701] fix(server): release the memory of duplicate blocks [uniffle]
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #2702:
URL: https://github.com/apache/uniffle/pull/2702#discussion_r2642394146
##########
server/src/main/java/org/apache/uniffle/server/buffer/ShuffleBufferManager.java:
##########
@@ -290,6 +290,10 @@ public StatusCode cacheShuffleData(
}
if (!isPreAllocated) {
updateUsedMemory(size);
+ } else {
Review Comment:
Why does the duplicated block satisfied the condition? Must it be duplicated block? Why do we produce the duplicated block?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
Re: [PR] [#2701] fix(server): release the memory of duplicate blocks [uniffle]
Posted by "xianjingfeng (via GitHub)" <gi...@apache.org>.
xianjingfeng commented on code in PR #2702:
URL: https://github.com/apache/uniffle/pull/2702#discussion_r2642351178
##########
server/src/main/java/org/apache/uniffle/server/buffer/ShuffleBufferManager.java:
##########
@@ -290,6 +290,10 @@ public StatusCode cacheShuffleData(
}
if (!isPreAllocated) {
updateUsedMemory(size);
+ } else {
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
Re: [PR] [#2701] fix(server): release the memory of duplicate blocks [uniffle]
Posted by "xianjingfeng (via GitHub)" <gi...@apache.org>.
xianjingfeng commented on PR #2702:
URL: https://github.com/apache/uniffle/pull/2702#issuecomment-3688462441
Merged. Thanks for your review. @jerqi @zuston
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
Re: [PR] [#2701] fix(server): release the memory of duplicate blocks [uniffle]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #2702:
URL: https://github.com/apache/uniffle/pull/2702#issuecomment-3685766301
## Test Results
3 175 files ± 0 3 175 suites ±0 6h 55m 34s ⏱️ -14s
1 238 tests + 1 1 237 ✅ + 1 1 💤 ±0 0 ❌ ±0
15 689 runs +15 15 674 ✅ +15 15 💤 ±0 0 ❌ ±0
Results for commit 25ee499c. ± Comparison against base commit a2c2d056.
[test-results]:data:application/gzip;base64,H4sIABlXSmkC/03MOQ7DIBCF4atY1CnYBkwuE7FKKF4iDFWUuwc7mLh834z+Nwpx8hu6D4xIuA1oKzFftytJ57guVShXjFeq97x/EMrGcz62Yu3P5N+e8bVbh6DjVAF38CmtqUkqy1EFMao2exWE5N1aFU64VI99jdp1nmOuA1HwnitlhXVESGOcxoYR8KOpdcDgRKCacY0+XxaG9sIUAQAA
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
Re: [PR] [#2701] fix(server): release the memory of duplicate blocks [uniffle]
Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston commented on code in PR #2702:
URL: https://github.com/apache/uniffle/pull/2702#discussion_r2643022083
##########
server/src/main/java/org/apache/uniffle/server/buffer/ShuffleBufferManager.java:
##########
@@ -290,6 +290,10 @@ public StatusCode cacheShuffleData(
}
if (!isPreAllocated) {
updateUsedMemory(size);
+ } else {
Review Comment:
This looks good in this case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
Re: [PR] [#2701] fix(server): release the memory of duplicate blocks [uniffle]
Posted by "xianjingfeng (via GitHub)" <gi...@apache.org>.
xianjingfeng commented on code in PR #2702:
URL: https://github.com/apache/uniffle/pull/2702#discussion_r2642428179
##########
server/src/main/java/org/apache/uniffle/server/buffer/ShuffleBufferManager.java:
##########
@@ -290,6 +290,10 @@ public StatusCode cacheShuffleData(
}
if (!isPreAllocated) {
updateUsedMemory(size);
+ } else {
Review Comment:
@jerqi See the code below, if `addBlock(block) == false`, the duplicate block will be released. However, the `encodedLength` of the buffer will not include the `encodedLength` of the duplicate block. And the `encodedLength` of the buffer is the memory size to be released where the buffer flush. And duplicated block is a known problem. #926
https://github.com/apache/uniffle/blob/a2c2d056ce8fd49af025eaa48785b94d21515557/server/src/main/java/org/apache/uniffle/server/buffer/ShuffleBufferWithLinkedList.java#L61-L75
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
Re: [PR] [#2701] fix(server): release the memory of duplicate blocks [uniffle]
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #2702:
URL: https://github.com/apache/uniffle/pull/2702#discussion_r2642440648
##########
server/src/main/java/org/apache/uniffle/server/buffer/ShuffleBufferManager.java:
##########
@@ -290,6 +290,10 @@ public StatusCode cacheShuffleData(
}
if (!isPreAllocated) {
updateUsedMemory(size);
+ } else {
Review Comment:
let @zuston take a look at this issue. He also has some thoughts about this issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
Re: [PR] [#2701] fix(server): release the memory of duplicate blocks [uniffle]
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #2702:
URL: https://github.com/apache/uniffle/pull/2702#discussion_r2642395219
##########
server/src/main/java/org/apache/uniffle/server/buffer/ShuffleBufferManager.java:
##########
@@ -290,6 +290,10 @@ public StatusCode cacheShuffleData(
}
if (!isPreAllocated) {
updateUsedMemory(size);
+ } else {
Review Comment:
cc @zuston
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
Re: [PR] [#2701] fix(server): release the memory of duplicate blocks [uniffle]
Posted by "xianjingfeng (via GitHub)" <gi...@apache.org>.
xianjingfeng merged PR #2702:
URL: https://github.com/apache/uniffle/pull/2702
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org