[
https://issues.apache.org/jira/browse/HDFS-12159?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16123912#comment-16123912
]
Xiaoyu Yao commented on HDFS-12159:
-----------------------------------
Thanks [~anu] for the update. The latest patch looks good to me. Below are some
new comments.
Ozone.proto
Line 78: Do we need a separate STAND_ALONE?
This will be either RATIS of size 1 or CHAINED of size 1, which will be more
generic.
StorageContainerLocationProtocol.proto
Line 121: should we name it nodepool to be consistent?
StorageContainerLocationProtocol.java
Line 38-39: We are changing to use protobuf types like
OzoneProtos.ReplicationFactor/ReplicationType
directly in APIs. Is this the style want to do moving forward in other places
to avoid the conversion
between PB types and the Ozone types?
ContainerOperationClient.java
NIT: line 108-109 java doc needs update
(or we should use @inherit for the @override methods to avoid double document
overhead)
ScmClient.java
NIT: line 102 java doc incomplete
StorageContainerLocationProtocolClientSideTranslatorPB.java
Line 219: NIT: should we call allocatePipeline instead of
createReplicationPipeline to be consistent?
In the container case, the caller of the allocateContainer could be different
from
the client actually create containers on datanodes.
StorageContainerLocationProtocol.proto
Line 166: based on the comments here, we seems to let SCM talks to datanodes to
create the pipeline,
which is different from how container is allocated/created. Is this correct? If
that is the case, pipeline
will not need to separate states between allocate/create.
DatanodeStateMachine.java
Line 91-94: these can be removed.
StateContext.java
Line 102: NIT: can we define something like INVALID_PORT = -1 somewhere like
OzoneConsts.java?
XceiverServerRatis.java
Line 98-100: can be removed or change output to debug log
Line 111: should we have a special configuration keys for remapping of the
storageDir for Ratis in
MinOzoneCluster mode?
OzoneContainer.java
Line 196: use OzoneConsts#INVALID_PORT?
ContainerMapping.java
Line 165: NIT: unnecessary line break
PipelineManager.java
Line 41: do we assume the Pipleline returned in OPERATIONAL state?
Or do we allow getPipleLine by state for pipeline cleanup?
Line 43: NIT: can we rename getReplicationPipeline to getPipeline?
Line 63: NIT: can we call it getMembers? That matches well with the javadoc
above it.
Line 68: NIT: rename to updatePipeline()? we may also need updatePipeline with
states from PipelineManager.
StandaloneManagerImpl.java
Line 59: do we need more than one datanodes on standalong replication?
I would suggest we make this a ChainedManagerImpl.java and support standalone
as a special case of it and leave the the multi-node chained replication as
unimplemented.
This way, we do not need to maintain the standaloneManager. We could do that
in a separate patch since this one is already large.
> Ozone: SCM: Add create replication pipeline RPC
> -----------------------------------------------
>
> Key: HDFS-12159
> URL: https://issues.apache.org/jira/browse/HDFS-12159
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: ozone
> Affects Versions: HDFS-7240
> Reporter: Anu Engineer
> Assignee: Anu Engineer
> Fix For: HDFS-7240
>
> Attachments: HDFS-12159-HDFS-7240.001.patch,
> HDFS-12159-HDFS-7240.002.patch, HDFS-12159-HDFS-7240.003.patch
>
>
> Add an API that allows users to create replication pipelines using SCM.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
