[
https://issues.apache.org/jira/browse/HDFS-13300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16408553#comment-16408553
]
Xiaoyu Yao commented on HDFS-13300:
-----------------------------------
Thanks [~nandakumar131] for the patch. It looks good to me overall. Here are a
few comments:
Hdsl.proto
Line 34-36: should we make ipAddress/hostName/infoPort required?
StorageContainerDatanodeProtocol.proto
Line 35-40: unused imports can be removed from proto file (IntelliJ won't
report unused protobuf import)
(Let's comment out line 149 for the only reference of hdfs.StorageType as it is
not being used in the code)
Then we can completely remove the hdfs related proto dependencies.
{code}
//optionalhadoop.hdfs.StorageTypeProtostorageType=5[default=DISK];
{code}
CommandQueue.java
Line 43: I like the idea of minimize map key size to reduce ozone memory usage.
We should check other places to unnecessary usage of DatanodeDetails as key.
Line 109: We prefer to have shorter name like you mentioned here.
DatanodeDetails.Uuid is better than DatanodeDetails.datanodeUuid
Can you change that in Hdsl.proto.DatanodeDetails and DatanodeDetails.java?
This can be done in a separate JIRA considering the size of current patch.
ContainerManagerImpl.java
Line 124: NIT: remove the "ID"
DatanodeDeletedBlockTransactions.java
Line 44: this map can be keyed by UUID
DatanodeDetails.java
Can we directly use the protobuf generated class to avoid conversions?
This can be done is a separate JIRA.
HdfsUtils.java
Unrelated change in hdfs can be avoided.
HdslDatanodeService.java
Line 80: can we define a separate SCM Exception for this?
DataNodeServicePlugin.java
Agree with @Marton that we should use the ServicePlugin directly. But this can
be
fixed in a separate JIRA.
InitDatanodeState.java
Line 109: agree with @Marton that we might not need to persist all as the port
may change or unavailable across restart.
This can be fixed later.
ObjectStoreRestPlugin.java
Line 78-86: can be simplified with super class's implementation of the same
method.
SCMNodeManager.java
Line 126: can we change the nodeStats map to be keyed by UUID as well?
> Ozone: Remove DatanodeID dependency from HDSL and Ozone
> --------------------------------------------------------
>
> Key: HDFS-13300
> URL: https://issues.apache.org/jira/browse/HDFS-13300
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: ozone
> Reporter: Nanda kumar
> Assignee: Nanda kumar
> Priority: Major
> Attachments: HDFS-13300-HDFS-7240.000.patch,
> HDFS-13300-HDFS-7240.001.patch, HDFS-13300-HDFS-7240.002.patch
>
>
> DatanodeID has been modified to add HDSL/Ozone related information
> previously. This jira is to remove DatanodeID dependency from HDSL/Ozone to
> make it truly pluggable without having the need to modify DatanodeID.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
