Siyao Meng created HDDS-17250:
---------------------------------

             Summary: Key commit does not check that an object store open key 
belongs to the current bucket of that name
                 Key: HDDS-17250
                 URL: https://issues.apache.org/jira/browse/HDDS-17250
             Project: Apache Ozone
          Issue Type: Bug
            Reporter: Siyao Meng
         Attachments: CR-1-reject-commit-of-key-opened-in-earlier-bucket.patch

h3. Mechanism
For OBJECT_STORE and LEGACY buckets the open key table is keyed by 
{{/volume/bucket/key/clientID}}, with no bucket objectID in the key. Bucket 
delete checks the key table ({{OmMetadataManagerImpl.isBucketEmpty}}), 
incomplete multipart uploads and snapshots, but not the open key table, so an 
open key entry can remain after its bucket is deleted. 
{{OMKeyCommitRequest.validateAndUpdateCache}} finds the bucket and the open key 
by name only. Once a bucket with the same name exists again, the commit of that 
entry succeeds and the key is written into the later bucket.

FSO buckets are not affected: their open key path contains the bucket objectID, 
so the entry is not found under a later bucket.

h3. Trigger
An open key of an OBJECT_STORE or LEGACY bucket is committed after the bucket 
was deleted and a bucket with the same name was created in the same volume.

h3. Impact
Reproduced on unmodified source through the request classes, with the open key 
entry placed and the bucket table cache entry replaced directly before the 
commit:
* The commit returns OK and the key appears in the later bucket.
* The key's size and namespace are charged to the later bucket ({{usedBytes}} 0 
to 600 with three 200 byte blocks, {{usedNamespace}} 0 to 1).

h3. Reproduction
No standalone reproduction is attached. 
{{testValidateAndUpdateCacheKeyOpenedInEarlierBucket}} in the patch runs the 
real {{OMKeyCommitRequest}} {{preExecute}} and {{validateAndUpdateCache}} on an 
OBJECT_STORE bucket against a real {{OmMetadataManagerImpl}}, with object IDs 
in creation order as the OM assigns them, and fails on unmodified source with 
"expected: <KEY_NOT_FOUND> but was: <OK>".

h3. Patch
[^CR-1-reject-commit-of-key-opened-in-earlier-bucket.patch], against 
20d95ed37fe35013b5320713665db03c06ee36fa. It also applies to master at 
64d4a791502.

Object IDs come from the Ratis transaction index 
({{OmUtils.getObjectIdFromTxId}}), so a key opened in a bucket always has a 
larger objectID than that bucket. {{OMKeyCommitRequest.validateAndUpdateCache}} 
now rejects the commit with {{KEY_NOT_FOUND}} when the open key's objectID is 
below the bucket's ({{Long.compareUnsigned}}, as the epoch sets the top bit), 
next to the existing rejections of deleted or overwritten hsync keys. The same 
apply path serves hsync commits on OBJECT_STORE and LEGACY buckets; 
{{OMKeyCommitRequestWithFSO}} is unchanged, since FSO open keys are keyed by 
bucket objectID. An overwrite keeps the objectID of the key it replaces, so the 
check is skipped when the key table already holds a key with the same objectID; 
keys that were committed this way before the patch therefore stay overwritable. 
No request or proto field is added. Alternatives not taken: an open key field 
naming its bucket (needs a layout gate), or making bucket delete refuse while 
open keys exist, as it does for incomplete multipart uploads since HDDS-9318 
(blocks deletes until the open keys expire).

The check changes what an OM applies for such a commit. With the current 
prepare based upgrade, OMs of different versions do not apply the same entries; 
under zero downtime upgrade it would need a version flag once that framework is 
used in OM. The check assumes the transaction index never goes back below 
existing object IDs, which object ID uniqueness already assumes; {{ozone repair 
om update-transaction}} can set a lower index, and until the index passes the 
old values new keys in buckets created above it would be rejected at commit. A 
rejected open key stays in the open key table until {{OpenKeyCleanupService}} 
expires it, as for any other abandoned key.

The commit test fixtures now give open keys an objectID above their bucket's, 
as the OM does; before, they used 0 while fixture buckets use the current time. 
Covered by three tests in the existing {{TestOMKeyCommitRequest}}: 
{{testValidateAndUpdateCacheKeyOpenedInEarlierBucket}}, 
{{testValidateAndUpdateCacheOpenKeyObjectIDBelowBucketAndExistingKey}} (the 
existing key's objectID differs from the open key's, so the overwrite exception 
does not apply and the existing key stays unchanged) and 
{{testValidateAndUpdateCacheOverwriteOfKeyWithLowerObjectID}}. Without the 
change the first two fail with "expected: <KEY_NOT_FOUND> but was: <OK>"; with 
the overwrite exception reduced to "no existing key" the second fails the same 
way; without the overwrite exception the third fails with "expected: <OK> but 
was: <KEY_NOT_FOUND>". With it the key request suites pass (448 tests, 
including allocate block, open key delete, lease recovery and S3 commit part), 
as do {{TestHSync}} and {{TestLeaseRecovery}} (57 tests), and checkstyle is 
clean.

Found by code review of the OM volume, bucket and snapshot lifecycle paths, as 
part of the TLA+ verification effort under HDDS-15926, on commit 
20d95ed37fe35013b5320713665db03c06ee36fa. Checked against HDDS issues and 
apache/ozone pull requests for duplicates before filing. Related but not 
duplicates: HDDS-6682 (checks made in {{preExecute}} that are stale at apply; 
this entry is an open key that outlives its bucket, not one request, and its 
proposed fix (closed PR #3508) records the bucket at the commit's own 
{{preExecute}}, which already sees the later bucket), HDDS-10650 (reported that 
volume, bucket and directory deletes leave open key table entries, for hsync 
files in FSO buckets; its fix covers recursive directory delete only) and 
HDDS-9318 (bucket delete with incomplete multipart uploads). 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