imavishek opened a new pull request, #23246:
URL: https://github.com/apache/kafka/pull/23246

   ### JIRA
   https://issues.apache.org/jira/browse/KAFKA-20977
   
   ### Problem
   `UnifiedLog.onlyLocalLogSegmentsCount()` / `onlyLocalLogSegmentsSize()` 
filtered segments with
   `baseOffset() >= highestOffsetInRemoteStorage()`. 
`highestOffsetInRemoteStorage` holds the **end**
   offset of the last segment already copied to remote, so a segment whose 
base-offset equals that
   value has itself already been copied. This happens for **single-record 
segments**
   (`baseOffset == endOffset == highestOffsetInRemoteStorage`), which are 
common on low-throughput
   tiered-storage partitions. The `>=` comparison wrongly counted that 
already-copied boundary
   segment as "only local".
   
   `RemoteLogManager.RLMCopyTask.recordLagStats` derives 
`RemoteCopyLagSegments` as
   `onlyLocalLogSegmentsCount() - 1` (the `- 1` removes the active segment, per 
KAFKA-16895) and only
   refreshes the gauge after a copy. So on a partition that has gone idle the 
metric stays latched at a
   **phantom lag of 1** even though every closed segment is already in remote 
storage and only the
   active segment remains local. `RemoteCopyLagBytes` and the size-based 
retention total
   (`onlyLocalLogSegmentsSize + remoteLogSizeBytes`) are inflated the same way. 
Summed across
   partitions this looks like a growing copy backlog when nothing is actually 
lagging.
   
   This is a residual off-by-one on top of KAFKA-16895: that fix removed the 
*active* segment from the
   count; this one removes the *already-copied boundary* segment.
   
   ### Change
   Use a strict `>` in both `onlyLocalLogSegmentsCount()` and 
`onlyLocalLogSegmentsSize()` — a segment
   is "only local" iff its base-offset is strictly greater than 
`highestOffsetInRemoteStorage()`.
   Javadoc added explaining the boundary condition.
   
   ### Tests
   Added two `UnifiedLogTest` cases:
   - `onlyLocalLogSegmentsExcludeCopiedSingleRecordBoundarySegment` — 
single-record segments; asserts
     the count/lag is 0 once all closed segments are copied. **Fails on the 
previous `>=`**
     (`expected: <2> but was: <3>`), passes with the fix.
   - `onlyLocalLogSegmentsWithMultiRecordSegmentsIsUnaffectedByFix` — 
multi-record segments; guards the
     common case where `>=` and `>` agree so the fix does not change correct 
behavior.
   
   Verified locally:
   ```
   ./gradlew :storage:test --tests "*UnifiedLogTest.onlyLocalLogSegments*"
   ```
   Both pass; `checkstyleMain`, `checkstyleTest`, and `spotbugsMain` are clean.
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to