Skip to content

HDDS-15239. Support persist StorageType on Datanode Container YAML#10246

Open
xichen01 wants to merge 4 commits into
apache:HDDS-11233from
xichen01:HDDS-15239
Open

HDDS-15239. Support persist StorageType on Datanode Container YAML#10246
xichen01 wants to merge 4 commits into
apache:HDDS-11233from
xichen01:HDDS-15239

Conversation

@xichen01
Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

  • Support persist StorageType on Datanode Container YAML.
  • The StorageType field will be added to the YAML file of the Datanode Container.
  • Newly created Containers can then specify a StorageType.
  • This StorageType will be used for features such as Container replica migration.

FYI:

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-15239

How was this patch tested?

unit test

Copy link
Copy Markdown
Contributor

@ivandika3 ivandika3 left a comment

Choose a reason for hiding this comment

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

Thanks @xichen01 , left one comment.

Copy link
Copy Markdown
Contributor

@ivandika3 ivandika3 left a comment

Choose a reason for hiding this comment

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

LGTM +1.

Copy link
Copy Markdown
Contributor

@chungen0126 chungen0126 left a comment

Choose a reason for hiding this comment

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

Thanks @xichen01 for the patch. LGTM. Just a nit.

// If storageType participates in checksum calculation, when rolling back to older versions that don't
// support storageType, they would recalculate checksum without storageType field
// (as it's unknown to them), causing checksum mismatch and validation failure.
Yaml yaml = ContainerDataYaml.getYamlForContainerType(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please use ContainerDataYaml.getYamlForContainerType(this.containerType, withReplicaIndex); here. Alternatively, consider cleaning up the method getYamlForContainerType(ContainerType containerType, boolean withReplicaIndex) if it is no longer being invoked elsewhere.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I cleaned up the method getYamlForContainerType(ContainerType containerType, boolean withReplicaIndex)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants