[ 
https://issues.apache.org/jira/browse/HDDS-16790?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Siyao Meng updated HDDS-16790:
------------------------------
    Attachment: TestBugMC3SplitPurgeLeaksBucketUsage.java

> KeyDeletingService can purge a deleted key in one PurgeKeys request and carry 
> its size in another, leaving the size in the bucket usage for good
> ------------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: HDDS-16790
>                 URL: https://issues.apache.org/jira/browse/HDDS-16790
>             Project: Apache Ozone
>          Issue Type: Bug
>            Reporter: Siyao Meng
>            Priority: Minor
>         Attachments: 
> MC-3-keep-deleted-key-and-its-size-in-one-purge-request.patch, 
> TestBugMC3SplitPurgeLeaksBucketUsage.java
>
>
> h3. Mechanism
> {{KeyDeletingService.submitPurgeKeysRequest}} builds PurgeKeys requests from 
> block groups, one per version of a {{deletedTable}} entry. For every block 
> group it adds the name of the entry to the request, but it adds the size to 
> {{bucketPurgeKeysSize}} only for a committed version. After every block group 
> it compares the request size with 90% of 
> {{ozone.om.ratis.log.appender.queue.byte-limit}} and, when the limit is 
> reached, submits the request and starts a new one. An entry that holds a 
> committed version and a group of uncommitted blocks can therefore be named 
> without its size in one request and with its size in the next.
> {{OMKeyPurgeRequest}} and {{OMKeyPurgeResponse}} delete the entry by name and 
> subtract from {{snapshotUsedBytes}} and {{snapshotUsedNamespace}} of the 
> bucket only what the same request carries. If the first of the two requests 
> is applied and the second is not, the entry is gone and nothing subtracts its 
> size any more.
> The size based split (HDDS-13517) and the per bucket size in the request 
> (HDDS-13764) are both first contained in release 2.1.0 (by history, not run 
> on older code).
> h3. Trigger
> # A key is overwritten by a commit that leaves an allocated block 
> uncommitted, so its {{deletedTable}} entry has the old version and a group of 
> uncommitted blocks.
> # One run of the service collects more than fits into one request. At default 
> limits (32MB, {{ozone.key.deleting.limit.per.task}} 50000) that needs 
> {{deletedTable}} keys of about 600 bytes on average, which means key names or 
> paths of about that length (the average is over every entry of the run, 
> whatever the bucket layout). It needs less when the per task limit is raised 
> or the Ratis limit is lowered.
> # The cut falls between the two block groups of that entry, with the 
> uncommitted blocks first. The two groups are close to each other in iteration 
> order, so only the entries right at a cut are exposed. Which of the two comes 
> first is about even.
> # The next request of the run is not applied.
> For step 4 I ran this case: a snapshot of any bucket is created between the 
> two submits. {{OMKeyPurgeRequest}} then rejects every later request of that 
> run with {{INVALID_REQUEST}}, because the expected previous snapshot has 
> changed. No fault is involved. Not run by me: an OM stop or a leader change 
> between the two submits (argued, the state after the first request is the 
> same). Also argued, not run: the request that carries the size fails with a 
> {{ServiceException}}, which the service catches before it submits the next 
> request, and the request that only names the entry is applied.
> h3. Impact
> * The bucket keeps the size of the purged entry in {{snapshotUsedBytes}} and 
> one key in {{snapshotUsedNamespace}}. In the reproduction the bucket is 
> empty, its deleted table is empty, and both OM and the client still report 
> 805306368 bytes and 1 key in use.
> * The leaked amount counts against the quota. With a namespace quota of 1 the 
> first key written to the empty bucket is rejected with {{QUOTA_EXCEEDED}}.
> * Nothing recomputes the counters on its own. A quota repair started by an 
> admin does (by code reading, not run).
> * No data is lost and the blocks are deleted as intended. One incident leaks 
> the entries that sit at a cut, typically one.
> h3. Reproduction
> PASS on unmodified source. [^TestBugMC3SplitPurgeLeaksBucketUsage.java] 
> ({{hadoop-ozone/ozone-manager}}) drives one real OM through 
> {{OzoneManagerProtocol}}, without datanodes. It overwrites 20 keys so that 
> each leaves one uncommitted block, runs {{KeyDeletingService}} once, and 
> creates a snapshot of an unrelated bucket through the client API right after 
> the first applied request that names an entry without its size. It then shows 
> that the later request for that entry is rejected with {{INVALID_REQUEST}}, 
> that the deleted table is empty, and the leaked usage and the 
> {{QUOTA_EXCEEDED}} error described above. To keep the test small the Ratis 
> limit is 64KB and the key names are 59000 characters long, so every block 
> group fills its own request. A split at default limits (about 30MB of names 
> in one run) was not run. A passing test means the defect is present.
> h3. Patch
> [^MC-3-keep-deleted-key-and-its-size-in-one-purge-request.patch], against 
> a6332fb59879b80615b9233e953d6813d03ae7a8. It also applies to master at 
> 3a22acfaa21.
> {{KeyDeletingService.submitPurgeKeysRequest}} groups the block groups by 
> deleted key name and adds one {{deletedTable}} entry per step, its name once 
> and the sizes of all its committed versions together. The size check stays 
> where it is, so a request can only be cut between entries. An entry that 
> alone exceeds the limit is sent in a request of its own, as before. The 
> request format and the way it is applied do not change. One visible side 
> effect: {{NumKeysPurged}} is counted from the names in the request, so an 
> entry with two reclaimable versions now counts once instead of twice. 
> {{NumKeysSentForPurge}} and the deleted key count of the service still count 
> versions, so the two no longer agree for such entries.
> Covered by {{testPurgeKeysRequestKeepsKeyVersionsTogether}} in the existing 
> {{TestKeyDeletingService}}, which also pins that an entry that is rewritten 
> instead of purged carries its size in the request that updates it. Without 
> the change it fails because the overwritten entry is spread over two requests 
> (four requests instead of three). With it {{TestKeyDeletingService}}, 
> {{TestOMKeyPurgeRequestAndResponse}} and {{TestKeyManagerImpl}} pass (59 
> tests) and checkstyle is clean. {{TestKeyPurging}} and 
> {{TestSnapshotDeletingServiceIntegrationTest}} on a mini cluster (9 tests) 
> pass in a run that had this patch and a second, unrelated change to 
> {{OMKeyPurgeRequest}} applied together. With the patch the reproduction fails 
> at its first assertion, because no request names an entry without its size 
> any more. Buckets that already leaked are not repaired by the patch.
> Found by TLA+ model checking and code review of the OM key deletion service 
> and key purge paths under HDDS-15926, on commit 
> a6332fb59879b80615b9233e953d6813d03ae7a8. Checked against HDDS issues and 
> apache/ozone pull requests for duplicates before filing. Related but not 
> duplicates: HDDS-13517 (introduced the size based split of PurgeKeys 
> requests), HDDS-13764 (introduced the per bucket size in the request), 
> HDDS-16184 (quota repair of the same counters). 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