Siyao Meng created HDDS-16814:
---------------------------------
Summary: 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
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]
