[
https://issues.apache.org/jira/browse/HDDS-16852?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16852:
------------------------------
Attachment:
CR-7-reserve-hsync-metadata-names-and-tolerate-invalid-values.patch
> Reserve the OM owned hsync key metadata names at the request boundary and
> tolerate an invalid stored hsync client id in its readers
> -----------------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-16852
> URL: https://issues.apache.org/jira/browse/HDDS-16852
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Major
> Attachments:
> CR-7-reserve-hsync-metadata-names-and-tolerate-invalid-values.patch
>
>
> h3. Mechanism
> The OM records the hsync writer of a key in a key metadata entry
> ({{OzoneConsts.HSYNC_CLIENT_ID}}), alongside the other OM owned names
> {{OzoneConsts.LEASE_RECOVERY}}, {{OzoneConsts.DELETED_HSYNC_KEY}} and
> {{OzoneConsts.OVERWRITTEN_HSYNC_KEY}}. These names are set by the OM itself.
> One request type already strips them from the key metadata it receives; other
> request types do not reserve them.
> The readers of the stored writer id then parse it with {{Long.parseLong}} and
> assume the derived open key exists.
> {{OMKeyCommitRequest.validateAndUpdateCache}} and
> {{OMKeyCommitRequestWithFSO.prepareCommit}} parse it on the recovery path,
> and on the overwrite path both of them parse the id of the key being
> overwritten, build the open key name from it and read that open key entry to
> add {{OVERWRITTEN_HSYNC_KEY}} to it. {{OMRecoverLeaseRequest.doWork}} parses
> it to build the open file name it recovers.
> A stored value that is not a valid long, or a valid long with no matching
> open key row, makes these paths fail with an unchecked exception instead of
> an {{OMException}}, and nothing in them checks the value before using it.
> h3. Trigger
> # A key table row carries an {{HSYNC_CLIENT_ID}} metadata entry whose value
> is stale (the previous writer's own id, kept on the key after that writer
> committed, so the open key row is gone) or is not parseable as a long.
> # A commit of the same key arrives on the overwrite path, or a recovery
> commit or a lease recovery runs against that key.
> Both bucket layouts are affected: the overwrite path exists in
> {{OMKeyCommitRequest}} and in {{OMKeyCommitRequestWithFSO}}, and both
> recovery commit paths parse the same stored value.
> h3. Impact
> * A commit that overwrites a key whose stored writer id is stale or does not
> parse, and a recovery commit or lease recovery of such a key, fail with an
> unchecked exception instead of an {{OMException}} with a request status. With
> the patch the overwrite completes as a plain overwrite and the recovery paths
> return {{KEY_NOT_FOUND}}.
> * No key and no data is lost by the defect itself, and the stored value stays
> on the key until a later commit replaces it.
> h3. Reproduction
> There is no standalone reproduction. The regression tests in the attached
> patch set the state internally and fail against the unmodified main sources:
> * {{TestOMKeyCommitRequest.testOverwriteKeyWithStaleHsyncClientId}} (also in
> the inherited {{...WithFSO}}): a stored writer id that is stale and one that
> is not parseable, overwritten by a plain commit from another client. The
> commit must succeed and the new key must not carry the id.
> * {{TestOMKeyCommitRequest.testRecoveryCommitWithInvalidHsyncClientId}}: a
> recovery commit against a key whose stored id is not parseable must return
> {{KEY_NOT_FOUND}}, and the stored value must stay on the key.
> * {{TestOMRecoverLeaseRequest.testRecoverFileWithInvalidHsyncClientId}}: a
> lease recovery of a file whose stored id is not parseable must return
> {{KEY_NOT_FOUND}} and must not mark the open file with {{LEASE_RECOVERY}}.
> * {{TestOMKeyCommitRequest.testPreExecuteStripsReservedMetadata}} and
> {{TestS3InitiateMultipartUploadRequest.testPreExecuteStripsReservedMetadata}}:
> the four reserved names must be removed in {{preExecute}} while a custom
> name is retained, and the hsync commit must still record the writer's own
> client id on the key.
> h3. Patch
> [^CR-7-reserve-hsync-metadata-names-and-tolerate-invalid-values.patch],
> against a6b7bdb937109ba2893688b41a89470bc95c88cf. It also applies to master
> at b0aa6475b78, where none of the touched files differ.
> {{OMKeyRequest}} gains two protected helpers: {{stripReservedMetadata}},
> which filters the four OM owned names out of a {{KeyArgs}} metadata list and
> returns the same object when nothing was removed, and {{parseHsyncClientId}},
> which returns null for an absent or unparseable value instead of throwing.
> {{OMKeyCommitRequest.preExecute}} and
> {{S3InitiateMultipartUploadRequest.preExecute}} run the incoming {{KeyArgs}}
> through the strip, so the names are reserved at the request boundary for
> these two request types as well.
> Every reader of the stored writer id goes through the tolerant parse. The two
> recovery commit paths, {{OMKeyCommitRequest.validateAndUpdateCache}} and
> {{OMKeyCommitRequestWithFSO.prepareCommit}}, and
> {{OMRecoverLeaseRequest.doWork}} fail the request with an {{OMException}}
> carrying {{KEY_NOT_FOUND}} when the value does not parse. The two overwrite
> paths look up the open key only when the id parses, and when either the parse
> or the lookup yields nothing they log a warning with the existing
> "Potentially inconsistent DB state" wording, clear the open key name they
> were going to mark and commit as a plain overwrite. In
> {{OMKeyCommitRequestWithFSO.applyKeyCommit}} the deferred cache write for the
> overwritten open key is now guarded by the prepared open key info being
> present.
> There is no version gate. The strip runs in {{preExecute}} before
> replication, and the tolerant readers replace an unchecked failure with an
> error response or a plain overwrite, so a follower applying an already
> replicated request sees no behaviour change for a well formed stored value.
> With the patch the following suites pass: {{TestOMKeyCommitRequest}} (25),
> {{TestOMKeyCommitRequestWithFSO}} (25), {{TestOMRecoverLeaseRequest}} (12),
> {{TestS3InitiateMultipartUploadRequest}} (10) and {{...WithFSO}} (10),
> {{TestS3MultipartUploadCompleteRequest}} (9) and {{...WithFSO}} (9),
> {{TestS3MultipartUploadCommitPartRequest}} (19) and {{...WithFSO}} (19),
> {{TestOMKeyCreateRequest}} (83), {{TestOMKeyCreateRequestWithFSO}} (87, one
> existing skip), {{TestOpenKeyCleanupService}} (14). Checkstyle is clean.
> Affected versions: present at the pinned commit. Earlier releases were not
> checked.
> Found during 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 attached patch is a proposal for review. Generated with
> Specula (Claude Opus 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]
