[
https://issues.apache.org/jira/browse/HDDS-16848?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16848:
------------------------------
Attachment: TestBugMC1StaleAutoCommit.java
> Hard lease auto commit built before a later hsync shrinks the key below the
> length OM acknowledged and deletes the acknowledged block
> -------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-16848
> URL: https://issues.apache.org/jira/browse/HDDS-16848
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Major
> Attachments: MC-1-reject-commit-shorter-than-hsynced-length.patch,
> TestBugMC1StaleAutoCommit.java
>
>
> h3. Mechanism
> {{OmMetadataManagerImpl.getExpiredOpenKeys}} builds the hard lease auto
> commit of an hsync'ed key from the committed key entry ({{keyTable}} or
> {{fileTable}}) as it is at scan time: its {{dataSize}} and its block list.
> {{OpenKeyCleanupService}} submits the {{CommitKey}} requests after the scan,
> one after the other. {{OMKeyCommitRequest.validateAndUpdateCache}} and
> {{OMKeyCommitRequestWithFSO}} then apply the request with no check that the
> key is still in the scanned state. The only checks related to the hsync state
> are that the open key exists, that it has no {{DELETED_HSYNC_KEY}} or
> {{OVERWRITTEN_HSYNC_KEY}} marker and that it has no {{LEASE_RECOVERY}} flag.
> If the writer allocates a block and hsyncs between the scan and the apply,
> the stale commit sets the key length back to the scan time value, treats the
> new block as allocated but uncommitted
> ({{OmKeyInfo.updateLocationInfoList}}), moves it to {{deletedTable}} through
> {{wrapUncommittedBlocksAsPseudoKey}}, and removes the open key.
> h3. Trigger
> {{ozone.fs.hsync.enabled}} and {{ozone.hbase.enhancements.allowed}} are true
> (both default false). Any bucket layout.
> # A writer writes into a block and hsyncs.
> # Its open key reaches {{ozone.om.lease.hard.limit}} (default 7d). The
> modification time of the open key moves only on block allocation and on the
> first hsync, so this happens to a writer that was silent at OM for 7 days,
> and also to a live writer that stayed inside one block for 7 days
> (HDDS-16825).
> # A cleanup run scans the open key table and builds the auto commit.
> # Before that commit is applied, the writer moves to a new block, writes and
> hsyncs. OM acknowledges the longer length.
> # The auto commit is applied.
> The window in step 4 is the rest of the scan, the {{DeleteOpenKeys}} request
> of the same run, the auto commits submitted before this one and its own Ratis
> round trip. It is narrow, from one Ratis round trip to seconds on a large
> open key table (argued from code, not measured on a cluster).
> h3. Impact
> * The key is committed with the scan time length. Data OM had acknowledged to
> the writer is no longer readable.
> * The acknowledged block is queued in {{deletedTable}}, so
> {{KeyDeletingService}} sends it to SCM for deletion. Nothing restores it.
> * The open key is gone, but the stock client only calls OM on the first hsync
> of each block, so further hsyncs into the deleted block keep succeeding
> against the datanodes until the next block allocation or close fails with
> {{KEY_NOT_FOUND}} (argued from client code, not run).
> h3. Reproduction
> PASS, deterministic, unmodified source, for OBJECT_STORE and
> FILE_SYSTEM_OPTIMIZED buckets. [^TestBugMC1StaleAutoCommit.java]
> ({{hadoop-ozone/ozone-manager}}) runs one real OM through
> {{OzoneManagerProtocol}} with a 200 ms hard lease limit. It performs the two
> halves of {{OpenKeyCleanupTask.call}} itself, the real
> {{KeyManager.getExpiredOpenKeys}} scan and the submission of its
> {{CommitKey}} to the OM Ratis server built as the service builds it, and
> issues the writer's allocate block and hsync between them, so no thread has
> to be paused. Output on the OBS bucket: "Acknowledged length 3000 with blocks
> [13046302476, 13046302821]; after the auto commit: length 1000 with blocks
> [13046302476], blocks queued for deletion [13046302821]". The FSO bucket
> gives the same result. A passing test means the defect is present. Physical
> deletion on datanodes was not run.
> h3. Patch
> [^MC-1-reject-commit-shorter-than-hsynced-length.patch], against
> a6b7bdb937109ba2893688b41a89470bc95c88cf. It also applies to master at
> b0aa6475b78.
> A commit that is neither an hsync nor a recovery commit, sent by the client
> that holds the hsync'ed key, is rejected with {{INVALID_REQUEST}} when its
> length is smaller than the length of the hsync'ed key in the key table. A
> stale auto commit is exactly this case. A correct client never sends it,
> because {{KeyOutputStream}} only sends growing lengths and the final close is
> never shorter than an earlier hsync. Hsync and recovery commits are not
> changed, and the next cleanup run commits from fresh state if the lease is
> still expired. Both {{OMKeyCommitRequest}} and {{OMKeyCommitRequestWithFSO}}
> get the check.
> The check changes what the apply writes, so it is enabled by a new
> {{OzoneManagerVersion.HSYNC_COMMIT_VALIDATION}} (101) through
> {{OMVersionManager.isAllowed}}. Until the cluster is finalized to that
> version every OM applies the commit as before, so OMs on different software
> versions never apply the same log entry differently, and the defect remains
> until finalization. The version number is a placeholder: it would be the
> first OM version after the 2.x baseline, so a cluster at version 100 reports
> that finalization is needed after upgrading, and other open fixes may claim
> the same number. Which version (or a shared one) to use is for review to
> decide.
> A stricter alternative is to carry the scanned {{updateID}} in
> {{CommitKeyRequest}} and compare it on apply. It would also reject a stale
> commit of equal length (which cannot lose acknowledged data), but it needs a
> new request field and a change to the cleanup service, and it changes what
> the apply writes just as the length check does. The length check uses only
> state the apply already reads and rejects only the case that loses
> acknowledged data. Reading the key again in the service before each submit
> would only narrow the window.
> Covered by {{testCommitShorterThanHsyncedKey}} in the existing
> {{TestOMKeyCommitRequest}}, inherited by {{TestOMKeyCommitRequestWithFSO}},
> once with the version finalized (the stale commit is rejected, a full close
> is accepted) and once before finalization (the stale commit is applied as
> before). Without the change the finalized case fails for both layouts with
> "expected: <INVALID_REQUEST> but was: <OK>". With it the commit request,
> commit response, recover lease, key rename, key delete, metadata manager,
> open key cleanup, OM storage and OM version suites pass (222 tests),
> {{TestHSync}} passes on a mini cluster (42 tests, with both patches), and
> checkstyle is clean. With the patch the reproduction fails at its first
> assertion because the auto commit is rejected and the key keeps 3000 bytes.
> Found by TLA+ model checking and code review of the OM open key cleanup and
> hsync lease recovery paths under HDDS-15926, on commit
> a6b7bdb937109ba2893688b41a89470bc95c88cf. Checked against HDDS issues and
> apache/ozone pull requests for duplicates before filing; the closest are
> HDDS-16764 (the same staleness on the {{DeleteOpenKeys}} path) and HDDS-16825
> (how a live writer reaches the hard limit). 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]
