You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@fluss.apache.org by "gyang94 (via GitHub)" <gi...@apache.org> on 2026/09/10 06:52:11 UTC
[PR] [server] Normalize legacy remote log manifest ranges [fluss]
gyang94 opened a new pull request, #4274:
URL: https://github.com/apache/fluss/pull/4274
### Purpose
Fixes #4272.
Loading a legacy manifest containing `[10,30)` followed by `[10,20)` can make offset 20 unreadable: the shorter segment replaces the longer one in the offset index. Nested ranges with different starts can similarly hide existing records, and subsequent merges can shrink readable coverage.
This change establishes a consistent logical view before indexing or merging legacy manifests, independently of entry order.
### Brief change log
- Add deterministic logical-range normalization: discard contained ranges and clip overlapping suffixes when a segment extends coverage. Preserve existing logical bounds, gaps, and the highest copied end offset.
- Use the normalized manifest for the tablet's indexes, statistics, and current snapshot. Normalize before deletion in `trimAndMerge()` so removing a visible segment cannot restore a hidden range.
- Keep raw JSON deserialization unchanged so orphan cleanup still sees all persisted physical-file references. Loading does not rewrite snapshots or delete remote files.
As with the existing merge logic, normalization assumes overlapping committed records agree; it does not reconcile divergent WAL histories.
### Tests
Passed targeted verification on Java 11:
```bash
mvn -o -pl fluss-common,fluss-server -am verify \
'-Dtest=RemoteLog*Test,DefaultRemoteLogStorageTest,TieredLocalSegmentTTLTest,CommitRemoteLogManifestITCase' \
-Dsurefire.failIfNoSpecifiedTests=false
```
- 162 relevant test cases passed: 20 in common and 142 in server.
- Reproduced the legacy loading and coverage-shrinking merge failures before applying the fix.
- Added coverage for 120 entry permutations, identical/nested ranges, idempotence, existing clipped ranges, gaps, TTL deletion, copy progress, and persistence/reload.
- Verified KV recovery and FetchLog v0 reads against real overlapping remote files, including offsets and checksums in both manifest orders.
- Spotless, Checkstyle, and RAT passed.
### API and Format
Uses the existing version-1 manifest fields and logical-range model. No RPC or storage-format changes.
### Documentation
No new configuration or user-facing feature; no documentation changes.
Generative AI used: Codex (GPT-6).
--
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@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [server] Normalize legacy remote log manifest ranges [fluss]
Posted by "platinumhamburg (via GitHub)" <gi...@apache.org>.
platinumhamburg merged PR #4274:
URL: https://github.com/apache/fluss/pull/4274
--
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@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [server] Normalize legacy remote log manifest ranges [fluss]
Posted by "platinumhamburg (via GitHub)" <gi...@apache.org>.
platinumhamburg commented on code in PR #4274:
URL: https://github.com/apache/fluss/pull/4274#discussion_r3978863163
##########
fluss-server/src/test/java/org/apache/fluss/server/kv/RemoteLogFetcherTest.java:
##########
@@ -182,6 +238,45 @@ void testFetchOverlappingSegmentsFromReplicasWithDifferentBoundaries() throws Ex
assertThat(batch.baseLogOffset()).isEqualTo(offset);
assertThat(batch.nextLogOffset()).isEqualTo(offset + 1L);
}
+ assertThat(fetchChecksumsUsingFetchV0(targetBucket, 0L, 30L))
+ .containsExactlyElementsOf(expectedChecksums);
+ assertThat(fetchChecksumsUsingFetchV0(targetBucket, 20L, 30L))
+ .containsExactlyElementsOf(expectedChecksums.subList(20, 30));
+ }
+
+ private List<Long> fetchChecksumsUsingFetchV0(TableBucket bucket, long offset, long endOffset)
Review Comment:
@gyang94 Thanks for the fix! The normalization addresses the legacy overlap issue, and the explicit checks for version 1 and the absence of logical offset fields make the legacy-format coverage clear and deterministic.
One small suggestion: would you consider exercising the actual FetchLog RPC path instead of the custom read loop in fetchChecksumsUsingFetchV0()? Checking the returned segment and position at the shorter segment’s end offset could provide more direct regression coverage.
--
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@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org