[
https://issues.apache.org/jira/browse/HDDS-16767?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16767:
------------------------------
Attachment: TestBugCR6RecoverLeaseWallClock.java
> RecoverLease soft limit check uses the local wall clock at apply, so OMs
> diverge on the same log entry
> ------------------------------------------------------------------------------------------------------
>
> Key: HDDS-16767
> URL: https://issues.apache.org/jira/browse/HDDS-16767
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Major
> Attachments: CR-6-recover-lease-soft-limit-uses-leader-time.patch,
> TestBugCR6RecoverLeaseWallClock.java
>
>
> h3. Mechanism
> {{OMRecoverLeaseRequest.validateAndUpdateCache}} ({{doWork}}) compares
> {{Time.now()}} with {{openKey.modificationTime + ozone.om.lease.soft.limit}}
> and, on the pass branch, persists {{LEASE_RECOVERY=true}} and
> {{modificationTime=Time.now()}}. The check runs at Ratis apply on every OM,
> not in {{preExecute}}, so the branch taken depends on when each replica
> applies the entry.
> h3. Trigger
> 3 OM HA with the default 60 s soft limit, one follower down or lagging.
> # A writer hsyncs an FSO file.
> # Another client calls {{recoverLease}} (not forced) inside the soft limit.
> The leader rejects it with {{KEY_UNDER_LEASE_SOFT_LIMIT_PERIOD}}.
> # The follower applies the same entry after the soft limit has passed and
> accepts it.
> h3. Impact
> The late OM's {{openFileTable}} row carries {{LEASE_RECOVERY=true}} and the
> others do not. The writer's later close is acknowledged by the leader but
> rejected with {{KEY_UNDER_LEASE_RECOVERY}} on the late OM, whose
> {{fileTable}} keeps the old length and the hsync marker. After leadership
> moves to that OM, {{getFileStatus}} returns the old length and
> {{isFileClosed}} returns false for a file whose close was acknowledged. The
> data bytes remain readable. Only a snapshot install reconciles it.
> h3. Reproduction
> PASS with default settings: [^TestBugCR6RecoverLeaseWallClock.java]
> ({{hadoop-ozone/integration-test}}), 3 OM mini cluster with one follower
> applying late. Only the {{OMRecoverLeaseRequest}} clock branch was
> reproduced. The {{Time.now()}} stored as {{modificationTime}} of the
> overwritten hsync open key at {{OMKeyCommitRequestWithFSO.java:210}} and
> {{OMKeyCommitRequest.java:288}}, and of a renamed key at
> {{OMKeysRenameRequest.java:202}}, is the same pattern but weaker, from code
> only: no apply path branches on those values, they only make the rows differ
> between OMs.
> h3. Suggested fix
> Make the soft limit decision once on the leader and replicate it: carry a
> leader assigned timestamp in the request and compare against that in apply.
> Use the same timestamp for the persisted {{modificationTime}}.
> h3. Patch
> [^CR-6-recover-lease-soft-limit-uses-leader-time.patch], against
> 20e6a1c0f6039248373efd3eb1d7f2afcf7f1535. It also applies to master at
> c99d291e863.
> {{RecoverLeaseRequest}} gets {{optional uint64 modificationTime = 5}}.
> {{OMRecoverLeaseRequest.preExecute}} sets it to {{Time.now()}} on the leader,
> overwriting anything a client sends, and {{doWork}} uses it for both the soft
> limit comparison and the stored {{modificationTime}}. When the field is
> absent (an entry written by a leader without this change) apply falls back to
> the local clock, which is the old behaviour, so no layout feature is needed.
> Covered by {{testLeaseSoftLimitUsesTimeOfRequest}} in the existing
> {{TestOMRecoverLeaseRequest}} (clock moved with a static mock, no sleeps),
> which fails without the change with "expected:
> <KEY_UNDER_LEASE_SOFT_LIMIT_PERIOD> but was: <OK>", and
> {{testLeaseSoftLimitUsesLocalTimeIfRequestHasNone}} for the fallback.
> {{TestOMRecoverLeaseRequest}} (13 tests) and the key commit unit suites pass,
> checkstyle is clean. {{TestLeaseRecovery}} has one failure,
> {{testGetCommittedBlockLengthWithException}}, which fails the same way
> without the patch. The proto backwards compatibility check was not run.
> Not addressed: the three weaker {{Time.now()}} sites above, and
> {{OMRecoverLeaseRequest.updateBlockInfo}} calling SCM from apply after the
> cache was already updated, which can turn one OM's apply into an error
> response. Both are worth separate issues.
> Found by TLA+ model checking and code review of the OM FSO namespace mutation
> paths under HDDS-15926, on commit 20e6a1c0f6039248373efd3eb1d7f2afcf7f1535.
> Checked against HDDS issues and apache/ozone pull requests for duplicates
> before filing. The attached patch is a proposal for review. Generated with
> Specula (Claude Opus 5.5).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
