You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@uniffle.apache.org by "advancedxy (via GitHub)" <gi...@apache.org> on 2023/02/06 12:45:42 UTC

[GitHub] [incubator-uniffle] advancedxy opened a new pull request, #555: [#554] infer rss base storage conf from env

advancedxy opened a new pull request, #555:
URL: https://github.com/apache/incubator-uniffle/pull/555

   ### What changes were proposed in this pull request?
   new method `getConfiguredLocalDirs` is added to retrieve rss base storage path from env first
   
   
   
   ### Why are the changes needed?
   Fixes #554 
   
   
   ### Does this PR introduce _any_ user-facing change?
   RSS admin may specify rss base path for shuffle server when deploying
   
   ### How was this patch tested?
   Added 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

[GitHub] [incubator-uniffle] kaijchen commented on a diff in pull request #555: [#554] infer rss base storage conf from env

Posted by "kaijchen (via GitHub)" <gi...@apache.org>.
kaijchen commented on code in PR #555:
URL: https://github.com/apache/incubator-uniffle/pull/555#discussion_r1098235116


##########
common/src/main/java/org/apache/uniffle/common/util/RssUtils.java:
##########
@@ -292,4 +296,12 @@ public static Roaring64NavigableMap generateTaskIdBitMap(Roaring64NavigableMap b
     }
     return taskIdBitmap;
   }
+
+  public static List<String> getConfiguredLocalDirs(RssConf conf) {

Review Comment:
   Sounds good.



-- 
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] advancedxy commented on pull request #555: [#554] feat: infer rss base storage conf from env

Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on PR #555:
URL: https://github.com/apache/incubator-uniffle/pull/555#issuecomment-1422049195

   gently ping @kaijchen 


-- 
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] advancedxy commented on a diff in pull request #555: [#554] infer rss base storage conf from env

Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on code in PR #555:
URL: https://github.com/apache/incubator-uniffle/pull/555#discussion_r1098491791


##########
common/src/main/java/org/apache/uniffle/common/util/RssUtils.java:
##########
@@ -292,4 +296,12 @@ public static Roaring64NavigableMap generateTaskIdBitMap(Roaring64NavigableMap b
     }
     return taskIdBitmap;
   }
+
+  public static List<String> getConfiguredLocalDirs(RssConf conf) {

Review Comment:
   > For example: marking the config, whose value could be from env. Like that
   > 
   > ```
   > public static final ConfigOption<Long> SERVER_BUFFER_CAPACITY = ConfigOptions
   >       .key("rss.server.buffer.capacity")
   >       .longType()
   >       .noDefaultValue()
   >       .allowFromEnv()
   >       .withDescription("Max memory of buffer manager for shuffle server");
   > ```
   > 
   > Once one config option is allowed from env, it will be initialized from env when creating the `shuffleServerConf`. WDYT?
   
   Thought about it. In general it seems good to me. But I wonder how many more configurations would be loaded from env. I referenced Spark, which doesn't have this `loadFromEnv` option..
   
   How about we keep this a while, if there are two/three more configurations that should be read from env, then let's refactor and add the feature for ConfigBuilder?



-- 
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] advancedxy commented on a diff in pull request #555: [#554] infer rss base storage conf from env

Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on code in PR #555:
URL: https://github.com/apache/incubator-uniffle/pull/555#discussion_r1098538995


##########
common/src/test/java/org/apache/uniffle/common/util/RssUtilsTest.java:
##########
@@ -177,6 +181,24 @@ public void testGenerateServerToPartitions() {
     assertEquals(serverToPartitions.get(server4), Sets.newHashSet(2, 4));
   }
 
+  @Test
+  public void testGetConfiguredLocalDirs() {
+    RssConf conf = new RssConf();
+    withEnv(RssUtils.RSS_LOCAL_DIR_KEY, "/path/a", () -> {

Review Comment:
   Thanks. Addressed.



-- 
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] kaijchen commented on a diff in pull request #555: [#554] feat: infer rss base storage conf from env

Posted by "kaijchen (via GitHub)" <gi...@apache.org>.
kaijchen commented on code in PR #555:
URL: https://github.com/apache/incubator-uniffle/pull/555#discussion_r1098579142


##########
common/src/test/java/org/apache/uniffle/common/util/RssUtilsTest.java:
##########
@@ -177,6 +181,24 @@ public void testGenerateServerToPartitions() {
     assertEquals(serverToPartitions.get(server4), Sets.newHashSet(2, 4));
   }
 
+  @Test
+  public void testGetConfiguredLocalDirs() {
+    RssConf conf = new RssConf();
+    withEnv(RssUtils.RSS_LOCAL_DIR_KEY, "/path/a", () -> {

Review Comment:
   Thanks for abstracting all use case of `setEnv()`. However now you are mixing two things in one PR.
   I would suggest split them for easier review (and revert, just in 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

[GitHub] [incubator-uniffle] codecov-commenter commented on pull request #555: [#554] infer rss base storage conf from env

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #555:
URL: https://github.com/apache/incubator-uniffle/pull/555#issuecomment-1419052057

   # [Codecov](https://codecov.io/gh/apache/incubator-uniffle/pull/555?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 [#555](https://codecov.io/gh/apache/incubator-uniffle/pull/555?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (86fb351) into [master](https://codecov.io/gh/apache/incubator-uniffle/commit/ebbe2db238cc6a611aea479fab988e23c636c952?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ebbe2db) will **increase** coverage by `4.26%`.
   > The diff coverage is `100.00%`.
   
   ```diff
   @@             Coverage Diff              @@
   ##             master     #555      +/-   ##
   ============================================
   + Coverage     60.21%   64.47%   +4.26%     
   + Complexity     1784     1666     -118     
   ============================================
     Files           205      186      -19     
     Lines         11557     9129    -2428     
     Branches       1042      928     -114     
   ============================================
   - Hits           6959     5886    -1073     
   + Misses         4190     2925    -1265     
   + Partials        408      318      -90     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-uniffle/pull/555?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...java/org/apache/uniffle/common/config/RssConf.java](https://codecov.io/gh/apache/incubator-uniffle/pull/555?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS91bmlmZmxlL2NvbW1vbi9jb25maWcvUnNzQ29uZi5qYXZh) | `32.41% <100.00%> (+0.46%)` | :arrow_up: |
   | [.../java/org/apache/uniffle/common/util/RssUtils.java](https://codecov.io/gh/apache/incubator-uniffle/pull/555?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS91bmlmZmxlL2NvbW1vbi91dGlsL1Jzc1V0aWxzLmphdmE=) | `59.31% <100.00%> (+0.85%)` | :arrow_up: |
   | [...org/apache/uniffle/server/LocalStorageChecker.java](https://codecov.io/gh/apache/incubator-uniffle/pull/555?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS91bmlmZmxlL3NlcnZlci9Mb2NhbFN0b3JhZ2VDaGVja2VyLmphdmE=) | `70.19% <100.00%> (ø)` | |
   | [...org/apache/uniffle/server/ShuffleFlushManager.java](https://codecov.io/gh/apache/incubator-uniffle/pull/555?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=) | `84.37% <100.00%> (ø)` | |
   | [...he/uniffle/server/storage/LocalStorageManager.java](https://codecov.io/gh/apache/incubator-uniffle/pull/555?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS91bmlmZmxlL3NlcnZlci9zdG9yYWdlL0xvY2FsU3RvcmFnZU1hbmFnZXIuamF2YQ==) | `85.87% <100.00%> (ø)` | |
   | [...he/uniffle/client/impl/ShuffleWriteClientImpl.java](https://codecov.io/gh/apache/incubator-uniffle/pull/555?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS91bmlmZmxlL2NsaWVudC9pbXBsL1NodWZmbGVXcml0ZUNsaWVudEltcGwuamF2YQ==) | `34.44% <0.00%> (-0.51%)` | :arrow_down: |
   | [...a/org/apache/uniffle/common/ShuffleServerInfo.java](https://codecov.io/gh/apache/incubator-uniffle/pull/555?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS91bmlmZmxlL2NvbW1vbi9TaHVmZmxlU2VydmVySW5mby5qYXZh) | `75.00% <0.00%> (ø)` | |
   | [.../org/apache/uniffle/server/ShuffleTaskManager.java](https://codecov.io/gh/apache/incubator-uniffle/pull/555?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS91bmlmZmxlL3NlcnZlci9TaHVmZmxlVGFza01hbmFnZXIuamF2YQ==) | `76.76% <0.00%> (ø)` | |
   | [...pache/uniffle/server/ShuffleServerGrpcMetrics.java](https://codecov.io/gh/apache/incubator-uniffle/pull/555?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS91bmlmZmxlL3NlcnZlci9TaHVmZmxlU2VydmVyR3JwY01ldHJpY3MuamF2YQ==) | `100.00% <0.00%> (ø)` | |
   | ... and [31 more](https://codecov.io/gh/apache/incubator-uniffle/pull/555?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] advancedxy merged pull request #555: [#554] feat: infer rss base storage conf from env

Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy merged PR #555:
URL: https://github.com/apache/incubator-uniffle/pull/555


-- 
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] zuston commented on a diff in pull request #555: [#554] infer rss base storage conf from env

Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston commented on code in PR #555:
URL: https://github.com/apache/incubator-uniffle/pull/555#discussion_r1098230745


##########
common/src/main/java/org/apache/uniffle/common/util/RssUtils.java:
##########
@@ -292,4 +296,12 @@ public static Roaring64NavigableMap generateTaskIdBitMap(Roaring64NavigableMap b
     }
     return taskIdBitmap;
   }
+
+  public static List<String> getConfiguredLocalDirs(RssConf conf) {

Review Comment:
   Could we have a general way to support conf value from env instead of supporting this one by one? 



##########
common/src/main/java/org/apache/uniffle/common/util/RssUtils.java:
##########
@@ -292,4 +296,12 @@ public static Roaring64NavigableMap generateTaskIdBitMap(Roaring64NavigableMap b
     }
     return taskIdBitmap;
   }
+
+  public static List<String> getConfiguredLocalDirs(RssConf conf) {

Review Comment:
   For example: marking the config, whose value could be from env. Like that
   
   ```
   public static final ConfigOption<Long> SERVER_BUFFER_CAPACITY = ConfigOptions
         .key("rss.server.buffer.capacity")
         .longType()
         .noDefaultValue()
         .allowFromEnv()
         .withDescription("Max memory of buffer manager for shuffle server");
   ```
   
   Once one config option is allowed from env, it will be initialized from env when creating the `shuffleServerConf`. WDYT? 



-- 
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] kaijchen commented on a diff in pull request #555: [#554] infer rss base storage conf from env

Posted by "kaijchen (via GitHub)" <gi...@apache.org>.
kaijchen commented on code in PR #555:
URL: https://github.com/apache/incubator-uniffle/pull/555#discussion_r1098135723


##########
common/src/test/java/org/apache/uniffle/common/util/RssUtilsTest.java:
##########
@@ -177,6 +181,24 @@ public void testGenerateServerToPartitions() {
     assertEquals(serverToPartitions.get(server4), Sets.newHashSet(2, 4));
   }
 
+  @Test
+  public void testGetConfiguredLocalDirs() {
+    RssConf conf = new RssConf();
+    withEnv(RssUtils.RSS_LOCAL_DIR_KEY, "/path/a", () -> {

Review Comment:
   There is a better way to test environment variables:
   
   https://github.com/apache/incubator-uniffle/blob/6ddf8a7f0ae7069b869e5734c40226ac2c81494d/server/src/test/java/org/apache/uniffle/server/ShuffleServerConfTest.java#L33-L38
   
   https://github.com/apache/incubator-uniffle/blob/6ddf8a7f0ae7069b869e5734c40226ac2c81494d/server/src/test/java/org/apache/uniffle/server/ShuffleServerConfTest.java#L49-L58



-- 
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] advancedxy commented on a diff in pull request #555: [#554] feat: infer rss base storage conf from env

Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on code in PR #555:
URL: https://github.com/apache/incubator-uniffle/pull/555#discussion_r1098635263


##########
common/src/test/java/org/apache/uniffle/common/util/RssUtilsTest.java:
##########
@@ -177,6 +181,24 @@ public void testGenerateServerToPartitions() {
     assertEquals(serverToPartitions.get(server4), Sets.newHashSet(2, 4));
   }
 
+  @Test
+  public void testGetConfiguredLocalDirs() {
+    RssConf conf = new RssConf();
+    withEnv(RssUtils.RSS_LOCAL_DIR_KEY, "/path/a", () -> {

Review Comment:
   I would argue that these 'two things' are tightly related and it's not that much to review. 
   It would be better to split into two prs and I believe one should be totally fine.



-- 
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] advancedxy commented on a diff in pull request #555: [#554] feat: infer rss base storage conf from env

Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on code in PR #555:
URL: https://github.com/apache/incubator-uniffle/pull/555#discussion_r1098679317


##########
common/src/test/java/org/apache/uniffle/common/util/RssUtilsTest.java:
##########
@@ -177,6 +181,24 @@ public void testGenerateServerToPartitions() {
     assertEquals(serverToPartitions.get(server4), Sets.newHashSet(2, 4));
   }
 
+  @Test
+  public void testGetConfiguredLocalDirs() {
+    RssConf conf = new RssConf();
+    withEnv(RssUtils.RSS_LOCAL_DIR_KEY, "/path/a", () -> {

Review Comment:
   There are other compiling issues related. It's more change to use `setEnv` correctly, I would create a separated PR then. 



-- 
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] kaijchen commented on a diff in pull request #555: [#554] feat: infer rss base storage conf from env

Posted by "kaijchen (via GitHub)" <gi...@apache.org>.
kaijchen commented on code in PR #555:
URL: https://github.com/apache/incubator-uniffle/pull/555#discussion_r1098677566


##########
common/src/test/java/org/apache/uniffle/common/util/RssUtilsTest.java:
##########
@@ -177,6 +181,24 @@ public void testGenerateServerToPartitions() {
     assertEquals(serverToPartitions.get(server4), Sets.newHashSet(2, 4));
   }
 
+  @Test
+  public void testGetConfiguredLocalDirs() {
+    RssConf conf = new RssConf();
+    withEnv(RssUtils.RSS_LOCAL_DIR_KEY, "/path/a", () -> {

Review Comment:
   OK, no big deal.



-- 
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] zuston commented on a diff in pull request #555: [#554] feat: infer rss base storage conf from env

Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston commented on code in PR #555:
URL: https://github.com/apache/incubator-uniffle/pull/555#discussion_r1098562121


##########
common/src/main/java/org/apache/uniffle/common/util/RssUtils.java:
##########
@@ -292,4 +296,12 @@ public static Roaring64NavigableMap generateTaskIdBitMap(Roaring64NavigableMap b
     }
     return taskIdBitmap;
   }
+
+  public static List<String> getConfiguredLocalDirs(RssConf conf) {

Review Comment:
   It's 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