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: [email protected]
For additional commands, e-mail: [email protected]
