[
https://issues.apache.org/jira/browse/YARN-7988?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16382946#comment-16382946
]
Naganarasimha G R commented on YARN-7988:
-----------------------------------------
Thanks for working on the patch [~bibinchundatt]
May be little write up on the class organization would have been a help here as
it involves lot of generics. And also what gets reused in terms of Attributes
Few comments :
FSNodeStoreLogOp.java
* ln no 46, Documentation required for understanding a class with generics.
Else we will be lost assuming what they represent
* ln no 46, Also instead of W,R & M can we have OUT, IN, MGR ? or even more
specific that OUT extends FSDataOutputStream & IN extends FSDataInputStream.
Also are we expecting anything else over here ?
* ln no 46, If this class is specific to labels then we should better have it
as FSNodeLabelStoreLogOp? abstract is only used for factoring here.
* ln no 53,54,57,58,59,70,100,139,179 Generics havent been used to the
fullest and hence eclipse shows warnings @ these lines. Some can be easily
avoided.
* .ln no 70, getStoreOp is not used anywhere, is it required?
FileSystemNodeLabelsStore.java
* ln no 44, AbstractFSNodeStore cant be parameterised here ? I think we can
have the specifix manager we want here
FSNodeLogOpCode.java
* Do we require this static operation ? cant we make use of enum's values()
method directly ? and also given the string name of enum we can get the
specified Enum.
FSNodeStoreMirrorOp.java
* do we require it to be abstract ? we are not doing any common functionality
other than having factory kind of interface here and also IMO NodeLabelMirrorOp
should be class alone and not embedded here
StoreOp.java
* i think we require one more interface lile _FileStoreOP<M> extends
StoreOp<FSDataOutputStream, FSDataInputStream, M>_ to make other extending
classes simplified
Also we have all the new classes under
"org.apache.hadoop.yarn.nodelabels.store" package may be better to add it under
some other package name ? And as well PackageInfo.Java is missing for the new
package
> Refactor FSNodeLabelStore code for attributes store support
> -----------------------------------------------------------
>
> Key: YARN-7988
> URL: https://issues.apache.org/jira/browse/YARN-7988
> Project: Hadoop YARN
> Issue Type: Sub-task
> Reporter: Bibin A Chundatt
> Assignee: Bibin A Chundatt
> Priority: Major
> Attachments: YARN-7988-YARN-3409.002.patch, YARN-7988.001.patch
>
>
> # Abstract out file FileSystemStore operation
> # Define EditLog Operartions and Mirror operation
> # Support compatibility with old nodelabel store
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
