elek commented on pull request #1363:
URL: https://github.com/apache/hadoop-ozone/pull/1363#issuecomment-694144300


   > I'm thinking of renaming BasicRootedOzoneClientAdapterImpl to 
BasicRootedOzoneFileSystemHelper. This way the class name should make more 
sense?
   
   Definitely better, IMHO. It's not clear why we need to move out some 
functions to a helper class (`"Can you please explain what are the differences 
between the two classes and the responsibilities?"`), but I can live with it, 
just to merge the patch earlier.
   
   > The class is dealing with OzoneFSStorageStatistics, but not used anywhere. 
The o3fs counterpart is OzoneClientAdapterImpl, which is used in 
OzoneFileSystem.
   
   This is only about the statistics. Independent to what type of classes do 
you have: 
   
    You need a `OzoneFSStorageStatistics storageStatistics` which is updated 
for each of the operations, but only for Hadoop3 (!!!).
   
   For the old-school `o3fs` it's done by subclasses:
   
    * Default implementation is hadoop2 compatibility as `incdementCounter` 
does nothing
    * non-Basic implementation increments a real counter.
   
   As far as I see we already have this logic for `ofs` as the imlementation of 
`RootedOzoneFileSystem` in `hadoop2` and `hadoop3` projects are different. 
(Later one update the statistics.)
   
   If your helper class can be used from both project without problem, you 
don't need to create two helper classses for the two use-cases. 
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org

Reply via email to