[
https://issues.apache.org/jira/browse/HDDS-17044?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17044:
------------------------------
Attachment: TestBugMC3DirRenameEventOrder.java
> Recon unlinks a renamed FSO directory in the namespace summary when the
> rename's PUT event is processed before its DELETE
> -------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-17044
> URL: https://issues.apache.org/jira/browse/HDDS-17044
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Major
> Attachments: MC-3-keep-renamed-directory-linked-in-nssummary.patch,
> TestBugMC3DirRenameEventOrder.java
>
>
> h3. Mechanism
> OM writes an FSO directory rename as a DELETE of the old {{directoryTable}}
> key and a PUT of the new key in the same write batch
> ({{OMKeyRenameResponseWithFSO.addToDBBatch}}). {{RDBBatchOperation}} keeps
> the pending operations of each column family in a {{HashMap}} keyed by the
> key bytes, so the two records reach the RocksDB WAL, and Recon, in key hash
> order rather than in call order. Recon passes them through
> {{OMDBUpdatesHandler}} unchanged, so {{NSSummaryTaskWithFSO}} sees a PUT of
> the new key and a DELETE of the old key, in either order.
> When the PUT comes first, {{NSSummaryTaskDbEventHandler.handlePutDirEvent}}
> links the directory to its new parent and adds its totals there.
> {{handleDeleteDirEvent}} then subtracts the totals from the old parent, which
> is correct, but also sets the directory's {{parentId}} to 0 unconditionally.
> For a rename within the same parent it also removes the directory from that
> parent's child set. The directory is live in OM but unlinked in the namespace
> summary.
> h3. Trigger
> An ordinary rename of a directory in an FSO bucket, for example {{ozone fs
> -mv /A/D /C/D1}} or {{/A/D /A/D1}}. Which record comes first depends only on
> the hash of the two key byte strings, so it is deterministic per name and
> needs no timing or fault. In my run 12 of 18 renames (9 names, each to the
> same and to a different parent) were processed PUT first, and all 12 were
> corrupted. The 6 processed DELETE first were correct.
> h3. Impact
> * Writes under the renamed directory stop rolling up: after writing a 50 byte
> file into the renamed directory, the bucket total stayed at 100 bytes instead
> of 150.
> * A later recursive delete of the directory subtracts the full total from the
> ancestors: bucket and parent ended at 1 file and 50 bytes below zero
> ({{files=-1 size=-50}}) instead of 0.
> * Paths built from the summary lose the ancestors
> ({{ReconUtils.constructFullPath}} returned {{vol/fso/D1/f2}} instead of
> {{vol/fso/C/D1/f2}}), which the container key listing and the OM DB insight
> endpoints use.
> * The state is kept by every later incremental sync and is only corrected by
> a full NSSummary rebuild. OM data is not affected.
> * Since when: the unconditional unlink came with HDDS-13432 (2.1.0). The
> 2.1.x releases still wrote batch operations in call order (HDDS-13415, later
> reverted by HDDS-14240), so the DELETE of a rename came first there; 2.2.0
> and later write them in hash order and are affected as described. Since
> HDDS-8128 (1.4.0) releases up to 2.0.x already lost the child link of a
> directory renamed within the same parent when the PUT came first.
> h3. Reproduction
> Deterministic, unmodified source. [^TestBugMC3DirRenameEventOrder.java]
> ({{hadoop-ozone/recon}}) builds a real OM DB with {{/A/D/f1}} (100 bytes) and
> an empty {{/C}}, bootstraps Recon from it, and for each rename commits the
> real {{OMKeyRenameResponseWithFSO}} through one OM store batch, reads the
> delta with {{RDBStore.getUpdatesSince}}, runs it through
> {{OMDBUpdatesHandler}} and applies it to the Recon OM DB as
> {{OzoneManagerServiceProviderImpl}} does, then runs
> {{NSSummaryTaskWithFSO.processWithFSO}}. It then writes a 50 byte file into
> the renamed directory and deletes the directory recursively the same way. On
> the unfixed code it fails and lists the 12 corrupted renames, for example
> {{mv /A/D /C/D1 (PUT first): D.parentId=0 (expect 2003), bucket size after
> write=100 (expect 150), path=vol/fso/D1/f2 (expect vol/fso/C/D1/f2), bucket
> after rm -r: files=-1 size=-50 (expect 0/0)}}. With the patch all 18 renames
> are correct.
> h3. Patch
> [^MC-3-keep-renamed-directory-linked-in-nssummary.patch], against
> 0665932dd35da4d05087e02748af0a64eb846ad5. It also applies to master at
> 2264d4d48ac.
> {{handleDeleteDirEvent}} applies the unlink only when the directory's stored
> summary still describes the deleted key (same {{parentId}} and same name).
> Otherwise the PUT for the new key was already processed: the old parent still
> loses the totals, and the child link unless the new key is under the same
> parent, but the directory stays linked. A real delete, an update of the same
> key and a rename processed DELETE first behave as before. This makes the
> handler independent of the order of the two records and works with any OM
> version, without touching the shared {{RDBBatchOperation}}.
> Covered by {{testProcessDirRenameInEitherEventOrder}} (four cases: same or
> different parent, either order) in the existing {{TestNSSummaryTaskWithFSO}},
> and {{testProcessDirDeleteRename}} now also checks that a deleted directory
> is still unlinked. Without the change both PUT first cases fail on the
> directory's {{parentId}} (0). With it the Recon NSSummary, endpoint and event
> handler suites pass and checkstyle is clean.
> Not covered: two directory renames inside one OM flush that swap nesting (for
> example {{mv /P/Q /Q}} then {{mv /P /Q/P}}). The batch also loses the order
> between the two transactions, and correct totals then depend on that order.
> Without the patch the swap left a directory unlinked or the totals wrong in 8
> of the 12 name pairs I ran. With the patch the parent links are right in all
> of them, and 3 of the 12 are still wrong, all 3 also wrong without the patch.
> In 2 the totals are off because P's totals still included Q when P was moved.
> In 1 the second rename's PUT is processed before both records of the first,
> which makes P and Q each other's parent for a moment, so
> {{propagateSizeUpwards}} overflows the stack, as it does without the patch;
> the task then fails and Recon rebuilds the summary. Fixing these needs
> ordered events (call order in the OM batch, or batch boundaries in the events
> Recon passes to its tasks).
> Found by TLA+ model checking and code review of the Recon namespace summary
> and task sync paths under HDDS-15926, on commit
> 0665932dd35da4d05087e02748af0a64eb846ad5. 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]
