Siyao Meng created HDDS-16852:
---------------------------------
Summary: 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
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]
