Thanks Marton. I have added a few new sub tasks under HDDS-2665 uber jira. Surely let's discuss the todos before the next release.
I have just kicked off a 6th run on the PR#1021 for checks. So far only 1 of the 5 runs passed all tests (except the false alarm from SonarCloud), the rest of the runs all have some flaky tests unrelated to OFS. -Siyao 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 > >
