Siyao Meng created HDDS-16767:
---------------------------------

             Summary: 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
         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]

Reply via email to