[
https://issues.apache.org/jira/browse/HDFS-13300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16410541#comment-16410541
]
Nanda kumar commented on HDFS-13300:
------------------------------------
Thanks [~xyao] and [~elek] for the review, updated the patch v003 accordingly.
Please find my response below
Hdsl.proto
{quote}should we make ipAddress/hostName/infoPort required?
{quote}
Made ipAddress/hostName as a required field. infoPort is not required in
HDSL/Ozone, HDFS-13324 is to track the removal of InfoPort and InfoSecurePort.
StorageContainerDatanodeProtocol.proto
{quote}Line 35-40: unused imports can be removed from proto file (IntelliJ
won't report unused protobuf import)
{quote}
Fixed
CommandQueue.java
{quote}DatanodeDetails.Uuid is better than DatanodeDetails.datanodeUuid
{quote}
Fixed
ContainerManagerImpl.java
{quote}Line 124: NIT: remove the "ID"
{quote}
Fixed
DatanodeDeletedBlockTransactions.java
Line 44: this map can be keyed by UUID
Fixed
DatanodeDetails.java
{quote}Can we directly use the protobuf generated class to avoid conversions?
{quote}
This can be tricky as we made IP and hostname a required field in protobuf.
When we create DatanodeDetails instance in {{HdslDatanodeService}} we don't
have value for IP/hostname, it's set later by XceiverServer/XceiverServerRatis.
If we directly use protobuf generated class we have to set the values in
builder before calling {{builder.build()}}
HdfsUtils.java
{quote}Unrelated change in hdfs can be avoided.
{quote}
Fixed
HdslDatanodeService.java
{quote}Line 80: can we define a separate SCM Exception for this?
{quote}
{{IllegalArgumentException}} is used in a similar manner in HDSL/Ozone whenever
a mandatory property is missing. Created HDFS-13333 to introduce new Exception
to handle such cases.
DataNodeServicePlugin.java
{quote}Agree with @Marton that we should use the ServicePlugin directly.
{quote}
Planning to handle this as part of HDFS-13325
InitDatanodeState.java
{quote}Line 109: agree with @Marton that we might not need to persist all as
the port may change or unavailable across a restart.
{quote}
Created HDFS-13334 to track this
ObjectStoreRestPlugin.java
{quote}Line 78-86: can be simplified with super class's implementation of the
same method.
{quote}
The super class/interface (DataNodeServicePlugin) will be removed as part of
HDFS-13325
SCMNodeManager.java
{quote}Line 126: can we change the nodeStats map to be keyed by UUID as well?
{quote}
Fixed
{quote}ServiceDiscovery is based on the healthyNodes/staleNodes/deadNodes maps
in SCMNodeManager. Let me know if I am wrong but I can imagine the following
situation.
# HdslDatanodeService.start() is called by the DataNode()
## DatanodeDescriptor is created
## DatanodeMachine is started (Async! Separated threads)
# ObjectStoreRestPlugin.start() is called
## In the mean time the DatanodeMachine is started in a separated thread (see
the previous point). RunningDatanodeState may send the first registration
request to the SCM.
## Rest interface is started and restPort is updated in the
DatanodeDescriptor. But the descriptor has already been sent to the SCM.
# With the next heartbeats the latest datanodeDescriptor will be sent to the
scm. But the maps (healthyNodes/staleNodes/deadNodes) are not updated with the
new descriptor. Still the original descriptor will be used which (in some
cases) doesn't contain the rest port.{quote}
@Marton, your understanding is completely right, but the ports are set during
the construction of {{DatanodeStateMachine}} not after
{{datanodeStateMachine.startDaemon()}}. So when
{{ObjectStoreRestPlugin.start()}} we have already created
{{DatanodeStateMachine}} instance which would have updated the ports properly.
In DataNode#startPlugins
# HdslDatanodeService#start is called
## Create DatanodeDetails --> Blocking call
## Create DatanodeStateMachine --> Blocking call
### Create OzoneContainer instance --> Blocking call
#### Create XceiverServer --> Blocking call
##### Port is updated in DatanodeDetails reference
#### Create XceiverServerRatis --> Blocking call
##### Port is updated in DatanodeDetails reference
## Start DatanodeStateMachine --> NON Blocking call (Control is returned to
DataNode)
# ObjectStoreRestPlugin.start is called (The ports are already updated through
DatanodeStateMachine)
> 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,
> HDFS-13300-HDFS-7240.003.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
