You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@uniffle.apache.org by "zuston (via GitHub)" <gi...@apache.org> on 2026/02/28 03:29:15 UTC
[PR] [#2716] feat(spark): Introduce option of max segments decompression to control memory usage [uniffle]
zuston opened a new pull request, #2735:
URL: https://github.com/apache/uniffle/pull/2735
### What changes were proposed in this pull request?
Introduce option of max segments decompression to control memory usage
### Why are the changes needed?
To address issue #2716, this PR introduces an option to set the maximum number of concurrent decompression segments, allowing better control over overall memory usage. Setting this value to 1 restricts decompression to a single segment at a time.
### Does this PR introduce _any_ user-facing change?
Yes
### How was this patch tested?
Unit test
--
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] [#2716] feat(spark): Introduce option of max segments decompression to control memory usage [uniffle]
Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston merged PR #2735:
URL: https://github.com/apache/uniffle/pull/2735
--
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] [#2716] feat(spark): Introduce option of max segments decompression to control memory usage [uniffle]
Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston commented on PR #2735:
URL: https://github.com/apache/uniffle/pull/2735#issuecomment-3981771175
> > updated. cc @wForget @roryqi .
> > BTW, to better tune this option, it would be helpful to add some statistics to the decompression worker, such as tracking the number of segments per batch.
>
> You can calculate the default value according to some data which you have known.
I have set this 10
--
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] [#2716] feat(spark): Introduce option of max segments decompression to control memory usage [uniffle]
Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston commented on code in PR #2735:
URL: https://github.com/apache/uniffle/pull/2735#discussion_r2867129482
##########
client/src/main/java/org/apache/uniffle/client/impl/DecompressionWorker.java:
##########
@@ -132,6 +163,7 @@ public DecompressedShuffleBlock get(int batchIndex, int segmentIndex) {
// block
if (block != null) {
nowMemoryUsed.addAndGet(-block.getUncompressLength());
+ segmentPermits.ifPresent(x -> x.release());
Review Comment:
We have ensured the block is not always null here.
--
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] [#2716] feat(spark): Introduce option of max segments decompression to control memory usage [uniffle]
Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston commented on PR #2735:
URL: https://github.com/apache/uniffle/pull/2735#issuecomment-3981714652
updated. cc @wForget @roryqi .
BTW, to better tune this option, it would be helpful to add some statistics to the decompression worker, such as tracking the number of segments per batch.
--
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] [#2716] feat(spark): Introduce option of max segments decompression to control memory usage [uniffle]
Posted by "wForget (via GitHub)" <gi...@apache.org>.
wForget commented on code in PR #2735:
URL: https://github.com/apache/uniffle/pull/2735#discussion_r2867096743
##########
common/src/main/java/org/apache/uniffle/common/config/RssClientConf.java:
##########
@@ -389,4 +389,11 @@ public class RssClientConf {
.defaultValue(-1)
.withDescription(
"Fetch seconds threshold for overlapping decompress shuffle blocks.");
+
+ public static final ConfigOption<Integer>
+ RSS_READ_OVERLAPPING_DECOMPRESSION_MAX_CONCURRENT_SEGMENTS =
+ ConfigOptions.key("rss.client.read.overlappingDecompressionMaxConcurrentSegments")
+ .intType()
+ .defaultValue(-1)
Review Comment:
Should we set the default value to 1? That would make memory usage similar to when overlapping disabled.
--
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] [#2716] feat(spark): Introduce option of max segments decompression to control memory usage [uniffle]
Posted by "roryqi (via GitHub)" <gi...@apache.org>.
roryqi commented on code in PR #2735:
URL: https://github.com/apache/uniffle/pull/2735#discussion_r2867150231
##########
client/src/main/java/org/apache/uniffle/client/impl/DecompressionWorker.java:
##########
@@ -132,6 +163,7 @@ public DecompressedShuffleBlock get(int batchIndex, int segmentIndex) {
// block
if (block != null) {
nowMemoryUsed.addAndGet(-block.getUncompressLength());
+ segmentPermits.ifPresent(x -> x.release());
Review Comment:
OK.
--
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] [#2716] feat(spark): Introduce option of max segments decompression to control memory usage [uniffle]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #2735:
URL: https://github.com/apache/uniffle/pull/2735#issuecomment-3976280694
## Test Results
3 151 files ± 0 3 151 suites ±0 6h 35m 53s ⏱️ - 1m 43s
1 246 tests + 1 1 244 ✅ + 1 1 💤 ±0 0 ❌ ±0 1 🔥 ±0
15 725 runs +15 15 709 ✅ +16 15 💤 ±0 0 ❌ ±0 1 🔥 - 1
For more details on these errors, see [this check](https://github.com/apache/uniffle/runs/65225161438).
Results for commit 52c763a4. ± Comparison against base commit 5986591c.
[test-results]:data:application/gzip;base64,H4sIAN5momkC/02PzQ7CIBAGX6Xh7KEsLFhfxpClJMS2GH5OxneXVsQeZ/bLJPtizi9zYrdBcOSXgaXi85ltiSb7sFUDQqOoqt7zvuAg1Q/vqRB9nfy7h3/urgtn/FLF2MUcY4htEst2VFEDNuxV1OPUXav20al68DlKYV19rsAQSCthpFLkCFDCaIV1cgJuSZMRkq71Y87Z+wNJaJmBFAEAAA==
--
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] [#2716] feat(spark): Introduce option of max segments decompression to control memory usage [uniffle]
Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston commented on code in PR #2735:
URL: https://github.com/apache/uniffle/pull/2735#discussion_r2867131264
##########
common/src/main/java/org/apache/uniffle/common/config/RssClientConf.java:
##########
@@ -389,4 +389,11 @@ public class RssClientConf {
.defaultValue(-1)
.withDescription(
"Fetch seconds threshold for overlapping decompress shuffle blocks.");
+
+ public static final ConfigOption<Integer>
+ RSS_READ_OVERLAPPING_DECOMPRESSION_MAX_CONCURRENT_SEGMENTS =
+ ConfigOptions.key("rss.client.read.overlappingDecompressionMaxConcurrentSegments")
+ .intType()
+ .defaultValue(-1)
Review Comment:
I think we could adopt a more aggressive value, which should result in better performance.
--
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] [#2716] feat(spark): Introduce option of max segments decompression to control memory usage [uniffle]
Posted by "roryqi (via GitHub)" <gi...@apache.org>.
roryqi commented on code in PR #2735:
URL: https://github.com/apache/uniffle/pull/2735#discussion_r2867117130
##########
client/src/main/java/org/apache/uniffle/client/impl/DecompressionWorker.java:
##########
@@ -132,6 +163,7 @@ public DecompressedShuffleBlock get(int batchIndex, int segmentIndex) {
// block
if (block != null) {
nowMemoryUsed.addAndGet(-block.getUncompressLength());
+ segmentPermits.ifPresent(x -> x.release());
Review Comment:
When block is null, we don't release. Will it cause dead lock? Should we use try finally to gurantee the release of the lock.
--
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] [#2716] feat(spark): Introduce option of max segments decompression to control memory usage [uniffle]
Posted by "roryqi (via GitHub)" <gi...@apache.org>.
roryqi commented on code in PR #2735:
URL: https://github.com/apache/uniffle/pull/2735#discussion_r2867117130
##########
client/src/main/java/org/apache/uniffle/client/impl/DecompressionWorker.java:
##########
@@ -132,6 +163,7 @@ public DecompressedShuffleBlock get(int batchIndex, int segmentIndex) {
// block
if (block != null) {
nowMemoryUsed.addAndGet(-block.getUncompressLength());
+ segmentPermits.ifPresent(x -> x.release());
Review Comment:
When block is null, we don't release. Will it cause dead lock? Should we use try finally to gurantee the release of the lock.
--
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] [#2716] feat(spark): Introduce option of max segments decompression to control memory usage [uniffle]
Posted by "roryqi (via GitHub)" <gi...@apache.org>.
roryqi commented on PR #2735:
URL: https://github.com/apache/uniffle/pull/2735#issuecomment-3981733398
> updated. cc @wForget @roryqi .
>
> BTW, to better tune this option, it would be helpful to add some statistics to the decompression worker, such as tracking the number of segments per batch.
You can calculate the default value according to some data which you have known.
--
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