You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@uniffle.apache.org by GitBox <gi...@apache.org> on 2022/08/22 09:16:13 UTC
[GitHub] [incubator-uniffle] leixm opened a new pull request, #176: [Improvement] ShuffleBufferManager supports triggering flush according to the size of single ShuffleBuffer
leixm opened a new pull request, #176:
URL: https://github.com/apache/incubator-uniffle/pull/176
### What changes were proposed in this pull request?
for issue https://github.com/apache/incubator-uniffle/issues/173, Add a new flush strategy for ShuffleBufferManager, flush if the size of a single buffer reaches `rss.server.buffer.flush.threshold`, which can make more full use of disk io without waiting for the buffer to reach `(rss.server.buffer.capacity * rss.server.memory.shuffle.highWaterMark.percentage)`
### Why are the changes needed?
### Does this PR introduce any user-facing change?
### How was this patch tested?
Already added
--
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
[GitHub] [incubator-uniffle] codecov-commenter commented on pull request #176: [Improvement] ShuffleBufferManager supports triggering flush according to the size of single ShuffleBuffer
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #176:
URL: https://github.com/apache/incubator-uniffle/pull/176#issuecomment-1222224483
# [Codecov](https://codecov.io/gh/apache/incubator-uniffle/pull/176?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#176](https://codecov.io/gh/apache/incubator-uniffle/pull/176?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ab96bd2) into [master](https://codecov.io/gh/apache/incubator-uniffle/commit/74cd40b9fc5f41b8b04fbaac9bab31985e7b6d15?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (74cd40b) will **decrease** coverage by `1.27%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head ab96bd2 differs from pull request most recent head ea9c2d0. Consider uploading reports for the commit ea9c2d0 to get more accurate results
```diff
@@ Coverage Diff @@
## master #176 +/- ##
============================================
- Coverage 58.29% 57.01% -1.28%
+ Complexity 1262 1186 -76
============================================
Files 158 149 -9
Lines 8397 7919 -478
Branches 779 749 -30
============================================
- Hits 4895 4515 -380
+ Misses 3251 3162 -89
+ Partials 251 242 -9
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-uniffle/pull/176?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...a/org/apache/uniffle/server/ShuffleServerConf.java](https://codecov.io/gh/apache/incubator-uniffle/pull/176/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS91bmlmZmxlL3NlcnZlci9TaHVmZmxlU2VydmVyQ29uZi5qYXZh) | `99.18% <100.00%> (+0.03%)` | :arrow_up: |
| [...he/uniffle/server/buffer/ShuffleBufferManager.java](https://codecov.io/gh/apache/incubator-uniffle/pull/176/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS91bmlmZmxlL3NlcnZlci9idWZmZXIvU2h1ZmZsZUJ1ZmZlck1hbmFnZXIuamF2YQ==) | `82.77% <100.00%> (+0.52%)` | :arrow_up: |
| [...storage/handler/impl/DataSkippableReadHandler.java](https://codecov.io/gh/apache/incubator-uniffle/pull/176/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3RvcmFnZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvdW5pZmZsZS9zdG9yYWdlL2hhbmRsZXIvaW1wbC9EYXRhU2tpcHBhYmxlUmVhZEhhbmRsZXIuamF2YQ==) | `81.25% <0.00%> (-3.13%)` | :arrow_down: |
| [...org/apache/uniffle/server/ShuffleFlushManager.java](https://codecov.io/gh/apache/incubator-uniffle/pull/176/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS91bmlmZmxlL3NlcnZlci9TaHVmZmxlRmx1c2hNYW5hZ2VyLmphdmE=) | `77.65% <0.00%> (-1.68%)` | :arrow_down: |
| [.../java/org/apache/spark/shuffle/RssSparkConfig.java](https://codecov.io/gh/apache/incubator-uniffle/pull/176/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50LXNwYXJrL2NvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc3Bhcmsvc2h1ZmZsZS9Sc3NTcGFya0NvbmZpZy5qYXZh) | | |
| [...e/spark/shuffle/reader/RssShuffleDataIterator.java](https://codecov.io/gh/apache/incubator-uniffle/pull/176/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50LXNwYXJrL2NvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc3Bhcmsvc2h1ZmZsZS9yZWFkZXIvUnNzU2h1ZmZsZURhdGFJdGVyYXRvci5qYXZh) | | |
| [...org/apache/spark/shuffle/writer/AddBlockEvent.java](https://codecov.io/gh/apache/incubator-uniffle/pull/176/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50LXNwYXJrL2NvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc3Bhcmsvc2h1ZmZsZS93cml0ZXIvQWRkQmxvY2tFdmVudC5qYXZh) | | |
| [...pache/spark/shuffle/writer/WriteBufferManager.java](https://codecov.io/gh/apache/incubator-uniffle/pull/176/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50LXNwYXJrL2NvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc3Bhcmsvc2h1ZmZsZS93cml0ZXIvV3JpdGVCdWZmZXJNYW5hZ2VyLmphdmE=) | | |
| [...che/spark/shuffle/writer/BufferManagerOptions.java](https://codecov.io/gh/apache/incubator-uniffle/pull/176/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50LXNwYXJrL2NvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc3Bhcmsvc2h1ZmZsZS93cml0ZXIvQnVmZmVyTWFuYWdlck9wdGlvbnMuamF2YQ==) | | |
| [.../org/apache/spark/shuffle/writer/WriterBuffer.java](https://codecov.io/gh/apache/incubator-uniffle/pull/176/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50LXNwYXJrL2NvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc3Bhcmsvc2h1ZmZsZS93cml0ZXIvV3JpdGVyQnVmZmVyLmphdmE=) | | |
| ... and [3 more](https://codecov.io/gh/apache/incubator-uniffle/pull/176/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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
[GitHub] [incubator-uniffle] leixm commented on pull request #176: [Improvement] ShuffleBufferManager supports triggering flush according to the size of single ShuffleBuffer
Posted by GitBox <gi...@apache.org>.
leixm commented on PR #176:
URL: https://github.com/apache/incubator-uniffle/pull/176#issuecomment-1222251799
Thanks for your review @jerqi
--
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
[GitHub] [incubator-uniffle] jerqi merged pull request #176: [Improvement] ShuffleBufferManager supports triggering flush according to the size of single ShuffleBuffer
Posted by GitBox <gi...@apache.org>.
jerqi merged PR #176:
URL: https://github.com/apache/incubator-uniffle/pull/176
--
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
[GitHub] [incubator-uniffle] leixm commented on a diff in pull request #176: [Improvement] ShuffleBufferManager supports triggering flush according to the size of single ShuffleBuffer
Posted by GitBox <gi...@apache.org>.
leixm commented on code in PR #176:
URL: https://github.com/apache/incubator-uniffle/pull/176#discussion_r951308969
##########
server/src/main/java/org/apache/uniffle/server/ShuffleServerConf.java:
##########
@@ -284,6 +284,18 @@ public class ShuffleServerConf extends RssBaseConf {
.defaultValue(0L)
.withDescription("For localstorage, it will exit when the failed initialized local storage exceed the number");
+ public static final ConfigOption<Boolean> BUFFER_FLUSH_ENABLED = ConfigOptions
+ .key("rss.server.buffer.flush.enabled")
+ .booleanType()
+ .defaultValue(false)
+ .withDescription("Whether buffer flush when size exceeded rss.server.buffer.flush.threshold");
+
+ public static final ConfigOption<Long> BUFFER_FLUSH_THRESHOLD = ConfigOptions
+ .key("rss.server.buffer.flush.threshold")
+ .longType()
+ .defaultValue(32 * 1024 * 1024L)
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
[GitHub] [incubator-uniffle] jerqi commented on pull request #176: [Improvement] ShuffleBufferManager supports triggering flush according to the size of single ShuffleBuffer
Posted by GitBox <gi...@apache.org>.
jerqi commented on PR #176:
URL: https://github.com/apache/incubator-uniffle/pull/176#issuecomment-1222309788
> I think we should change `this.bufferFlushEnabled && buffer.getSize() >= this.bufferFlushThreshold` to `this.bufferFlushEnabled && buffer.getSize() > this.bufferFlushThreshold` , which should be consistent with `event.getSize() > flushColdStorageThresholdSize` @jerqi
It's ok and you can raise a follow-up pr. You'd better 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
[GitHub] [incubator-uniffle] leixm closed pull request #176: [Improvement] ShuffleBufferManager supports triggering flush according to the size of single ShuffleBuffer
Posted by GitBox <gi...@apache.org>.
leixm closed pull request #176: [Improvement] ShuffleBufferManager supports triggering flush according to the size of single ShuffleBuffer
URL: https://github.com/apache/incubator-uniffle/pull/176
--
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
[GitHub] [incubator-uniffle] jerqi commented on a diff in pull request #176: [Improvement] ShuffleBufferManager supports triggering flush according to the size of single ShuffleBuffer
Posted by GitBox <gi...@apache.org>.
jerqi commented on code in PR #176:
URL: https://github.com/apache/incubator-uniffle/pull/176#discussion_r951234031
##########
server/src/main/java/org/apache/uniffle/server/ShuffleServerConf.java:
##########
@@ -284,6 +284,18 @@ public class ShuffleServerConf extends RssBaseConf {
.defaultValue(0L)
.withDescription("For localstorage, it will exit when the failed initialized local storage exceed the number");
+ public static final ConfigOption<Boolean> BUFFER_FLUSH_ENABLED = ConfigOptions
+ .key("rss.server.buffer.flush.enabled")
Review Comment:
Could we change the name from `rss.server.buffer.flush.enabled` to `rss.server.single.buffer.flush.enabled`?
##########
server/src/main/java/org/apache/uniffle/server/ShuffleServerConf.java:
##########
@@ -284,6 +284,18 @@ public class ShuffleServerConf extends RssBaseConf {
.defaultValue(0L)
.withDescription("For localstorage, it will exit when the failed initialized local storage exceed the number");
+ public static final ConfigOption<Boolean> BUFFER_FLUSH_ENABLED = ConfigOptions
+ .key("rss.server.buffer.flush.enabled")
+ .booleanType()
+ .defaultValue(false)
+ .withDescription("Whether buffer flush when size exceeded rss.server.buffer.flush.threshold");
+
+ public static final ConfigOption<Long> BUFFER_FLUSH_THRESHOLD = ConfigOptions
Review Comment:
Could we change the name from `rss.server.buffer.flush.threshold` to `rss.server.single.buffer.flush.threshold`?
##########
server/src/main/java/org/apache/uniffle/server/ShuffleServerConf.java:
##########
@@ -284,6 +284,18 @@ public class ShuffleServerConf extends RssBaseConf {
.defaultValue(0L)
.withDescription("For localstorage, it will exit when the failed initialized local storage exceed the number");
+ public static final ConfigOption<Boolean> BUFFER_FLUSH_ENABLED = ConfigOptions
+ .key("rss.server.buffer.flush.enabled")
+ .booleanType()
+ .defaultValue(false)
+ .withDescription("Whether buffer flush when size exceeded rss.server.buffer.flush.threshold");
+
+ public static final ConfigOption<Long> BUFFER_FLUSH_THRESHOLD = ConfigOptions
+ .key("rss.server.buffer.flush.threshold")
+ .longType()
+ .defaultValue(32 * 1024 * 1024L)
Review Comment:
Could the default value be consistent with `rss.server.flush.cold.storage.threshold.size`?
--
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
[GitHub] [incubator-uniffle] leixm commented on pull request #176: [Improvement] ShuffleBufferManager supports triggering flush according to the size of single ShuffleBuffer
Posted by GitBox <gi...@apache.org>.
leixm commented on PR #176:
URL: https://github.com/apache/incubator-uniffle/pull/176#issuecomment-1222086420
@jerqi Can you approve running workflows for this pr plz?
--
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
[GitHub] [incubator-uniffle] jerqi commented on a diff in pull request #176: [Improvement] ShuffleBufferManager supports triggering flush according to the size of single ShuffleBuffer
Posted by GitBox <gi...@apache.org>.
jerqi commented on code in PR #176:
URL: https://github.com/apache/incubator-uniffle/pull/176#discussion_r951315081
##########
README.md:
##########
@@ -245,6 +245,8 @@ For more details of advanced configuration, please see [Uniffle Coordinator Guid
|rss.storage.type|-|Supports MEMORY_LOCALFILE, MEMORY_HDFS, MEMORY_LOCALFILE_HDFS|
|rss.server.flush.cold.storage.threshold.size|64M| The threshold of data size for LOACALFILE and HDFS if MEMORY_LOCALFILE_HDFS is used|
|rss.server.tags|-|The comma-separated list of tags to indicate the shuffle server's attributes. It will be used as the assignment basis for the coordinator|
+|rss.server.single.buffer.flush.enabled|false|Whether single buffer flush when size exceeded rss.server.single.buffer.flush.enabled|
Review Comment:
Should we change
from `exceeded rss.server.single.buffer.flush.enabled`
to `rss.server.single.buffer.flush.threshold`?
--
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
[GitHub] [incubator-uniffle] jerqi commented on pull request #176: [Improvement] ShuffleBufferManager supports triggering flush according to the size of single ShuffleBuffer
Posted by GitBox <gi...@apache.org>.
jerqi commented on PR #176:
URL: https://github.com/apache/incubator-uniffle/pull/176#issuecomment-1222124087
> ### What changes were proposed in this pull request?
> for issue #173, Add a new flush strategy for ShuffleBufferManager, flush if the size of a single buffer reaches `rss.server.buffer.flush.threshold`, which can make more full use of disk io without waiting for the buffer to reach `(rss.server.buffer.capacity * rss.server.memory.shuffle.highWaterMark.percentage)`
>
> ### Why are the changes needed?
> ### Does this PR introduce any user-facing change?
> ### How was this patch tested?
> Already added
This is a user-facing change, Could you add the configuration to the document? https://github.com/apache/incubator-uniffle/blob/master/README.md
--
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
[GitHub] [incubator-uniffle] leixm commented on a diff in pull request #176: [Improvement] ShuffleBufferManager supports triggering flush according to the size of single ShuffleBuffer
Posted by GitBox <gi...@apache.org>.
leixm commented on code in PR #176:
URL: https://github.com/apache/incubator-uniffle/pull/176#discussion_r951308676
##########
server/src/main/java/org/apache/uniffle/server/ShuffleServerConf.java:
##########
@@ -284,6 +284,18 @@ public class ShuffleServerConf extends RssBaseConf {
.defaultValue(0L)
.withDescription("For localstorage, it will exit when the failed initialized local storage exceed the number");
+ public static final ConfigOption<Boolean> BUFFER_FLUSH_ENABLED = ConfigOptions
+ .key("rss.server.buffer.flush.enabled")
+ .booleanType()
+ .defaultValue(false)
+ .withDescription("Whether buffer flush when size exceeded rss.server.buffer.flush.threshold");
+
+ public static final ConfigOption<Long> BUFFER_FLUSH_THRESHOLD = ConfigOptions
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
[GitHub] [incubator-uniffle] leixm commented on a diff in pull request #176: [Improvement] ShuffleBufferManager supports triggering flush according to the size of single ShuffleBuffer
Posted by GitBox <gi...@apache.org>.
leixm commented on code in PR #176:
URL: https://github.com/apache/incubator-uniffle/pull/176#discussion_r951308367
##########
server/src/main/java/org/apache/uniffle/server/ShuffleServerConf.java:
##########
@@ -284,6 +284,18 @@ public class ShuffleServerConf extends RssBaseConf {
.defaultValue(0L)
.withDescription("For localstorage, it will exit when the failed initialized local storage exceed the number");
+ public static final ConfigOption<Boolean> BUFFER_FLUSH_ENABLED = ConfigOptions
+ .key("rss.server.buffer.flush.enabled")
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
[GitHub] [incubator-uniffle] leixm commented on a diff in pull request #176: [Improvement] ShuffleBufferManager supports triggering flush according to the size of single ShuffleBuffer
Posted by GitBox <gi...@apache.org>.
leixm commented on code in PR #176:
URL: https://github.com/apache/incubator-uniffle/pull/176#discussion_r951320702
##########
README.md:
##########
@@ -245,6 +245,8 @@ For more details of advanced configuration, please see [Uniffle Coordinator Guid
|rss.storage.type|-|Supports MEMORY_LOCALFILE, MEMORY_HDFS, MEMORY_LOCALFILE_HDFS|
|rss.server.flush.cold.storage.threshold.size|64M| The threshold of data size for LOACALFILE and HDFS if MEMORY_LOCALFILE_HDFS is used|
|rss.server.tags|-|The comma-separated list of tags to indicate the shuffle server's attributes. It will be used as the assignment basis for the coordinator|
+|rss.server.single.buffer.flush.enabled|false|Whether single buffer flush when size exceeded rss.server.single.buffer.flush.enabled|
Review Comment:
my mistake
--
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
[GitHub] [incubator-uniffle] leixm commented on pull request #176: [Improvement] ShuffleBufferManager supports triggering flush according to the size of single ShuffleBuffer
Posted by GitBox <gi...@apache.org>.
leixm commented on PR #176:
URL: https://github.com/apache/incubator-uniffle/pull/176#issuecomment-1222305629
I think we should change `this.bufferFlushEnabled && buffer.getSize() >= this.bufferFlushThreshold` to `this.bufferFlushEnabled && buffer.getSize() > this.bufferFlushThreshold` , which should be consistent with `event.getSize() > flushColdStorageThresholdSize` @jerqi
--
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