Siyao Meng created HDDS-16810:
---------------------------------
Summary: Reconciliation raises the container BCSID to a block
BCSID taken from a peer whose own container BCSID is lower
Key: HDDS-16810
URL: https://issues.apache.org/jira/browse/HDDS-16810
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
Attachments:
MC-1-cap-container-bcsid-advance-at-peer-container-bcsid.patch,
TestBugMC1ContainerBcsIdFromIncompletePeer.java
h3. Mechanism
After each peer round {{KeyValueHandler.reconcileContainerInternal}} calls
{{advanceContainerBcsIdForFullyCoveredRound}}. When every block repaired in the
round ended complete and the diff dropped no chunk as unhealthy, the container
BCSID is raised to the highest block BCSID adopted in the round. The Javadoc of
the method reasons that such a round inherits the peer's own container level
claim, but the peer's container BCSID is never read, and a block BCSID on the
peer says nothing about the peer's other blocks:
* A peer that was itself only partially repaired holds complete blocks above
its own container BCSID. It correctly did not raise its container BCSID, but it
serves those blocks.
* A block that both replicas hold in the same incomplete form is not in the
diff, so the round counts as clean.
A replica that reconciles with such a peer ends with a container BCSID that
covers a committed chunk it does not hold. Nothing on the datanode corrects
this afterwards: the scanner checks a block against its own metadata, which
lists only the chunks the replica has (run).
The logic came with HDDS-15542 and is on master only. Releases 2.1 and 2.2
raise the container BCSID after every fully pulled block, which HDDS-15542
replaced (read from the release code, not run). HDDS-16011 states the rule that
is broken: a replica that advertises BCSID N holds every chunk committed
through N.
h3. Trigger
In the reproduced schedules the container is QUASI_CLOSED in SCM and its
replicas are QUASI_CLOSED or UNHEALTHY, and reconciliation is started with
{{ozone admin container reconcile}}. Layout of the reproduction: two blocks of
two chunks, block 0 committed at BCSID 1 and 2, block 1 at BCSID 3 and 4. d1
applied every commit.
Schedule A, no RPC failure:
# d1 has a corrupt second chunk in block 0, its scanner has found it and marked
d1 UNHEALTHY. d2 and d3 are QUASI_CLOSED and empty.
# d2 reconciles. It gets block 1 and the first chunk of block 0, and keeps
container BCSID 0.
# d3 reconciles and its first peer is d2. The round is clean, d3 adopts block 1
at BCSID 4 and raises its container BCSID to 4. The round with d1 cannot add
the corrupt chunk.
SCM sends the peers as a set, so their order is not controlled. SCM also sends
the command to every replica at the same time, so whether d2 has finished its
round with d1 before d3 starts its round with d2 depends on which datanode runs
its command first. With d1 first in d3's list, d3 stays at 0 (the control in
the reproduction).
Schedule B, found by model checking, all three replicas QUASI_CLOSED and d1
healthy:
# d2 is empty, d3 applied only the first commit.
# d2 reconciles and its GetBlock for block 0 to d1 fails. d2 gets block 1 from
d1 and the short block 0 from d3, and ends at container BCSID 1.
# d3 reconciles while d1 is unreachable. It gets block 1 from d2, the round is
clean, and its container BCSID goes from 1 to 4.
h3. Impact
* d3 is QUASI_CLOSED with container BCSID 4, the same as d1, and holds one of
the two chunks of block 0 (run, both schedules).
* SCM treats the two as equal. {{QuasiClosedContainerHandler}} force closes
every QUASI_CLOSED replica with the highest sequence ID, and the datanode
accepts the close for d3 (the datanode side is run in schedule B, the SCM
decision is read from code). {{RatisUnderReplicationHandler}} keeps only
replicas with the highest sequence ID as sources, so d3 can be the copy source
when d2 is replaced (read from code, not run). The same value on a QUASI_CLOSED
replica of a CLOSED container makes {{MismatchedReplicasHandler}} force close
it (read from code, not run).
* In schedule A the chunk is already damaged on d1, the only replica that had
it. What the defect changes there is visibility:
{{QuasiClosedContainerHandler}} keeps a container QUASI_CLOSED while an
UNHEALTHY replica has a higher sequence ID than every QUASI_CLOSED one, and d3
at BCSID 4 lifts that hold (read from code, not run).
* A client read of block 0 at its committed BCSID is still rejected on d3 by
the block level check in {{BlockManagerImpl.getBlock}}, so the client fails
over (read from code, not run). The container level check is the one that no
longer protects.
* A later reconcile of d3 against a complete healthy replica would repair the
block (read from code, not run), but only an administrator starts one.
h3. Reproduction
PASS, deterministic, unmodified source.
[^TestBugMC1ContainerBcsIdFromIncompletePeer.java]
({{hadoop-hdds/container-service}}) uses the setup of
{{TestContainerReconciliationWithMockDatanodes}}: three datanodes, each a real
{{KeyValueHandler}} with a real container on disk and a real on demand scanner,
the real {{BlockInputStream}}, and only the static {{ContainerProtocolCalls}}
transport routed to the peer, which answers the request for its checksum info
through its own {{KeyValueHandler}}. There is no SCM. Each replica is created
as the prefix of the commit log that it applied and is then quasi closed.
Schedule B uses two switches in that routing (one GetBlock fails, one datanode
is unreachable). The tests assert the data and the BCSID of d2 and d3 after
each step, the control order for schedule A, and for schedule B that
{{closeContainer}} leaves d3 CLOSED at BCSID 4 with the chunk missing. A
passing test means the defect is present.
h3. Patch
[^MC-1-cap-container-bcsid-advance-at-peer-container-bcsid.patch], against
1e528b97aed3eef1be6846f1f0767c1ef98a883a. It also applies to master at
8ca44914771.
{{GetContainerChecksumInfoResponseProto}} gets an optional field with the
container BCSID of the replica that the checksum info comes from. The serving
datanode reads it before it reads the tree. The reconciling datanode keeps the
conditions from HDDS-15542 for a clean round and raises its container BCSID to
the highest adopted block BCSID as before, but never above the value the peer
reported. The field is on a datanode to datanode read response, nothing is
persisted, and the proto compatibility check of the build passes without a
change to {{proto.lock}}. The same field is the input for the rule proposed in
HDDS-16011, raising the container BCSID to the higher of the two when the trees
are equal.
A peer that does not send the field is handled as today. During a rolling
upgrade a round against a datanode without the patch can therefore still raise
the BCSID too far, and the defect is closed once both datanodes run the patch.
The other choice, never raising against such a peer, would leave replicas that
were repaired completely below the sequence ID of the container for good,
because a later reconcile finds an empty diff and adopts nothing.
Limits:
* When a partially repaired peer precedes a complete one in the list, the
replica ends complete but with the BCSID of the blocks adopted from the
complete peer only, which can be lower than that peer's container BCSID. Such a
replica rejects reads of the blocks above its container BCSID, is not force
closed, and after the container closes it is replaced and deleted as a stale
copy (read from code; the resulting BCSID is run). A later reconcile does not
raise it because the diff is then empty. HDDS-16011 already describes this
case, a replica with complete content that claims a lower BCSID, and today's
code already produces it for a replica whose own pull failed once (run).
Adopting the peer's BCSID when the trees are equal is left to HDDS-16011.
* If the only replica at the full BCSID is UNHEALTHY, that limit can leave
every other replica complete and below it, and the container then stays
QUASI_CLOSED (the hold in SCM is read from code). This needs no failed call. It
needs a peer that holds blocks above its own container BCSID, which is also
what the defect needs. Run at the datanodes: d1 is UNHEALTHY at 4 with a
corrupt second chunk in block 0, d3 is at 2 and holds block 0 intact, d2 is
empty. d2 reconciles first and ends complete at 2, with block 1 at BCSID 4. d3
then reconciles with d2 before d1 and ends complete at 2, and further
reconciles of all three change no BCSID (d1 repairs its chunk and stays
UNHEALTHY at 4). Without the patch d3 reaches 4 there, through the round that
is the defect described here, and SCM can then close the container on d3 (read
from code). With the patch the container stays QUASI_CLOSED until a replica can
adopt the container BCSID of a peer whose tree equals its own, as HDDS-16011
proposes. When d3 reconciles before d2 has pulled block 1, it reaches 4 from d1
with and without the patch.
* The patch only ever lowers the value compared with today, so it adds no way
to claim too much. It does not close the case where the peer's tree is older
than the BCSID sent with it: a reconciling peer raises its container BCSID
before it writes its tree (argued, not run). HDDS-12986 is the open issue for a
new BCSID paired with an older tree.
Covered in the existing {{TestContainerReconciliationWithMockDatanodes}} by
{{testContainerBcsIdNotAdoptedAbovePeerContainerBcsId}} (a complete peer, whose
BCSID is still adopted, and schedule A),
{{testContainerBcsIdNotAdoptedAfterPeerFailureAndOutage}} (schedule B) and
{{testContainerBcsIdAdoptedFromPeerThatDoesNotReportContainerBcsId}}. The first
two use the layout of that suite, not that of the reproduction: replicas that
start CLOSED, and three blocks that are each committed once, at BCSID 0, 1 and
2. In schedule A the corrupt chunk is the first one of block 0, so the
partially repaired peer holds none of block 0. The mock datanodes of that class
now answer the checksum info request through the real handler. With the cap
taken out and the rest of the patch in place, two of the four invocations fail,
the schedule A case of the first test and the second test, because the
reconciling replica reports BCSID 2 while its peer is at 0. With the patch the
unit suites of the {{keyvalue}}, {{ozoneimpl}} and {{checksum}} packages in
{{hadoop-hdds/container-service}} pass (1127 tests, 18 skipped by the suites),
checkstyle is clean on the three touched modules, and both schedules of the
reproduction fail because d3 keeps its own container BCSID. Not run on a mini
cluster, and not run against a datanode of an older version.
Found by TLA+ model checking and code review of the container reconciliation
paths in the datanode and SCM 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]
