[
https://issues.apache.org/jira/browse/HDDS-17282?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18129884#comment-18129884
]
Siyao Meng commented on HDDS-17282:
-----------------------------------
Closing as Not A Problem. This needs the legacy bucket versioning flag together
with snapshots. As discussed on HDDS-16127, no code path exercises the multi
version key path today, and the new object versioning in HDDS-15728 will not
use multiple version location groups; HDDS-10051 also notes that versioning is
not meant to coexist with snapshots. The legacy path is to be handled together
with HDDS-15728.
> 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: [email protected]
For additional commands, e-mail: [email protected]
