[
https://issues.apache.org/jira/browse/YARN-8953?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16687561#comment-16687561
]
Sunil Govindan commented on YARN-8953:
--------------------------------------
Hi [~cheersyang]
Thanks for working on this patch. Few comments.
# In {{CsiAdaptorPB.java}}, first line of including package name is not
correct. It should be after ASF license.
# Missing javadocs in CsiAdaptorProtocol
# Does GetPluginInfoRequest need to be an abstract class?
# It seems there could be various VolumeCapability for a volume. However
ValidateVolumeCapabilitiesRequest ctor support adding only one. If this is
added for a long term vision, could we also change the ctor in such a way that
it takes a List of one item. (List<VolumeCapability> volumeCapability)
# In {{ValidateVolumeCapabilitiesResponse}} is it more like a diagnostics or
a return feedback which ll be set from {{setMessage}}. I think it can be
renamed to diag or response message etc.
# I think {{NM_CSI_ADAPTOR_ADDRESSES}} and {{NM_SIMPLE_CSI_ADAPTOR_ADDRESS}}
are confusing name. could we avoid one in this? or could give a better name.
SIMPLE looks like a default adaptor address?
# Is YarnCsiAdaptor.proto going to compiled against protobuff 3.0? If so, i
think there is an ambiguity. This file is present in same proto folder. Not
sure how to categorize for future where some proto belong to 3.0 and some 2.5.
What do u think?
# Since yarn_csi_adaptor.proto has AccessMode and VolumeType enum, do we still
need same in ValidateVolumeCapabilitiesRequest?
# Now hadoop-yarn-csi/pom.xml needs hadoop-yarn-common, hadoop-yarn-api etc.
Vice versa is not needed, correct? What i meant was, we might NOT need
hadoop-yarn-common depend on hadoop-yarn-csi, correct?
# Can adaptorServiceAddress be a remote host address?
# In CsiAdaptorProtocolService#serviceStart, any possible exception handling
needed due to ops in YarnRPC?
# Does any change needed for Transformer classes, given we add a new field in
the csi protos later? Earlier, when we have a change in proto, we usually
change the PBImpls classes and respective converter class. Now the PBImpl are
in yarn-api package and transformer code is in csi module. Any guide or change
in code structure needed to ease this in future?
> Add CSI driver adaptor module
> -----------------------------
>
> Key: YARN-8953
> URL: https://issues.apache.org/jira/browse/YARN-8953
> Project: Hadoop YARN
> Issue Type: Sub-task
> Reporter: Weiwei Yang
> Assignee: Weiwei Yang
> Priority: Major
> Attachments: YARN-8953.001.patch, YARN-8953.002.patch,
> YARN-8953.003.patch, YARN-8953.004.patch, csi_adaptor_workflow.png
>
>
> CSI adaptor is a layer between YARN and CSI driver, it transforms YARN
> internal concepts and boxes them according to CSI protocol. Then forward the
> call to a CSI driver. The adaptor should support both
> controller/node/identity services.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
