HDDS-10372. SCM and Datanode communication for reconciliation #6506

errose28 · 2024-04-10T06:30:47Z

What changes were proposed in this pull request?

A lot of boilerplate code to do something very simple:

Tell SCM to start reconciliation for a container from the CLI.
Have SCM tell Datanodes to reconcile that container with their peers.
Datanodes send back a placeholder container data checksum which we can fill in with reconciliation implementation later.
- There is no communication between datanodes added in this change.
SCM updates its replica info based on the container report received after the Datanodes reconcile.

I've tried to avoid making any design related decisions in this PR. It is intended as a skeleton we can use to plug in the reconciliation implementation for end to end testing in future changes.

In scope for this change

Add new ozone admin container reconcile <container-id> command.
New command should be restricted to admins
Audit logging for new command
Blocking reconciliation of invalid containers (EC, 1 replica, still open)
Datanode queue metrics for reconciliation commands
Datanode and SCM application logs to follow the command as it moves through the system.
SCM saves container replicas' data checksums in memory, and they can be retrieved with ozone admin container info --json

Out of scope for this change (but will be handled in later tasks)

Any actual checksum related implementations
- Currently byte strings are used as placeholders just to move filler data around for testing.
Recon integration with container data checksums
- This includes Recon's ContainerReplicaHistoryProto
Finalized protobuf changes
- Since the change is going to a feature branch we have the flexibility to evolve the protos later.
Good UX 😄
- This includes flags for the reconcile command, an easy way to track reconciliation progress, and reading containers from stdin like other container subcommands support.
- These will need some discussion so are probably best done as their own set of changes.
The following tasks have been moved out to do in follow up changes:
- HDDS-10714 datanode status filtering for reconciliation peers and targets
- HDDS-10759 Consider allowing reconciliation when not all replicas have reached closed state
- HDDS-10760 SCMExceptions resulting from admin CLI commands are treated as retriable

What is the link to the Apache JIRA

HDDS-10372

How was this patch tested?

Acceptance test for CLI added
Manually tested the CLI with valid and invalid containers. Also manually checked SCM audit logging
Unit and integration tests added in the following classes:
- TestReconcileContainerEventHandler: Tests SCM's filtering of reconciliation requests based on eligible container and replica states. When containers are eligible, tests that reconcile commands are sent to datanodes.
- TestStateContext: Tests that the new command shows up in datanode queue metrics.
- TestReconcileContainerCommandHandler: Tests datanode queue and runtime metrics when a reconcile command is received. Also tests that the ICR sent as the result of the command has the expected data checksum.
- TestContainerDataYaml: Tests that the data checksum is not written to the .container file. Merkle tree information will be written to its own file in a different change.
- TestHeartbeatEndpointTask: Tests that datanodes add a reconcile command to their queue when it is received on an SCM heartbeat response.
- TestKeyValueHandler: Tests that the the KeyValueHandler triggers an ICR back to SCM with the expected values when reconciliation is invoked.
- TestContainerReportHandler, TestIncrementalContainerReportHandler: Tests that SCM correctly saves replicas' data checksum information it receives on a heartbeat.

Still working on best way to get errors back to the client

Also renamed existing checksum field and methods in ContainerData.

Some request blocking based on container states is not yet implemented.

Also move some reconiliation specific classes to their own package.

Still not done

It's coming back as 0 always for some reason.

errose28 · 2024-05-07T03:22:52Z

At last the CI is green

kerneltime

Finished one round, will go over testing next.

hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/client/ScmClient.java

kerneltime · 2024-05-12T19:24:56Z

hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerReplicaInfo.java

@@ -35,6 +40,8 @@ public final class ContainerReplicaInfo {
  private long keyCount;
  private long bytesUsed;
  private int replicaIndex = -1;
+  @JsonSerialize(using = LongToHexJsonSerializer.class)
+  private long dataChecksum;


Q: Hex for printing to make it more human friendly?

Yeah. That seemed standard but we could use a different format if there's a better option.

kerneltime · 2024-05-12T19:30:27Z

...p-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/ReconcileSubcommand.java

+    System.out.println("Use \"ozone admin container info --json " + containerId + "\" to see the checksums of each " +
+        "container replica");
+  }


Suggested change

System.out.println("Use \"ozone admin container info --json " + containerId + "\" to see the checksums of each " +

"container replica");

}

System.out.println("Once reconciliation is complete, use \"ozone admin container info --json " + containerId + "\" to see the checksums of each " +

"container replica");

}

.github/workflows/repeat-acceptance.yml

aswinshakil

Thanks for working on this @errose28. I just have a few minor nits. Otherwise LGTM

...rver-scm/src/main/java/org/apache/hadoop/hdds/scm/container/reconciliation/package-info.java

hadoop-ozone/dist/src/main/smoketest/admincli/container.robot

…concile-cli * HDDS-10239-container-reconciliation: (296 commits) HDDS-10897. Refactor OzoneQuota (apache#6714) HDDS-10422. Fix some warnings about exposing internal representation in hdds-common (apache#6351) HDDS-10899. Refactor Lease callbacks (apache#6715) HDDS-10890. Increase default value for hdds.container.ratis.log.appender.queue.num-elements (apache#6711) HDDS-10832. Client should switch to streaming based on OpenKeySession replication (apache#6683) HDDS-10435. Support S3 object tags for existing requests (apache#6607) HDDS-10883. Improve logging in Recon for finalising DN logic. (apache#6704) HDDS-8752. Enable TestOzoneRpcClientAbstract#testOverWriteKeyWithAndWithOutVersioning (apache#6702) HDDS-10875. XceiverRatisServer#getRaftPeersInPipeline should be called before XceiverRatisServer#removeGroup (apache#6696) HDDS-10514. Recon - Provide DN decommissioning detailed status and info inline with current CLI command output. (apache#6376) HDDS-10878. Bump zstd-jni to 1.5.6-3 (apache#6701) HDDS-10877. Bump Dropwizard metrics to 3.2.6 (apache#6699) HDDS-10876. Bump jackson to 2.16.2 (apache#6697) HDDS-6116. Remove flaky tag from TestSCMInstallSnapshot (apache#6695) HDDS-2643. TestOzoneDelegationTokenSecretManager#testRenewTokenFailureRenewalTime fails intermittently. HDDS-10699. Refactor ContainerBalancerTask and TestContainerBalancerTask (apache#6537) HDDS-10861. Ozone cli supports default ozone.om.service.id (apache#6680) HDDS-10859. Improve error messages when decommission and maintenance fail-early (apache#6678) HDDS-9031. Upgrade acceptance tests to Docker Compose v2 (apache#6667) HDDS-10559. Add a warning or a check to run repair tool as System user (apache#6574) ... Conflicts: hadoop-ozone/dist/src/main/smoketest/admincli/container.robot

* HDDS-10239-container-reconciliation: HDDS-10372. SCM and Datanode communication for reconciliation (apache#6506) HDDS-10239. Storage Container Reconciliation. (apache#6121)

errose28 added 9 commits March 1, 2024 15:37

WIP SCM changes for reconcile cli

8339f96

Changes for SCM WIP pt2

573ec30

Still working on best way to get errors back to the client

Use SCMException only for error handling

b259dae

Add SCM event handler for reconcile events

4a89c8e

Add datanode reconcile stub.

f9d1bfd

Also renamed existing checksum field and methods in ContainerData.

Updates after reviewing diff

ddf3ce8

Fix checkstyle

47f6c06

Basic reconcile scm <-> DN works

9154e3c

Improve error handling

d036229

errose28 added the container-reconciliation label Apr 10, 2024

errose28 added 20 commits April 12, 2024 15:08

Add DN side unit tests

9aecbcf

Test with two containers

2775807

Add container report handler tests

600174f

Add ICR tests, improve FCR tests

6ec2acb

Remove duplicate line from report handler test

4841c8f

Add (currently failing) test for scm event handler

8e7e8f7

Some request blocking based on container states is not yet implemented.

Refactor contaienr eligibility, SCM event handler tests pass

74bb00a

Also move some reconiliation specific classes to their own package.

Checkstyle

2effd78

Add robot test that may not pass yet

d055094

Rat

3586cc3

Test repeat run with fixed acc test workflow

ac6ff0e

Some acceptance test fixes

d84d3ef

Update comment

a4a0a1c

findbugs

6f2c7e4

Separate container handler test for metrics

9c94f1c

Still not done

Almost finished separated metrics and report tests

e42324e

TestReconcileContainerCommandHandler complete and passing

3cb0416

Checkstyle

18f4f2a

Fix TestSCMExceptionResultCodes

2163008

Might have fixed acceptance test

13a335e

errose28 added 10 commits May 2, 2024 15:50

Use long as checksum representation

b03c4fc

Apparently snakeyaml is coupled to Java variable names

bc434ec

checkstyle

290fbb7

Rename two existing container file checksum methods for clarity

afd6043

Test that container data checksum is not written to .container file

d8e86bd

Fix simple acceptance test issue

990735f

Print data checksum as hex string in container info output

f4ace2d

Add log of placeholder checksum generated

36f92ee

It's coming back as 0 always for some reason.

checkstyle

f26a402

putLong increments position, need to rewind.

c8bdffc

errose28 marked this pull request as ready for review May 7, 2024 03:22

Undo acceptance workflow change from master

f862836

kerneltime reviewed May 13, 2024

View reviewed changes

kerneltime reviewed May 21, 2024

View reviewed changes

.github/workflows/repeat-acceptance.yml Outdated Show resolved Hide resolved

errose28 added 2 commits May 21, 2024 11:08

Undo accidental gh actions change

8a2427b

Fix comment typo

31e60a9

aswinshakil self-requested a review May 22, 2024 16:34

aswinshakil approved these changes May 28, 2024

View reviewed changes

...rver-scm/src/main/java/org/apache/hadoop/hdds/scm/container/reconciliation/package-info.java Outdated Show resolved Hide resolved

hadoop-ozone/dist/src/main/smoketest/admincli/container.robot Show resolved Hide resolved

errose28 added 2 commits May 28, 2024 14:31

Address review comments, fix error after merge commit

28b8862

kerneltime approved these changes May 29, 2024

View reviewed changes

kerneltime merged commit 930d3f0 into apache:HDDS-10239-container-reconciliation May 29, 2024
39 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HDDS-10372. SCM and Datanode communication for reconciliation #6506

HDDS-10372. SCM and Datanode communication for reconciliation #6506

errose28 commented Apr 10, 2024 •

edited

Loading

errose28 commented May 7, 2024

kerneltime left a comment

kerneltime May 12, 2024

errose28 May 21, 2024

kerneltime May 12, 2024

aswinshakil left a comment

HDDS-10372. SCM and Datanode communication for reconciliation #6506

HDDS-10372. SCM and Datanode communication for reconciliation #6506

Conversation

errose28 commented Apr 10, 2024 • edited Loading

What changes were proposed in this pull request?

In scope for this change

Out of scope for this change (but will be handled in later tasks)

What is the link to the Apache JIRA

How was this patch tested?

errose28 commented May 7, 2024

kerneltime left a comment

Choose a reason for hiding this comment

kerneltime May 12, 2024

Choose a reason for hiding this comment

errose28 May 21, 2024

Choose a reason for hiding this comment

kerneltime May 12, 2024

Choose a reason for hiding this comment

aswinshakil left a comment

Choose a reason for hiding this comment

errose28 commented Apr 10, 2024 •

edited

Loading