Thanks Xiaoyu for the +1. - I have created a OFS user guide jira HDDS-3803. I'll add /tmp doc under that jira. - getTrashRoots(), which is a prerequisite of trash cleanup, will be done in HDDS-3705. - OFS ACL work tracking jira: HDDS-2991 - I'm working on the refactoring. One of the jira: HDDS-3805
-Siyao On Fri, Jun 12, 2020 at 11:25 AM Xiaoyu Yao <x...@cloudera.com.invalid> wrote: > Thanks Siyao for adding this useful feature. I reviewed several PRs on the > feature branch > and feel positive to merge it master. Here are a few suggestions: > > 1. /tmp mount needs better document: > Anyone can create a bucket under global /tmp volume in addition to > their personal mounted /tmp when using o3 protocol? The two additional /tmp > provision steps > seem to be different from using HDFS. We should document it clearly. > > 2. Some follow up work on trash clean up implementation on OM. That can be > added after the merge. > > 3. Some missing features from both o3fs and ofs, such as ACL. This need > further discussion to > map HCFS acl to ozone acl. I'm OK with adding it post merge. > > 4. Some refactor work to dedup the FS implementation between ofs and o3fs. > > Overall, I'm +1 on merge. > > Thanks, > Xiaoyu > > On Fri, Jun 12, 2020 at 12:02 AM Elek, Marton <e...@apache.org> wrote: > > > > > > Yes I admit there are quite a few duplicated lines of code. And this > is a > > > problem that needs to be solved sooner or later. > > > In the early stage of OFS development I discussed this with @Xiaoyu. We > > > figured this could be resolved "later" -- which means now or soon after > > > merge. > > > At that time (beginning of the feature branch) I was trying to touch as > > few > > > existing classes as possible to minimize the conflict when rebasing the > > > feature branch. This is also partially the reason for the duplication. > > Also > > > the plan didn't work out as expected. > > > > Thanks to explain it. I am fine with doing it after a merge, if later > > means near-future/before the next release ;-) > > > > And I understand the motivation to keep the o3fs untouched, but I would > > like to encourage you: feel free to modify old code, too. > > > > > > >> We discussed in PR the BasicRootedOzoneFileSystem#adapterImpl can be > > > removed by exposing getVolume() in OzoneClientAdapter. > > > https://github.com/apache/hadoop-ozone/pull/1021#discussion_r436749198 > > > > > > *ClientAdapters are created to separate OzoneClient/ObjectStore from the > > OzoneFileSystem to make it possible to use the FilteredClassloader. > > > > As the classloader is removed we can consider to remove ClientAdapters, > > too (at least from the new code). I think some of my concerns are rooted > > in the approach which tried to keep the previous structure. Again: feel > > free to do refactors if it makes it simpler / cleaner code. > > > > I also learned that the ClientProxy might be required to get better > > performance (ObjectStore/getVolume/getBucket executes a new GetBucket > > request all the time). > > > > As of now it is marked as @VisibleForTesting. We might need to remove > > the annotations and use it as now. > > > > > > Summary: I am fine with merging it to the master (it couldn't break > > anything), but I would prefer to continue the discussion and have an > > agreement what should we do before the next release... > > > > And I propose to run at least 4-5 builds in #1021 and analyze all the > > failures (!), before the merge (As I understood > > 2eb3181b552824ea8a5e5956387e4c8b540b97c9 from the #1021 is the proposed > > head to merge) > > > > Thanks, again, all the great work > > Marton > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: ozone-dev-unsubscr...@hadoop.apache.org > > For additional commands, e-mail: ozone-dev-h...@hadoop.apache.org > > > > >
