You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@uniffle.apache.org by "maobaolong (via GitHub)" <gi...@apache.org> on 2024/10/18 04:29:45 UTC
[PR] [#XXXX] Support reg app conf to server and avoid update committed/cached blockIds bitmap [incubator-uniffle]
maobaolong opened a new pull request, #2196:
URL: https://github.com/apache/incubator-uniffle/pull/2196
<!--
1. Title: [#<issue>] <type>(<scope>): <subject>
Examples:
- "[#123] feat(operator): support xxx"
- "[#233] fix: check null before access result in 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?
Support reg app conf to server.
### Why are the changes needed?
Avoid update committed/cached blockIds bitmap while storage type is with memory.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Locally
--
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] [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap [incubator-uniffle]
Posted by "maobaolong (via GitHub)" <gi...@apache.org>.
maobaolong commented on PR #2196:
URL: https://github.com/apache/incubator-uniffle/pull/2196#issuecomment-2429121873
Encountered famous flaky test `QuorumTest.case8`
```
org.opentest4j.AssertionFailedError: expected: <true> but was: <false>
at org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55)
at org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:40)
at org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:35)
at org.junit.jupiter.api.Assertions.assertTrue(Assertions.java:179)
at org.apache.uniffle.test.QuorumTest.case8(QuorumTest.java:829)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
```
--
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] [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap [incubator-uniffle]
Posted by "maobaolong (via GitHub)" <gi...@apache.org>.
maobaolong closed pull request #2196: [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap
URL: https://github.com/apache/incubator-uniffle/pull/2196
--
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] [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap [incubator-uniffle]
Posted by "maobaolong (via GitHub)" <gi...@apache.org>.
maobaolong closed pull request #2196: [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap
URL: https://github.com/apache/incubator-uniffle/pull/2196
--
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] [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap [incubator-uniffle]
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on PR #2196:
URL: https://github.com/apache/incubator-uniffle/pull/2196#issuecomment-2440496094
We would better add check in the commit rpc implement for server side if client uses commit rpc.
--
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] [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap [incubator-uniffle]
Posted by "maobaolong (via GitHub)" <gi...@apache.org>.
maobaolong commented on code in PR #2196:
URL: https://github.com/apache/incubator-uniffle/pull/2196#discussion_r1816919574
##########
server/src/main/java/org/apache/uniffle/server/ShuffleTaskInfo.java:
##########
@@ -307,4 +318,25 @@ public String toString() {
+ shuffleDetailInfos
+ '}';
}
+
+ public void setProperties(Map<String, String> properties, ShuffleServerConf serverConf) {
Review Comment:
Since I need to check whether this server enabled bitmap optimized feature by SERVER_WITH_MEMORY_STORAGE_TYPE_OPTIMIZE_ENABLED
--
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] [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap [incubator-uniffle]
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #2196:
URL: https://github.com/apache/incubator-uniffle/pull/2196#discussion_r1818284682
##########
server/src/main/java/org/apache/uniffle/server/ShuffleTaskManager.java:
##########
@@ -498,15 +502,23 @@ public void updateCachedBlockIds(
}
ShuffleTaskInfo shuffleTaskInfo =
shuffleTaskInfos.computeIfAbsent(appId, x -> new ShuffleTaskInfo(appId));
- Roaring64NavigableMap bitmap =
- shuffleTaskInfo
- .getCachedBlockIds()
- .computeIfAbsent(shuffleId, x -> Roaring64NavigableMap.bitmapOf());
-
long size = 0L;
- synchronized (bitmap) {
+ // With memory storage type should never need cachedBlockIds,
+ // since client do not need call finish shuffle rpc
+ if (!shuffleTaskInfo.isClientStorageTypeWithMemory()) {
Review Comment:
Maybe this config option should be same between server and client side.
--
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] [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap [incubator-uniffle]
Posted by "maobaolong (via GitHub)" <gi...@apache.org>.
maobaolong commented on PR #2196:
URL: https://github.com/apache/incubator-uniffle/pull/2196#issuecomment-2443160638
@jerqi Thanks for remind me, I have misunderstood your suggestion, now I throw exception for these two method, PTAL
--
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] [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap [incubator-uniffle]
Posted by "maobaolong (via GitHub)" <gi...@apache.org>.
maobaolong commented on code in PR #2196:
URL: https://github.com/apache/incubator-uniffle/pull/2196#discussion_r1816911715
##########
server/src/main/java/org/apache/uniffle/server/ShuffleTaskManager.java:
##########
@@ -498,15 +502,23 @@ public void updateCachedBlockIds(
}
ShuffleTaskInfo shuffleTaskInfo =
shuffleTaskInfos.computeIfAbsent(appId, x -> new ShuffleTaskInfo(appId));
- Roaring64NavigableMap bitmap =
- shuffleTaskInfo
- .getCachedBlockIds()
- .computeIfAbsent(shuffleId, x -> Roaring64NavigableMap.bitmapOf());
-
long size = 0L;
- synchronized (bitmap) {
+ // With memory storage type should never need cachedBlockIds,
+ // since client do not need call finish shuffle rpc
+ if (!shuffleTaskInfo.isClientStorageTypeWithMemory()) {
Review Comment:
@jerqi It is different, this is a client side config, it configured by client, so I have to get it from shuffle TaskInfo.
--
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] [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap [incubator-uniffle]
Posted by "maobaolong (via GitHub)" <gi...@apache.org>.
maobaolong closed pull request #2196: [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap
URL: https://github.com/apache/incubator-uniffle/pull/2196
--
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] [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap [incubator-uniffle]
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on PR #2196:
URL: https://github.com/apache/incubator-uniffle/pull/2196#issuecomment-2422220901
Why do we need to register app configuration to server?
--
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] [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap [incubator-uniffle]
Posted by "maobaolong (via GitHub)" <gi...@apache.org>.
maobaolong commented on PR #2196:
URL: https://github.com/apache/incubator-uniffle/pull/2196#issuecomment-2422444255
@jerqi I plan to use the client side Conf as a app level session config, base on this, we do not need to send BlockLayout for each reportShuffleResult, we only need to get it from app session Conf from ShuffleTaskInfo.
Leverage the appConf, we can easy to extends new features without proto change, even without client upgrade.
--
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] [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap [incubator-uniffle]
Posted by "maobaolong (via GitHub)" <gi...@apache.org>.
maobaolong closed pull request #2196: [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap
URL: https://github.com/apache/incubator-uniffle/pull/2196
--
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] [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap [incubator-uniffle]
Posted by "maobaolong (via GitHub)" <gi...@apache.org>.
maobaolong closed pull request #2196: [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap
URL: https://github.com/apache/incubator-uniffle/pull/2196
--
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] [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap [incubator-uniffle]
Posted by "maobaolong (via GitHub)" <gi...@apache.org>.
maobaolong commented on PR #2196:
URL: https://github.com/apache/incubator-uniffle/pull/2196#issuecomment-2434161394
ping @jerqi Sorry to remind
--
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] [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap [incubator-uniffle]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #2196:
URL: https://github.com/apache/incubator-uniffle/pull/2196#issuecomment-2421343699
## Test Results
2 551 files - 375 2 551 suites - 375 2h 43m 28s :stopwatch: - 3h 12m 34s
950 tests - 93 932 :white_check_mark: - 109 2 :zzz: ±0 0 :x: ±0 16 :fire: + 16
12 216 runs - 817 11 946 :white_check_mark: - 1 057 30 :zzz: ±0 0 :x: ±0 240 :fire: +240
For more details on these errors, see [this check](https://github.com/apache/incubator-uniffle/runs/31713235785).
Results for commit 73426703. ± Comparison against base commit 4c14b977.
<details>
<summary>This pull request <b>removes</b> 93 tests.</summary>
```
org.apache.spark.shuffle.FunctionUtilsTests ‑ testOnceFunction0
org.apache.spark.shuffle.RssShuffleManagerTest ‑ testCreateShuffleManagerServer
org.apache.spark.shuffle.RssShuffleManagerTest ‑ testGetDataDistributionType
org.apache.spark.shuffle.RssShuffleManagerTest ‑ testRssShuffleManagerInterface
org.apache.spark.shuffle.RssShuffleManagerTest ‑ testRssShuffleManagerRegisterShuffle{int}[1]
org.apache.spark.shuffle.RssShuffleManagerTest ‑ testRssShuffleManagerRegisterShuffle{int}[2]
org.apache.spark.shuffle.RssShuffleManagerTest ‑ testRssShuffleManagerRegisterShuffle{int}[3]
org.apache.spark.shuffle.RssShuffleManagerTest ‑ testWithStageRetry
org.apache.spark.shuffle.RssSpark3ShuffleUtilsTest ‑ testCreateFetchFailedException
org.apache.spark.shuffle.RssSpark3ShuffleUtilsTest ‑ testIsStageResubmitSupported
…
```
</details>
[test-results]:data:application/gzip;base64,H4sIAAPnEWcC/03MTQ7CIBCG4as0rF0AM5TWyxgylITYH0NhZby7g4a2y+9heN8ixHnaxb3TxqhbJ/YS83X7klyO28oyDnJg4edcD0Yj23rshagS6JOe8VUzBwQXZ4bz05TSllhUz5TKWqNK62O2qlIjnvbPgmxwyf52q2qsRNuyxMxTWEDdWwnGEUAgnBBIB4TeK7LWWINE4AcvPl+yAwi8FAEAAA==
--
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] [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap [incubator-uniffle]
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on PR #2196:
URL: https://github.com/apache/incubator-uniffle/pull/2196#issuecomment-2421592044
This may be better as a server configuration.
--
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] [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap [incubator-uniffle]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #2196:
URL: https://github.com/apache/incubator-uniffle/pull/2196#issuecomment-2421660916
## [Codecov](https://app.codecov.io/gh/apache/incubator-uniffle/pull/2196?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: Patch coverage is `50.00000%` with `25 lines` in your changes missing coverage. Please review.
> Project coverage is 52.72%. Comparing base [(`4c14b97`)](https://app.codecov.io/gh/apache/incubator-uniffle/commit/4c14b977def9d8f56a702ae54ee4881c5abd2cb3?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`62d571b`)](https://app.codecov.io/gh/apache/incubator-uniffle/commit/62d571b9a1fbb0b53309a8ccf8469df631333606?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 2 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/incubator-uniffle/pull/2196?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [.../apache/uniffle/client/api/ShuffleWriteClient.java](https://app.codecov.io/gh/apache/incubator-uniffle/pull/2196?src=pr&el=tree&filepath=client%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Funiffle%2Fclient%2Fapi%2FShuffleWriteClient.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS91bmlmZmxlL2NsaWVudC9hcGkvU2h1ZmZsZVdyaXRlQ2xpZW50LmphdmE=) | 0.00% | [6 Missing :warning: ](https://app.codecov.io/gh/apache/incubator-uniffle/pull/2196?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ffle/client/request/RssRegisterShuffleRequest.java](https://app.codecov.io/gh/apache/incubator-uniffle/pull/2196?src=pr&el=tree&filepath=internal-client%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Funiffle%2Fclient%2Frequest%2FRssRegisterShuffleRequest.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aW50ZXJuYWwtY2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS91bmlmZmxlL2NsaWVudC9yZXF1ZXN0L1Jzc1JlZ2lzdGVyU2h1ZmZsZVJlcXVlc3QuamF2YQ==) | 0.00% | [6 Missing :warning: ](https://app.codecov.io/gh/apache/incubator-uniffle/pull/2196?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ava/org/apache/uniffle/server/ShuffleTaskInfo.java](https://app.codecov.io/gh/apache/incubator-uniffle/pull/2196?src=pr&el=tree&filepath=server%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Funiffle%2Fserver%2FShuffleTaskInfo.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS91bmlmZmxlL3NlcnZlci9TaHVmZmxlVGFza0luZm8uamF2YQ==) | 61.53% | [2 Missing and 3 partials :warning: ](https://app.codecov.io/gh/apache/incubator-uniffle/pull/2196?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ffle/client/impl/grpc/ShuffleServerGrpcClient.java](https://app.codecov.io/gh/apache/incubator-uniffle/pull/2196?src=pr&el=tree&filepath=internal-client%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Funiffle%2Fclient%2Fimpl%2Fgrpc%2FShuffleServerGrpcClient.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aW50ZXJuYWwtY2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS91bmlmZmxlL2NsaWVudC9pbXBsL2dycGMvU2h1ZmZsZVNlcnZlckdycGNDbGllbnQuamF2YQ==) | 0.00% | [4 Missing :warning: ](https://app.codecov.io/gh/apache/incubator-uniffle/pull/2196?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...pache/uniffle/server/ShuffleServerGrpcService.java](https://app.codecov.io/gh/apache/incubator-uniffle/pull/2196?src=pr&el=tree&filepath=server%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Funiffle%2Fserver%2FShuffleServerGrpcService.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS91bmlmZmxlL3NlcnZlci9TaHVmZmxlU2VydmVyR3JwY1NlcnZpY2UuamF2YQ==) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/incubator-uniffle/pull/2196?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...org/apache/uniffle/server/ShuffleFlushManager.java](https://app.codecov.io/gh/apache/incubator-uniffle/pull/2196?src=pr&el=tree&filepath=server%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Funiffle%2Fserver%2FShuffleFlushManager.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS91bmlmZmxlL3NlcnZlci9TaHVmZmxlRmx1c2hNYW5hZ2VyLmphdmE=) | 50.00% | [0 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/incubator-uniffle/pull/2196?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [.../org/apache/uniffle/server/ShuffleTaskManager.java](https://app.codecov.io/gh/apache/incubator-uniffle/pull/2196?src=pr&el=tree&filepath=server%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Funiffle%2Fserver%2FShuffleTaskManager.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS91bmlmZmxlL3NlcnZlci9TaHVmZmxlVGFza01hbmFnZXIuamF2YQ==) | 91.66% | [0 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/incubator-uniffle/pull/2196?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #2196 +/- ##
============================================
+ Coverage 52.31% 52.72% +0.40%
+ Complexity 2814 2791 -23
============================================
Files 452 439 -13
Lines 21211 20811 -400
Branches 1950 1926 -24
============================================
- Hits 11097 10972 -125
+ Misses 9405 9150 -255
+ Partials 709 689 -20
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/incubator-uniffle/pull/2196?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
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] [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap [incubator-uniffle]
Posted by "maobaolong (via GitHub)" <gi...@apache.org>.
maobaolong commented on PR #2196:
URL: https://github.com/apache/incubator-uniffle/pull/2196#issuecomment-2425489596
@jerqi I see. It make sense about your concern. How about a trade off that we only filter and send the rss config to server by the `spark.rss` prefix?
--
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] [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap [incubator-uniffle]
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #2196:
URL: https://github.com/apache/incubator-uniffle/pull/2196#discussion_r1816397265
##########
server/src/main/java/org/apache/uniffle/server/ShuffleTaskInfo.java:
##########
@@ -307,4 +318,25 @@ public String toString() {
+ shuffleDetailInfos
+ '}';
}
+
+ public void setProperties(Map<String, String> properties, ShuffleServerConf serverConf) {
Review Comment:
Why do we need `serverConf` 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] [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap [incubator-uniffle]
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #2196:
URL: https://github.com/apache/incubator-uniffle/pull/2196#discussion_r1820003778
##########
server/src/main/java/org/apache/uniffle/server/ShuffleServerGrpcService.java:
##########
@@ -635,6 +645,15 @@ public void finishShuffle(
String appId = req.getAppId();
int shuffleId = req.getShuffleId();
auditContext.withAppId(appId).withShuffleId(shuffleId);
+ org.apache.uniffle.common.StorageType storageType =
+ shuffleServer.getShuffleServerConf().get(ShuffleServerConf.RSS_STORAGE_TYPE);
+ boolean storageTypeWithMemory =
+ StorageType.withMemory(StorageType.valueOf(storageType.name()));
+ if (storageTypeWithMemory) {
+ LOG.warn(
Review Comment:
I means that it's ok for me to throw exception 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] [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap [incubator-uniffle]
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #2196:
URL: https://github.com/apache/incubator-uniffle/pull/2196#discussion_r1818519245
##########
server/src/main/java/org/apache/uniffle/server/ShuffleServerGrpcService.java:
##########
@@ -635,6 +645,15 @@ public void finishShuffle(
String appId = req.getAppId();
int shuffleId = req.getShuffleId();
auditContext.withAppId(appId).withShuffleId(shuffleId);
+ org.apache.uniffle.common.StorageType storageType =
+ shuffleServer.getShuffleServerConf().get(ShuffleServerConf.RSS_STORAGE_TYPE);
+ boolean storageTypeWithMemory =
+ StorageType.withMemory(StorageType.valueOf(storageType.name()));
+ if (storageTypeWithMemory) {
+ LOG.warn(
Review Comment:
OK for me.
--
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] [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap [incubator-uniffle]
Posted by "maobaolong (via GitHub)" <gi...@apache.org>.
maobaolong commented on code in PR #2196:
URL: https://github.com/apache/incubator-uniffle/pull/2196#discussion_r1818252500
##########
server/src/main/java/org/apache/uniffle/server/ShuffleServerConf.java:
##########
@@ -719,6 +719,11 @@ public class ShuffleServerConf extends RssBaseConf {
.booleanType()
.defaultValue(false)
.withDescription("Whether to enable app detail log");
+ public static final ConfigOption<Boolean> SERVER_WITH_MEMORY_STORAGE_TYPE_OPTIMIZE_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
Re: [PR] [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap [incubator-uniffle]
Posted by "maobaolong (via GitHub)" <gi...@apache.org>.
maobaolong commented on PR #2196:
URL: https://github.com/apache/incubator-uniffle/pull/2196#issuecomment-2444103433
@jerqi Thanks for your review!
--
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] [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap [incubator-uniffle]
Posted by "maobaolong (via GitHub)" <gi...@apache.org>.
maobaolong commented on PR #2196:
URL: https://github.com/apache/incubator-uniffle/pull/2196#issuecomment-2440315668
@jerqi Thanks for reply and review, I renamed the property key, I can change to other name if you feel it is not good. PTAL
--
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] [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap [incubator-uniffle]
Posted by "maobaolong (via GitHub)" <gi...@apache.org>.
maobaolong merged PR #2196:
URL: https://github.com/apache/incubator-uniffle/pull/2196
--
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] [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap [incubator-uniffle]
Posted by "maobaolong (via GitHub)" <gi...@apache.org>.
maobaolong commented on code in PR #2196:
URL: https://github.com/apache/incubator-uniffle/pull/2196#discussion_r1818506364
##########
server/src/main/java/org/apache/uniffle/server/ShuffleServerGrpcService.java:
##########
@@ -635,6 +645,15 @@ public void finishShuffle(
String appId = req.getAppId();
int shuffleId = req.getShuffleId();
auditContext.withAppId(appId).withShuffleId(shuffleId);
+ org.apache.uniffle.common.StorageType storageType =
+ shuffleServer.getShuffleServerConf().get(ShuffleServerConf.RSS_STORAGE_TYPE);
+ boolean storageTypeWithMemory =
+ StorageType.withMemory(StorageType.valueOf(storageType.name()));
+ if (storageTypeWithMemory) {
+ LOG.warn(
Review Comment:
@jerqi If we throw exception, it will never support for test mode and `LOCALFILE` client to access the server with `LOCALFILE_MEMORY` storageType?
--
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] [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap [incubator-uniffle]
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #2196:
URL: https://github.com/apache/incubator-uniffle/pull/2196#discussion_r1816399429
##########
server/src/main/java/org/apache/uniffle/server/ShuffleTaskManager.java:
##########
@@ -498,15 +502,23 @@ public void updateCachedBlockIds(
}
ShuffleTaskInfo shuffleTaskInfo =
shuffleTaskInfos.computeIfAbsent(appId, x -> new ShuffleTaskInfo(appId));
- Roaring64NavigableMap bitmap =
- shuffleTaskInfo
- .getCachedBlockIds()
- .computeIfAbsent(shuffleId, x -> Roaring64NavigableMap.bitmapOf());
-
long size = 0L;
- synchronized (bitmap) {
+ // With memory storage type should never need cachedBlockIds,
+ // since client do not need call finish shuffle rpc
+ if (!shuffleTaskInfo.isClientStorageTypeWithMemory()) {
Review Comment:
We can get this config option from the server configuration directly instead getting from taskInfo.
--
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] [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap [incubator-uniffle]
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #2196:
URL: https://github.com/apache/incubator-uniffle/pull/2196#discussion_r1816396532
##########
server/src/main/java/org/apache/uniffle/server/ShuffleServerConf.java:
##########
@@ -719,6 +719,11 @@ public class ShuffleServerConf extends RssBaseConf {
.booleanType()
.defaultValue(false)
.withDescription("Whether to enable app detail log");
+ public static final ConfigOption<Boolean> SERVER_WITH_MEMORY_STORAGE_TYPE_OPTIMIZE_ENABLED =
Review Comment:
Could we use camel style for last segment and 4 - 5 segments for the whole config option like
`rss.server.storage.bitmapMemoryOptimizeEnabled`.
--
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] [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap [incubator-uniffle]
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #2196:
URL: https://github.com/apache/incubator-uniffle/pull/2196#discussion_r1806328484
##########
proto/src/main/proto/Rss.proto:
##########
@@ -193,6 +193,7 @@ message ShuffleRegisterRequest {
string comparatorClass = 11;
int32 mergedBlockSize = 12;
string mergeClassLoader = 13;
+ map<string, string> appConf = 1000;
Review Comment:
Prefer clear defining parameters instead of a map.
--
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] [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap [incubator-uniffle]
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on PR #2196:
URL: https://github.com/apache/incubator-uniffle/pull/2196#issuecomment-2425495175
> @jerqi I see. It make sense about your concern. How about a trade off that we only filter and send the rss config to server by filter the `spark.rss` prefix?
I feel that some options can be passed to the server. But important concepts should be extracted to explict fields in the interface such as BlockLayout
--
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] [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap [incubator-uniffle]
Posted by "maobaolong (via GitHub)" <gi...@apache.org>.
maobaolong commented on PR #2196:
URL: https://github.com/apache/incubator-uniffle/pull/2196#issuecomment-2425585820
@jerqi Thanks for your help for improve this PR.
- I change to use `properties` instead of `appConf` to be more generic to show that we only recommend to pass the common properties to server.
- Add a filter of properties to keep the rss client common properties only.
- Tested locally, verified there are only 9 items in this properties, it contains no default properties.
- Create an new issue to track the Reduce resend BlockLayout improvement https://github.com/apache/incubator-uniffle/issues/2212
--
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] [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap [incubator-uniffle]
Posted by "maobaolong (via GitHub)" <gi...@apache.org>.
maobaolong commented on code in PR #2196:
URL: https://github.com/apache/incubator-uniffle/pull/2196#discussion_r1820053411
##########
server/src/main/java/org/apache/uniffle/server/ShuffleServerGrpcService.java:
##########
@@ -635,6 +645,15 @@ public void finishShuffle(
String appId = req.getAppId();
int shuffleId = req.getShuffleId();
auditContext.withAppId(appId).withShuffleId(shuffleId);
+ org.apache.uniffle.common.StorageType storageType =
+ shuffleServer.getShuffleServerConf().get(ShuffleServerConf.RSS_STORAGE_TYPE);
+ boolean storageTypeWithMemory =
+ StorageType.withMemory(StorageType.valueOf(storageType.name()));
+ if (storageTypeWithMemory) {
+ LOG.warn(
Review Comment:
@jerqi Sorry for the misunderstand, 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] [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap [incubator-uniffle]
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #2196:
URL: https://github.com/apache/incubator-uniffle/pull/2196#discussion_r1818503939
##########
server/src/main/java/org/apache/uniffle/server/ShuffleServerGrpcService.java:
##########
@@ -635,6 +645,15 @@ public void finishShuffle(
String appId = req.getAppId();
int shuffleId = req.getShuffleId();
auditContext.withAppId(appId).withShuffleId(shuffleId);
+ org.apache.uniffle.common.StorageType storageType =
+ shuffleServer.getShuffleServerConf().get(ShuffleServerConf.RSS_STORAGE_TYPE);
+ boolean storageTypeWithMemory =
+ StorageType.withMemory(StorageType.valueOf(storageType.name()));
+ if (storageTypeWithMemory) {
+ LOG.warn(
Review Comment:
We should throw exception directly.
--
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] [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap [incubator-uniffle]
Posted by "maobaolong (via GitHub)" <gi...@apache.org>.
maobaolong closed pull request #2196: [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap
URL: https://github.com/apache/incubator-uniffle/pull/2196
--
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] [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap [incubator-uniffle]
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #2196:
URL: https://github.com/apache/incubator-uniffle/pull/2196#discussion_r1820158578
##########
server/src/main/java/org/apache/uniffle/server/ShuffleServerGrpcService.java:
##########
@@ -633,6 +648,18 @@ public void finishShuffle(
String appId = req.getAppId();
int shuffleId = req.getShuffleId();
auditContext.withAppId(appId).withShuffleId(shuffleId);
+ org.apache.uniffle.common.StorageType storageType =
+ shuffleServer.getShuffleServerConf().get(ShuffleServerConf.RSS_STORAGE_TYPE);
+ boolean storageTypeWithMemory =
+ StorageType.withMemory(StorageType.valueOf(storageType.name()));
+ if (storageTypeWithMemory) {
+ String errorMessage =
+ String.format(
+ "finishShuffle should not be called while server-side configured StorageType to %s for appId %s",
+ storageType, appId);
+ LOG.error(errorMessage);
+ throw new IllegalStateException(errorMessage);
Review Comment:
UnsupportedException?
--
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] [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap [incubator-uniffle]
Posted by "maobaolong (via GitHub)" <gi...@apache.org>.
maobaolong commented on PR #2196:
URL: https://github.com/apache/incubator-uniffle/pull/2196#issuecomment-2421643989
@jerqi Done, add a new configuration
--
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] [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap [incubator-uniffle]
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on PR #2196:
URL: https://github.com/apache/incubator-uniffle/pull/2196#issuecomment-2425432916
> @jerqi I plan to use the client side Conf as a app level session config, base on this, we do not need to send BlockLayout for each reportShuffleResult, we only need to get it from app session Conf from ShuffleTaskInfo.
>
> Leverage the appConf, we can easy to extends new features without proto change, even without client upgrade.
I know. But I am no sure that this is a good idea. Application configuration can contain many things. It lacks constraint. We can't get enough information from the interface. It's easy to develop but it's hard to understand.
--
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] [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap [incubator-uniffle]
Posted by "maobaolong (via GitHub)" <gi...@apache.org>.
maobaolong closed pull request #2196: [#2197] Support reg app conf to server and avoid update committed/cached blockIds bitmap
URL: https://github.com/apache/incubator-uniffle/pull/2196
--
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