Skip to content

HDDS-10374. Make container scanner generate merkle trees during the scan #7490

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 64 commits into
base: HDDS-10239-container-reconciliation
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
64 commits
Select commit Hold shift + click to select a range
a8b8dbc
Merge branch 'HDDS-10239-container-reconciliation' into HDDS-10374-sc…
errose28 Nov 22, 2024
999a913
Add code to build and write the tree from the data scanners
errose28 Nov 22, 2024
b0d1ba9
Update todo in acceptance test
errose28 Nov 25, 2024
382bce2
Add unit tests for tree generation by scanners based on container state
errose28 Nov 25, 2024
28b1889
Add initial (failing) unit test for KeyValueContaienrCheck
errose28 Nov 26, 2024
dc182e8
Update container data checksum when building the tree
errose28 Nov 26, 2024
a3401a9
Fix handling of fully truncated block of 0 size
errose28 Jan 7, 2025
a25d44d
Add unit tests for new addBlock method in tree
errose28 Jan 7, 2025
7550a3c
Test that SCM gets a checksum with the container report
errose28 Jan 7, 2025
847f8d8
Add (failing) tests that SCM sees updated checksums
errose28 Jan 7, 2025
452c294
Update acceptance test
errose28 Jan 8, 2025
dc45eca
Add javadoc for tree generation from metadata
errose28 Jan 8, 2025
1cb291f
Data integration tests passing
errose28 Jan 8, 2025
d6b21d2
Don't generate tree from metadata for unhealthy container
errose28 Jan 9, 2025
2a2dbbd
Checkstyle
errose28 Jan 9, 2025
c9a077c
Marking container unhealthy should not write a merkle tree (test fix)
errose28 Jan 9, 2025
0bbbdc5
Checkstyle
errose28 Jan 9, 2025
7b971a9
Address review comments
errose28 Jan 13, 2025
15d6848
Merge branch 'HDDS-10239-container-reconciliation' into HDDS-10374-sc…
errose28 Apr 11, 2025
0989881
Initial use of on demand scan in TestKeyValueHandler
errose28 Apr 11, 2025
834be96
Make on-demand scanner a normal instance
errose28 Apr 15, 2025
e73757e
Register on-demand scan callback in ContainerSet
errose28 Apr 15, 2025
f0d8efe
Migrate scanContainer usage in prod code
errose28 Apr 15, 2025
4cb054c
Switch terminology from error to scan. Add existence checks
errose28 Apr 15, 2025
8abedb6
Update tests
errose28 Apr 15, 2025
577a075
Add unit test for ContainerSet
errose28 Apr 16, 2025
4c8d843
Checkstyle
errose28 Apr 16, 2025
0bd4127
Improve comments and test
errose28 Apr 16, 2025
61fae12
Merge branch 'non-static-on-demand-scan' into HDDS-10374-scanner-buil…
errose28 Apr 16, 2025
61f30f3
WIP migrate reconciliation unit tests
errose28 Apr 17, 2025
192eb7b
Most tests passing
errose28 Apr 23, 2025
0cf79f6
Improve logging in test and prod code
errose28 Apr 28, 2025
8b30f54
Fix tree tracking during reconcile process
errose28 Apr 28, 2025
9c74f4b
Use mixin to standardize scanner operations, log checksum changes in …
errose28 Apr 29, 2025
d550669
Logging improvements
errose28 Apr 29, 2025
97e02ea
Add checksum validation, generate readable data
errose28 Apr 30, 2025
22b41b8
Use tree writer between peer updates. All tests pass
errose28 May 5, 2025
f49a9dd
Wait for on-demand scans to complete in test
errose28 May 5, 2025
f5d4dbf
Improve char data generation, reset scan metrics
errose28 May 5, 2025
1140c90
Update test name
errose28 May 5, 2025
e0aa7cb
Checkstyle
errose28 May 5, 2025
62d7794
Merge branch 'HDDS-10239-container-reconciliation' into HDDS-10374-sc…
errose28 May 6, 2025
9c3b87c
Merge branch 'reconcile-unit-test-framework' into HDDS-10374-scanner-…
errose28 May 6, 2025
9322b4a
Fix TODOs dependent on this patch
errose28 May 13, 2025
9b75957
Rename container scan helper
errose28 May 13, 2025
f615275
Add comment on failure type
errose28 May 13, 2025
dadc829
Fix checkstyle unique to this PR
errose28 May 13, 2025
076a82e
Merge branch 'HDDS-10239-container-reconciliation' into HDDS-10374-sc…
errose28 May 14, 2025
cc55527
Fix sending ICR when only checksum changes (pending test)
errose28 May 14, 2025
35879b4
Updates after reviewing diff
errose28 May 14, 2025
1ab8c14
Add unit test for KeyValueHandler#updateContainerChecksum
errose28 May 14, 2025
6c8be07
Improve and update scanner integration tests
errose28 May 14, 2025
60a1a6e
Add unit tests that checksum update failure does not stop container s…
errose28 May 14, 2025
d035c17
Checkstyle
errose28 May 14, 2025
53336ae
Fix scan gap for unit test
errose28 May 15, 2025
56e7ed4
Merge branch 'HDDS-10239-container-reconciliation' into HDDS-10374-sc…
errose28 May 16, 2025
2504638
Fix metadata scan test
errose28 May 16, 2025
4be9992
Update based on review
errose28 May 19, 2025
c0b89dd
pmd
errose28 May 19, 2025
e24a24e
Update ContainerData checksum info after reconcile with each peer
errose28 May 22, 2025
dc27f74
Support bypassing scan gap (tests are failing)
errose28 May 22, 2025
e2974b4
Checkstyle
errose28 May 27, 2025
34b4b9a
Fix scan gap bug. All tests expected to pass
errose28 May 27, 2025
5fda700
Fix scan gap call
errose28 Jun 2, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Data integration tests passing
  • Loading branch information
errose28 committed Jan 8, 2025
commit 1cb291f2e0e76e327f0920a018b773e89b49301e
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@ public void writeContainerDataTree(ContainerData data, ContainerMerkleTree tree)
long containerID = data.getContainerID();
Lock writeLock = getLock(containerID);
writeLock.lock();
// If there is an error generating the tree and we cannot obtain a final checksum, use 0 to indicate a metadata
// failure.
long dataChecksum = 0;
try {
ContainerProtos.ContainerChecksumInfo.Builder checksumInfoBuilder = null;
try {
Expand All @@ -103,14 +106,16 @@ public void writeContainerDataTree(ContainerData data, ContainerMerkleTree tree)

ContainerProtos.ContainerMerkleTree treeProto = captureLatencyNs(metrics.getCreateMerkleTreeLatencyNS(),
tree::toProto);
long dataChecksum = treeProto.getDataChecksum();
data.setDataChecksum(dataChecksum);
checksumInfoBuilder
.setContainerID(containerID)
.setContainerMerkleTree(treeProto);
write(data, checksumInfoBuilder.build());
// If write succeeds, update the checksum in memory. Otherwise 0 will be used to indicate the metadata failure.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should there be a way to tell if a checksum failed vs. scanner has not yet run? Should failure to generate checksum == -1?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed, the in-memory hash should be what is written to disk as it is an in-memory cache. If updating the Merkle tree is failing, we can have a metric + log message and/or a status report to SCM or a new Admin API to Datanode to query the Merkle tree (I think there are going to be multiple debug scenarios where we would like to query a Datanode what it knows independent of SCM reports).

dataChecksum = treeProto.getDataChecksum();
LOG.debug("Data merkle tree for container {} updated with container checksum {}", containerID, dataChecksum);
} finally {
// Even if persisting the tree fails, we should still update the data checksum in memory to report back to SCM.
data.setDataChecksum(dataChecksum);
writeLock.unlock();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,13 @@ public void scanContainer(Container<?> c)
if (result.isDeleted()) {
LOG.debug("Container [{}] has been deleted during the data scan.", containerId);
} else {
// Merkle tree write failure should not abort the scanning process. Continue marking the scan as completed.
try {
checksumManager.writeContainerDataTree(containerData, result.getDataTree());
} catch (IOException ex) {
LOG.error("Failed to write container merkle tree for container {}", containerId, ex);
}

if (!result.isHealthy()) {
logUnhealthyScanResult(containerId, result, LOG);

Expand All @@ -104,7 +111,6 @@ public void scanContainer(Container<?> c)
}
}
metrics.incNumContainersScanned();
checksumManager.writeContainerDataTree(containerData, result.getDataTree());
}

Instant now = Instant.now();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,12 @@ private static void performOnDemandScan(Container<?> container) {
if (result.isDeleted()) {
LOG.debug("Container [{}] has been deleted during the data scan.", containerId);
} else {
// Merkle tree write failure should not abort the scanning process. Continue marking the scan as completed.
try {
instance.checksumManager.writeContainerDataTree(containerData, result.getDataTree());
} catch (IOException ex) {
LOG.error("Failed to write container merkle tree for container {}", containerId, ex);
}
if (!result.isHealthy()) {
logUnhealthyScanResult(containerId, result, LOG);
boolean containerMarkedUnhealthy = instance.containerController
Expand All @@ -150,7 +156,6 @@ private static void performOnDemandScan(Container<?> container) {
}
}
instance.metrics.incNumContainersScanned();
instance.checksumManager.writeContainerDataTree(containerData, result.getDataTree());
}

Instant now = Instant.now();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,14 +98,14 @@ void testCorruptionDetected(TestContainerCorruptions corruption)
assertEquals(State.CLOSED, closedContainer.getContainerState());
assertTrue(containerChecksumFileExists(closedContainerID));
waitForScmToSeeReplicaState(closedContainerID, CLOSED);
long initialReportedClosedChecksum = getContainerReplica(closedContainerID).getDataChecksum();
long initialClosedChecksum = getContainerReplica(closedContainerID).getDataChecksum();
assertNotEquals(0, initialClosedChecksum);

long openContainerID = writeDataToOpenContainer();
Container<?> openContainer = getDnContainer(openContainerID);
assertEquals(State.OPEN, openContainer.getContainerState());
long initialReportedOpenChecksum = getContainerReplica(openContainerID).getDataChecksum();
// Open containers should not yet have a checksum generated.
assertEquals(0, initialReportedOpenChecksum);
assertEquals(0, getContainerReplica(openContainerID).getDataChecksum());

// Corrupt both containers.
corruption.applyTo(closedContainer);
Expand All @@ -120,10 +120,12 @@ void testCorruptionDetected(TestContainerCorruptions corruption)
500, 5000);

// Wait for SCM to get reports of the unhealthy replicas.
// The metadata scanner does not generate data checksums and the other scanners have been turned off for this
// test, so the data checksums should not change.
waitForScmToSeeReplicaState(closedContainerID, UNHEALTHY);
assertNotEquals(initialReportedClosedChecksum, getContainerReplica(closedContainerID).getDataChecksum());
assertEquals(initialClosedChecksum, getContainerReplica(closedContainerID).getDataChecksum());
waitForScmToSeeReplicaState(openContainerID, UNHEALTHY);
assertNotEquals(initialReportedOpenChecksum, getContainerReplica(openContainerID).getDataChecksum());
assertEquals(0, getContainerReplica(openContainerID).getDataChecksum());

// Once the unhealthy replica is reported, the open container's lifecycle
// state in SCM should move to closed.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@

import java.util.Collection;

import static org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.ContainerReplicaProto.State.CLOSED;
import static org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.ContainerReplicaProto.State.UNHEALTHY;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
Expand Down Expand Up @@ -102,9 +103,11 @@ void testCorruptionDetected(TestContainerCorruptions corruption)
// Container corruption has not yet been introduced.
Container<?> container = getDnContainer(containerID);
assertEquals(State.CLOSED, container.getContainerState());
long initialReportedDataChecksum = getContainerReplica(containerID).getDataChecksum();
assertTrue(containerChecksumFileExists(containerID));

waitForScmToSeeReplicaState(containerID, CLOSED);
long initialReportedDataChecksum = getContainerReplica(containerID).getDataChecksum();

// Corrupt the container.
corruption.applyTo(container);
// This method will check that reading from the corrupted key returns an
Expand Down