[
https://issues.apache.org/jira/browse/HDFS-12745?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16260643#comment-16260643
]
Mukul Kumar Singh commented on HDFS-12745:
------------------------------------------
NIT:
XceiverClientStandAlone.java -> XceiverClientStandalone.java and the class name
to be consistent.
bq. Done
Pipeline.java
Line 79: Should we use the OzoneProtos.Pipeline directly to avoid the
conversion, which also have a builder to construct pipeline object without
worrying about different constructors?
bq. The pipeline object is being used elsewhere in the code heavily. I would
like to keep just one constructor for pipeline class.
ContainerProtocolCalls.java
Line 229: agree with Anu Engineer that we can get the pipeline from a client
unless it is removed from the XceiverClientSpi interface and implementation.
Can you elaborate on adding the additional parameter for the pipeline?
bq. This cannot be used, as we still create one pipeline object per container.
The list of machine aka pipeline name is re-used but we still create a new
object. I had tried removing containername from pipeline but that change which
was still incomplete was spanning into 47+ files. I would like to solve that as
part of a different jira. I have raised HDFS-12841 to fix this.
Line 237: should we remove the containerName from protobuf definition for
pipeline as a pipeline may be shared by multiple containers?
bq. Yes, Would like to do it as part of a different jira. I have raised
HDFS-12841 to fix this.
XceiverServerInitializer.java
Line 33: NIT: XceiverServerStandAlone->XceiverServerStandalone
Done.
XceiverServerStandAlone.java
Line 45/47/60: same as above
Done.
PipelineManager.java
Line 40: suggest to change activePipelines from private to protect so that
subclass can access it to update/close pipeline.
bq. Right now, the pipelines always stay open. This would be fixed later with
HDFS-12722. We can then convert this to a concurrent List.
Also, how do we protect the thread safe of activePipelines among
get/update/close operations?
bq. Same as above.
Line 78: should we allow round-robin of existing pipelines if the newNodes is
less than the replication factor requested?
bq. Yes, that is already done. if we are not able to allocate sufficient number
of nodes. One of the already existing open pipeline is re-used.
Line 82/93: the number of parameters in the format string (1) is less than the
actual parameters (2), we need a {} after “for container”
bq. Done
> Ozone: XceiverClientManager should cache objects based on pipeline name
> -----------------------------------------------------------------------
>
> Key: HDFS-12745
> URL: https://issues.apache.org/jira/browse/HDFS-12745
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: ozone
> Affects Versions: HDFS-7240
> Reporter: Mukul Kumar Singh
> Assignee: Mukul Kumar Singh
> Fix For: HDFS-7240
>
> Attachments: HDFS-12745-HDFS-7240.001.patch,
> HDFS-12745-HDFS-7240.002.patch, HDFS-12745-HDFS-7240.003.patch,
> HDFS-12745-HDFS-7240.004.patch, HDFS-12745-HDFS-7240.005.patch,
> HDFS-12745-HDFS-7240.006.patch
>
>
> With just the standalone pipeline, a new pipeline was created for each and
> every container.
> This code can be optimized so that pipelines are craeted less frequently.
> Caching using pipeline names will help with Ratis clients as well.
> a) Remove Container name from Pipeline object.
> b) XceiverClientManager should cache objects based on pipeline name
> c) XceiverClient and XceiverServer should be renamed to
> XceiverClientStandAlone & XceiverServerRatis
> d) StandAlone pipeline should have notion of re-using pipeline objects.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
