fix(inkless): POD-1965 Use local log start in DisklessLeaderEndPoint#591
Open
viktorsomogyi wants to merge 1 commit into
Open
fix(inkless): POD-1965 Use local log start in DisklessLeaderEndPoint#591viktorsomogyi wants to merge 1 commit into
viktorsomogyi wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adjusts consolidation fetch behavior so the DisklessLeaderEndPoint does not propagate the coordinator-provided “diskless start offset” as the fetch response logStartOffset, preventing ReplicaManager from treating that as the true log start and incorrectly enforcing retention boundaries.
Changes:
- Update
DisklessLeaderEndPoint.fetchto prefer a locally-derived log start offset (and handle lookup failures). - Expand
DisklessLeaderEndPointTestcoverage around log start offset overlaying and error precedence. - Clarify offset semantics via comments in SQL migrations.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| storage/inkless/src/main/resources/db/migration/V11__Init_diskless_log.sql | Adds clarification comment for diskless_start_offset in init request type. |
| storage/inkless/src/main/resources/db/migration/V1__Create_tables.sql | Adds an explanatory comment on logs.log_start_offset (needs consistency with V11 semantics). |
| core/src/main/scala/io/aiven/inkless/consolidation/DisklessLeaderEndPoint.scala | Changes fetch response mapping to use a locally-derived LSO and adds inconsistency logging. |
| core/src/test/scala/io/aiven/inkless/consolidation/DisklessLeaderEndPointTest.scala | Updates existing fetch tests and adds new cases for LSO behavior under mixed diskless/local lookup outcomes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
viktorsomogyi
force-pushed
the
svv/ts-unification-local-lso
branch
from
3454f3f to
4c9d846
Compare
viktorsomogyi
force-pushed
the
svv/ts-unification-local-lso
branch
from
4c9d846 to
7e2313e
Compare
Use local log's log start offset instead of the one that is stored in the diskless coordinator. This is needed for consolidating partitions as they expect that the log start at the beginning of the already consolidated log that is in UnifiedLog. Fetching with FetchHandler returns the log_start_offset from the coordinator which is actually the diskless start offset. If we pass this on, then ReplicaManager will believe that log_start_offset is the first offset and therefore enforces retention. Instead of this behavior we should use the local log start offset to properly respect the offset boundaries and retention.
viktorsomogyi
force-pushed
the
svv/ts-unification-local-lso
branch
from
7e2313e to
ca597a3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fetching with FetchHandler returns the log_start_offset from the coordinator which is actually the diskless start offset. If we pass this on, then ReplicaManager will believe that log_start_offset is the first offset and therefore enforces retention.
Instead of this behavior we should use the local log start offset to properly respect the offset boundaries and retention.