[
https://issues.apache.org/jira/browse/HDFS-12159?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Anu Engineer updated HDFS-12159:
--------------------------------
Attachment: HDFS-12159-HDFS-7240.004.patch
[~xyao] Thank you for the detailed review. I have fixed all the issues and all
checkstyle warnings that are applicable. Please take a look at patch 4.
Details of changes below
bq. Ozone.proto: Line 78: Do we need a separate STAND_ALONE?
Stand alone is used for test purpose today and we can remove it in future if
needed.
bq. Do we need a separate STAND_ALONE?
I was hoping to develop a HDFS like pipeline in future. That is what Chained
meant. Stand alone is the used only for test purposes.
bq. StorageContainerLocationProtocol.proto: Line 121: should we name it
nodepool to be consistent?
Fixed.
bq. StorageContainerLocationProtocol.java Line 38-39: We are changing to use
protobuf types like OzoneProtos.ReplicationFactor/ReplicationType
I don't see much value in converting PB types to enums. For more complex
types, I do get that protobuf generated code does not feel very natural.
I am open to doing a Java Enum and then converting them to Protobuf for RPC
purposes, but it is an enum with a switch case for converation.
bq. ContainerOperationClient.java NIT: line 108-109 java doc needs update
Fixed.
bq. ScmClient.java NIT: line 102 java doc incomplete
Fixed.
bq. StorageContainerLocationProtocolClientSideTranslatorPB.java Line 219: NIT:
should we call allocatePipeline instead of createReplicationPipeline to be
consistent?
Fixed.
bq. StorageContainerLocationProtocol.proto Line 166: based on the comments
here, we seems to let SCM talks to datanodes to create the pipeline,
Fixed the wrong comment.
bq. DatanodeStateMachine.javaLine 91-94: these can be removed.
Fixed.
bq. StateContext.java
Line 102: NIT: can we define something like INVALID_PORT = -1 somewhere like
OzoneConsts.java?
Fixed.
bq. XceiverServerRatis.java Line 98-100: can be removed or change output to
debug log
Fixed.
bq. Line 111: should we have a special configuration keys for remapping of the
storageDir for Ratis in MinOzoneCluster mode?
Not really, the localPort is taken care by the DatanodeID. If want we can map
the storage directory under
datanode ID or localPort permanantley, so we don't need any special casing.
bq. OzoneContainer.java Line 196: use OzoneConsts#INVALID_PORT?
Fixed.
bq. ContainerMapping.java Line 165: NIT: unnecessary line break
Fixed.
bq. PipelineManager.java: Line 41: do we assume the Pipleline returned in
OPERATIONAL state?
Currently we do assume all pipelines are operational since currently we will
only return STAND_ALONE pipelines. When Ratis pipelines are are fully
operational we will have to handle
other states too. Added a comment to clarify this.
bq. Line 43: NIT: can we rename getReplicationPipeline to getPipeline?
Fixed.
bq. Line 63: NIT: can we call it getMembers? That matches well with the javadoc
above it.
Fixed.
bq. Line 68: NIT: rename to updatePipeline()? we may also need updatePipeline
with states from PipelineManager.
Fixed.
bq. Line 59: do we need more than one datanodes on standalong replication? I
would suggest we make this a ChainedManagerImpl.java and support standalone
You are right, we should probably remove stand alone replication in the long
run. Right now, this is the code path that is functional and we need to enable
testing for the pluggable framework. The chained replication at some point will
replicate to other machines. Stand alone, is what we have now, without the
ability to replicate.
> 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,
> HDFS-12159-HDFS-7240.004.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: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
