[jira] [Resolved] (HDDS-17282) KeyDeletingService reclaims a deleted key record whose earlier version group the previous snapshot still references

["Siyao Meng (Jira)" <ji...@apache.org>]

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

Siyao Meng resolved HDDS-17282.
-------------------------------
    Resolution: Not A Problem

> KeyDeletingService reclaims a deleted key record whose earlier version group the previous snapshot still references
> -------------------------------------------------------------------------------------------------------------------
>
>                 Key: HDDS-17282
>                 URL: https://issues.apache.org/jira/browse/HDDS-17282
>             Project: Apache Ozone
>          Issue Type: Bug
>    Affects Versions: 1.4.0, 2.0.0, 1.4.1, 2.1.0, 2.2.0, 2.1.1, 2.1.2, 2.2.1
>            Reporter: Siyao Meng
>            Priority: Major
>         Attachments: CR-9-keep-deleted-versioned-key-held-by-snapshot.patch
>
>
> h3. Mechanism
> A key record can hold several location version groups. Records written while the bucket's {{isVersionEnabled}} flag is true do ({{OMKeyRequest.prepareFileInfo}} calls {{OmKeyInfo.addNewVersion}} with keepOldVersions set), and such a record keeps its groups if the flag is turned off later. {{OMKeyDeleteRequest}} moves the whole record, with every group, to {{deletedTable}}.
> {{ReclaimableKeyFilter.getPreviousSnapshotKeyInfo}} keeps a deleted record only if the previous snapshot's copy has the same object ID and {{SnapshotUtils.isBlockLocationInfoSame}} is true. That check returns false as soon as the two records have a different number of version groups. So for this state:
> * a {{deletedTable}} entry for object X holds the groups [v0, v1],
> * the previous snapshot's copy of X holds [v0], with the same version number and the same blocks,
> the filter returns the entry as reclaimable, and {{KeyManagerImpl.getPendingDeletionKeys}} hands the blocks of every group to {{KeyDeletingService}}, including those of v0. By code reading the same happens when the previous snapshot holds [v0, v1] and is purged while the next older snapshot holds [v0]. The same filter is used for snapshot deep cleaning, {{DirectoryDeletingService}} and {{BucketDeletedDataCalculator}}. A record written while the flag is false has one version group and is not affected.
> The version count check was added with the block comparison in HDDS-8686 (1.4.0). Before that, a matching object ID was enough to keep the record.
> HDDS-10051 was resolved as Won't Fix because object versioning is not intended to coexist with Ozone snapshots, and its resolving comment says that validation rejecting the combination should be tracked separately. HDDS-16060 proposes that exclusion for the new S3 versioning status, and also that reclamation fail closed when OM finds a pre existing mixed state. OM does not reject a snapshot of a bucket whose {{isVersionEnabled}} flag is true, and the pull request for HDDS-16060 checks only the new versioning status. In that state the filter deletes blocks that the snapshot still lists, which is data loss and not only an accounting error. This report does not ask to support the combination. The patch applies the fail closed rule that HDDS-16060 describes to the records that exist today.
> h3. Impact
> * The blocks of v0 are sent to SCM for deletion while the previous snapshot still lists them, so reads of X through that snapshot lose their data once the datanodes delete the blocks. Run on the OM side: the blocks are selected for deletion and sent to a testing SCM. Not run: SCM and datanode deletion and the failed snapshot read.
> h3. Reproduction
> The regression test in the patch, {{TestKeyDeletingService.testOverwrittenKeyBlocksReferencedBySnapshotNotReclaimed}}, builds this state on one OM (deleted record [b0, b1], snapshot copy [b0]) with a testing SCM and evaluates the filter for the active object store. On the unmodified source the versioned case fails because both blocks are reclaimable while the snapshot exists:
> {noformat}
> TestKeyDeletingService$Normal.testOverwrittenKeyBlocksReferencedBySnapshotNotReclaimed:567
> Expecting
>   [945169052L, 945169400L]
> not to contain
>   [945169052L]
> but found
>   [945169052L]
> {noformat}
> The case with one version group per record passes with and without the change (the old record is a separate entry that matches the snapshot's copy and is kept, the newer one is reclaimable). Block IDs are time based and differ per run.
> h3. Patch
> [^CR-9-keep-deleted-versioned-key-held-by-snapshot.patch], against b10a7d427bdf83807a7e48a1be507d3b02d5a503. It also applies to master at 26388a80b37.
> {{ReclaimableKeyFilter.getPreviousSnapshotKeyInfo}} now also treats the record as present in the previous snapshot when it still holds one of the non empty version groups of the snapshot's copy (same version number, same blocks). The entry is then kept as a whole until no previous snapshot holds such a group, after which it is reclaimed as before. This fails closed: the newer groups of the entry wait for the snapshot as well, because the filter decides per key record and not per version group. For a record with one version group the decision is unchanged. {{SnapshotUtils.isBlockLocationInfoSame}} is not changed, because snapshot diff uses it to report a modified key. The same lookup feeds the exclusive size: for such an entry the previous snapshot is now credited with the size of its copy, which is still a whole key approximation.
> With the change the regression test passes, and after the snapshot is deleted both blocks are reclaimed. The key deleting, directory deleting and snapshot deleting service suites, the reclaimable filter suites, {{TestSnapshotUtils}}, {{TestSnapshotDiffManager}} and {{TestBucketDeletedDataCalculator}} pass, and on a mini cluster {{TestSnapshotDeletingServiceIntegrationTest}} and {{TestKeyPurging}} pass. Checkstyle is clean. Blocks already deleted before the patch are not recovered.
> Found by code review of the S3 gateway object put and key commit paths, as part of the TLA+ verification effort under HDDS-15926, on commit b10a7d427bdf83807a7e48a1be507d3b02d5a503. 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.5).



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org