[
https://issues.apache.org/jira/browse/HDDS-16814?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Krishna Kumar Asawa reassigned HDDS-16814:
------------------------------------------
Assignee: Gargi Jaiswal
> DiskBalancer move drops the in memory data checksum of the moved container
> replica, so the datanode reports data checksum 0 for it
> ----------------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-16814
> URL: https://issues.apache.org/jira/browse/HDDS-16814
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Assignee: Gargi Jaiswal
> Priority: Minor
> Attachments:
> CR-6-load-data-checksum-of-container-moved-by-diskbalancer.patch,
> TestBugCR6DiskBalancerMoveDataChecksum.java
>
>
> h3. Mechanism
> {{DiskBalancerService.DiskBalancerTask}} copies a container to the
> destination volume and imports the copy with
> {{KeyValueHandler.importContainer(ContainerData)}}. That method creates the
> new {{KeyValueContainerData}} with the copy constructor, which leaves the
> state {{OPEN}}, and calls
> {{KeyValueContainer.importContainerData(KeyValueContainerData)}}. The import
> runs {{KeyValueContainerUtil.parseKVContainerData}} first and restores the
> original state ({{CLOSED}} or {{QUASI_CLOSED}}) only afterwards. During the
> parse the container looks open, and {{loadAndSetContainerDataChecksum}}
> returns at once for an open container. The new replica is put into the
> {{ContainerSet}} without a data checksum in memory, while its merkle tree
> file and its RocksDB metadata on the destination volume carry the value of
> the source.
> The container report is built from the in memory value
> ({{KeyValueContainerData.buildContainerReplicaProto}}), so the replica is
> reported with data checksum 0. The move sends no incremental container
> report, so SCM and Recon receive the 0 with the next full container report
> ({{hdds.container.report.interval}}, default 60m). The datanode side was run,
> the SCM and Recon side is read from the code.
> Only the DiskBalancer move is affected. The import of a replicated container
> goes through the same {{importContainerData}}, but
> {{TarContainerPacker.unpackContainerData}} sets the new container to
> {{RECOVERING}} before the parse
> ({{ContainerPacker.persistCustomContainerState}}), so the checksum is loaded.
> The existing {{TestKeyValueContainer.testContainerImportExport}} asserts that
> and passes on the unmodified source (run). EC reconstruction and the
> container load at datanode start do not use this import.
> The open state also makes the parse skip the empty check
> ({{noBlocksInContainer}} returns false for an open container), so the import
> never marks a moved replica as empty. DiskBalancer does not pick containers
> with zero used bytes, so this is not expected to be reachable (read from the
> code, not run).
> The parse was moved in front of the state change by HDDS-12233, the early
> return for open containers is from HDDS-12745, and DiskBalancer imports this
> way since HDDS-12439. 2.2.0 and 2.2.1 contain all three with the same code
> (read from the tags, not run there). 2.1.x has no DiskBalancer. The nearest
> existing issue is HDDS-13023, which fixed the container file checksum
> verification that the same reordering broke for this import, and did not
> cover the data checksum.
> h3. Trigger
> # A {{CLOSED}} or {{QUASI_CLOSED}} container replica (the states DiskBalancer
> moves by default, {{hdds.datanode.disk.balancer.container.states}}) has a
> data checksum (from a data scan, from reconciliation, or built from metadata
> at close).
> # An administrator starts DiskBalancer on the datanode, and it moves the
> container to another volume.
> No concurrency and no failure is needed. Every move of a container that has a
> data checksum ends this way.
> h3. Impact
> * From the next full container report on, the moved replica is reported with
> data checksum 0 although its data did not change. {{ozone admin container
> reconcile --status}} then shows the replicas of the container as not matching
> ({{ReconcileSubcommand}}), and Recon lists the container as REPLICA_MISMATCH
> ({{ReconReplicationManager.hasDataChecksumMismatch}}). Both compare the raw
> values and do not treat 0 as unknown (read from the code, not run).
> * After DiskBalancer has balanced a datanode this holds for every container
> it moved.
> * The value comes back at a datanode restart (run at the level of the load
> call, see Reproduction), at the next data scan of the replica (background or
> on demand; the background scanner runs with
> {{hdds.container.scrub.data.scan.interval}} default 7d and is throttled to 5
> MB/s per volume by default), when the replica is reconciled with a peer, or
> when a moved {{QUASI_CLOSED}} replica is closed (the last three read from the
> code, not run).
> * No data is affected, and peers that reconcile with the replica read its
> merkle tree file, which is correct. SCM takes no automatic action on the data
> checksum today. The effect is a wrong mismatch signal for administrators and
> for Recon.
> h3. Reproduction
> PASS, deterministic, unmodified source.
> [^TestBugCR6DiskBalancerMoveDataChecksum.java]
> ({{hadoop-hdds/container-service}}) uses the setup of
> {{TestDiskBalancerTask}}: a real {{KeyValueHandler}}, a real container on
> disk, two volumes and the real {{DiskBalancerTask}}, for every schema version
> of {{ContainerTestVersionInfo}}. There is no SCM and no Recon. A {{CLOSED}}
> container with a block row and a merkle tree is loaded from disk as at
> datanode start and then moved. The test asserts that the tree file and
> RocksDB of the moved replica carry the checksum, that the replica in the
> {{ContainerSet}} has none and its container report has data checksum 0, that
> every other field of the report is equal to the report before the move, that
> no incremental container report was sent, and that parsing the moved replica
> from disk again (the call {{ContainerReader}} makes at datanode start)
> returns the checksum. A passing test means the defect is present.
> h3. Patch
> [^CR-6-load-data-checksum-of-container-moved-by-diskbalancer.patch], against
> 1e528b97aed3eef1be6846f1f0767c1ef98a883a. It also applies to master at
> 8ca44914771.
> {{KeyValueHandler.importContainer(ContainerData)}}, which only DiskBalancer
> calls, sets the new container object to {{RECOVERING}} before it calls
> {{importContainerData}}. That is the state {{DiskBalancerTask}} has already
> written to the container file of the copy, and the state the new object has
> during the parse in a replication import. {{importContainerData}} and the
> replication import are not changed. The state is set in memory only, on an
> object that is not yet in the {{ContainerSet}}. The container file is written
> as before, once, at the end of {{importContainerData}} and with the original
> state, so the copy is not left {{RECOVERING}} on disk after a successful move
> (the regression test reads the file back), and an import that fails half way
> leaves on disk what it left before the change. For a move the new replica
> gets the data checksum of the copy, and a copy without blocks is marked empty
> as it would be at datanode start.
> The move still sends no incremental container report. With the checksum
> loaded, the report of the moved replica is equal to the report before the
> move except for the in memory read and write counters, which start again from
> 0 as after a datanode restart, so there is nothing new for SCM.
> This change should go in after or together with HDDS-16807: in two of the
> orders described there, the incremental container report that the scan at the
> end of the reconciliation sends for the live replica is sent only because the
> moved replica has no data checksum in memory, so with this change alone SCM
> keeps the report built from the retired replica until the next full container
> report (run with the reproduction attached to HDDS-16807).
> Covered by {{moveKeepsContainerReport}} and
> {{moveOfContainerWithoutDataChecksum}} in the existing
> {{TestDiskBalancerTask}}. The first asserts that the whole container report
> of a replica that has seen no I/O is the same before and after the move, and
> that the container file of the copy is {{CLOSED}}; without the change it
> fails with {{dataChecksum: 0}} instead of {{dataChecksum: 1395005929}} as the
> only difference. The second moves a container without blocks and without a
> merkle tree and asserts that it still has no data checksum after the move, so
> a later scan can set it, and that it is marked empty; without the change it
> fails on the empty flag. With the change the unit suites of the {{keyvalue}},
> {{diskbalancer}}, {{replication}} and {{ozoneimpl}} packages in
> {{hadoop-hdds/container-service}} pass (1485 tests, 18 skipped by the
> suites), checkstyle is clean, and the reproduction fails on its first
> assertion about the missing checksum. Not run on a mini cluster.
> Found by code review of the container reconciliation paths in the datanode
> and SCM, as part of the TLA+ verification effort under HDDS-15926, on commit
> 1e528b97aed3eef1be6846f1f0767c1ef98a883a. 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]
