Skip to content

HDDS-4821. Use SCM service ID in SCMBlockClient and SCM Client.#1940

Merged
bharatviswa504 merged 3 commits intoapache:HDDS-2823from
bharatviswa504:HDDS-4821-l
Feb 22, 2021
Merged

HDDS-4821. Use SCM service ID in SCMBlockClient and SCM Client.#1940
bharatviswa504 merged 3 commits intoapache:HDDS-2823from
bharatviswa504:HDDS-4821-l

Conversation

@bharatviswa504
Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Use SCM service ID in SCMBlockClient and SCM Client.

If not defined fall back to RPC address and port config.

Rename ozone.scm.internal.service.id -> ozone.scm.default.service.id for better clarity.

Refactor/move few classes to use across ozone and hdds.

What is the link to the Apache JIRA

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

How was this patch tested?

Added tests.

…M service ID in SCMBlockClient and SCM Client.
String localScmServiceId = conf.getTrimmed(
ScmConfigKeys.OZONE_SCM_DEFAULT_SERVICE_ID);

Collection< String > scmServiceIds;
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.

Collection< String > -> Collection

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.

Here as Collection would be a string and during iteration, instead of typecasting to a string, so declared collection as String. (Removed spaces)

@@ -0,0 +1,246 @@
/*
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.

Can't we reuse SCMNodeDetails class here?

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.

The main idea here is SCMNodeInfo will be used by SCM Clients like OM and SCM.
As they need to fiigure out rpc address from newly added config key ozone.scm.address or old rpc address config.
Whereas SCM fetches address from bindhost. To make clear distinguish I have added the new class.

@bshashikant
Copy link
Copy Markdown
Contributor

Looks good with just minor comments. Let's add some comment on SCMNodeInfo class so as to differentiate between SCMNodeInfo and SCMNodedetails class.

@bharatviswa504
Copy link
Copy Markdown
Contributor Author

Thank You @bshashikant for the review.
I will address these review comments in HDDS-4837 which is blocked on this. Hope this would be okay with you.

@bharatviswa504 bharatviswa504 merged commit 8d49b81 into apache:HDDS-2823 Feb 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants