You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@uniffle.apache.org by "ConeyLiu (via GitHub)" <gi...@apache.org> on 2026/09/17 12:51:35 UTC
[PR] [#2790] fix(server): Return NO_REGISTER when shuffle metadata is missing [uniffle]
ConeyLiu opened a new pull request, #2791:
URL: https://github.com/apache/uniffle/pull/2791
### What changes were proposed in this pull request?
Return `NO_REGISTER` instead of a successful empty bitmap when shuffle registration metadata is missing.
Also prevent shuffle result queries from recreating an expired application.
### Why are the changes needed?
After an application is removed, a task retry may query an old shuffle and incorrectly receive an empty bitmap with `SUCCESS`, potentially causing data loss.
Fix: #2790
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Added tests for expired applications, missing shuffles, and valid empty shuffle results.
--
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] [#2790] fix(server): Return NO_REGISTER when shuffle metadata is missing [uniffle]
Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston commented on code in PR #2791:
URL: https://github.com/apache/uniffle/pull/2791#discussion_r4059771382
##########
server/src/main/java/org/apache/uniffle/server/ShuffleTaskManager.java:
##########
@@ -629,34 +629,43 @@ public void releaseMemory(
public byte[] getFinishedBlockIds(
String appId, Integer shuffleId, Set<Integer> partitions, BlockIdLayout blockIdLayout)
throws IOException {
- refreshAppId(appId);
Review Comment:
> However, the timestamp refresh should be preserved for valid requests
make sense!
--
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] [#2790] fix(server): Return NO_REGISTER when shuffle metadata is missing [uniffle]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #2791:
URL: https://github.com/apache/uniffle/pull/2791#issuecomment-5723695573
## Test Results
3 416 files - 383 3 416 suites - 383 4h 12m 29s ⏱️ - 3h 37m 55s
1 177 tests - 89 1 161 ✅ - 93 11 💤 ±0 0 ❌ ±0 5 🔥 + 4
18 329 runs - 763 18 191 ✅ - 852 48 💤 ±0 0 ❌ ±0 90 🔥 +89
For more details on these errors, see [this check](https://github.com/apache/uniffle/runs/105448412364).
Results for commit 12a16ebd. ± Comparison against base commit b9b5d6d9.
<details>
<summary>This pull request <b>removes</b> 89 tests.</summary>
```
org.apache.uniffle.test.AQERepartitionTest ‑ resultCompareTest
org.apache.uniffle.test.AQESkewedJoinTest ‑ resultCompareTest
org.apache.uniffle.test.AQESkewedJoinWithLocalOrderTest ‑ resultCompareTest
org.apache.uniffle.test.AutoAccessTest ‑ test{File}
org.apache.uniffle.test.CombineByKeyTest ‑ combineByKeyTest
org.apache.uniffle.test.CompressionOverlappingTest ‑ resultCompareTest
org.apache.uniffle.test.ContinuousSelectPartitionStrategyTest ‑ resultCompareTest
org.apache.uniffle.test.DynamicConfTest ‑ dynamicConfTest{ClientType}[1]
org.apache.uniffle.test.DynamicConfTest ‑ dynamicConfTest{ClientType}[2]
org.apache.uniffle.test.DynamicFetchClientConfTest ‑ test
…
```
</details>
[test-results]:data:application/gzip;base64,H4sIAHOVrGoC/03PTQ6DIBCG4asY1l04gMj0Mg0iJKT+NAgr07t3pJW6/B4mb8LOfJjcxu6NkKBuDdtySNc95mhSWBcS6EAiEb2n4wKg78/52LK1xRT87Rlexap4EyaStoKLcY0kHUnMS8lqwfE3a1YDQrVvVuoTLtWyzygeYtd5DokWA25AuWE0TnvP6TOgUODAtRlsx7H3wjvTjoK9P0njWfIWAQAA
--
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] [#2790] fix(server): Return NO_REGISTER when shuffle metadata is missing [uniffle]
Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston merged PR #2791:
URL: https://github.com/apache/uniffle/pull/2791
--
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] [#2790] fix(server): Return NO_REGISTER when shuffle metadata is missing [uniffle]
Posted by "ConeyLiu (via GitHub)" <gi...@apache.org>.
ConeyLiu commented on PR #2791:
URL: https://github.com/apache/uniffle/pull/2791#issuecomment-5714681561
@roryqi @zuston @wForget pls help to review this when you have a time.
--
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] [#2790] fix(server): Return NO_REGISTER when shuffle metadata is missing [uniffle]
Posted by "ConeyLiu (via GitHub)" <gi...@apache.org>.
ConeyLiu commented on code in PR #2791:
URL: https://github.com/apache/uniffle/pull/2791#discussion_r4045100159
##########
server/src/main/java/org/apache/uniffle/server/ShuffleTaskManager.java:
##########
@@ -629,34 +629,43 @@ public void releaseMemory(
public byte[] getFinishedBlockIds(
String appId, Integer shuffleId, Set<Integer> partitions, BlockIdLayout blockIdLayout)
throws IOException {
- refreshAppId(appId);
Review Comment:
Thanks for pointing this out. `refreshAppId` was removed intentionally because it may recreate an expired application via `computeIfAbsent`.
However, the timestamp refresh should be preserved for valid requests. I have added `taskInfo.setCurrentTimes(...)` after validating the application, shuffle, and partitions, so valid queries still refresh the application while invalid queries cannot recreate or keep it alive.
--
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] [#2790] fix(server): Return NO_REGISTER when shuffle metadata is missing [uniffle]
Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston commented on PR #2791:
URL: https://github.com/apache/uniffle/pull/2791#issuecomment-5756393748
The CI checks are still failing. Could you please take another look @ConeyLiu
--
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] [#2790] fix(server): Return NO_REGISTER when shuffle metadata is missing [uniffle]
Posted by "ConeyLiu (via GitHub)" <gi...@apache.org>.
ConeyLiu commented on PR #2791:
URL: https://github.com/apache/uniffle/pull/2791#issuecomment-5756738980
@zuston thanks for your reviewing. The CI failures seem unrelated to this PR. `testRetryAssgin` is flaky because only one shuffle server was registered before the test started, and the `mr-hadoop3.2` failure was caused by a Codecov `ECONNRESET`. A rerun should resolve them.
--
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] [#2790] fix(server): Return NO_REGISTER when shuffle metadata is missing [uniffle]
Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston commented on PR #2791:
URL: https://github.com/apache/uniffle/pull/2791#issuecomment-5770381668
let me merge this.
@ConeyLiu thanks for your contribution, welcome to the Uniffle Community!
--
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] [#2790] fix(server): Return NO_REGISTER when shuffle metadata is missing [uniffle]
Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston commented on code in PR #2791:
URL: https://github.com/apache/uniffle/pull/2791#discussion_r4043553343
##########
server/src/main/java/org/apache/uniffle/server/ShuffleTaskManager.java:
##########
@@ -629,34 +629,43 @@ public void releaseMemory(
public byte[] getFinishedBlockIds(
String appId, Integer shuffleId, Set<Integer> partitions, BlockIdLayout blockIdLayout)
throws IOException {
- refreshAppId(appId);
Review Comment:
this logic of `refreshAppId` is missing?
--
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] [#2790] fix(server): Return NO_REGISTER when shuffle metadata is missing [uniffle]
Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston commented on code in PR #2791:
URL: https://github.com/apache/uniffle/pull/2791#discussion_r4060046933
##########
server/src/main/java/org/apache/uniffle/server/ShuffleTaskManager.java:
##########
@@ -629,34 +629,43 @@ public void releaseMemory(
public byte[] getFinishedBlockIds(
String appId, Integer shuffleId, Set<Integer> partitions, BlockIdLayout blockIdLayout)
throws IOException {
- refreshAppId(appId);
Review Comment:
By the way, we might want to introduce separate methods for `createOrRefresh` and `refresh` . That said, I’m also fine with the current approach.
<img width="1011" height="397" alt="Image" src="https://github.com/user-attachments/assets/5506e19c-eb99-4837-812f-e5b64246abec" />
--
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