HDDS-2665. Implement new Ozone Filesystem scheme ofs:// #1021

smengcl · 2020-06-04T16:07:29Z

What changes were proposed in this pull request?

Implement a new scheme for Ozone Filesystem where all volumes (and buckets) can be accessed from a single root.

Also known as Rooted Ozone Filesystem.

This PR combines commits in feature branch HDDS-2665-ofs for review and discussion.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-2665

How was this patch tested?

Added FileSystem contract tests for ofs://.

Added new integration tests.

This reverts commit 79c0c1c.

…tributed by Siyao Meng.

Conflicts: hadoop-ozone/dist/src/main/smoketest/ozonefs/ozonefs.robot

Conflicts: (HDDS-3385) hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneShellHA.java

smengcl · 2020-06-05T21:05:08Z

Will open up a new jira to fix sonarcloud bugs, and address merge conflicts.

elek

Thanks to drive this effort @smengcl. Overall it looks good to me. I agree that we are in the last step before the merge.

I have some questions about the code.

(And I feel myself guilty of the conflict, I can explain the changes on the master what I did, or I can help to rebase it.)

...one/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java

elek · 2020-06-08T14:12:53Z

...one/ozonefs/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystemWithMocks.java

+@RunWith(PowerMockRunner.class)
+@PrepareForTest({ OzoneClientFactory.class, UserGroupInformation.class })
+@PowerMockIgnore("javax.management.*")
+public class TestRootedOzoneFileSystemWithMocks {


FYI: there is a full, in memory implementation of ObjectStore in s3 project. Can be useful for similar tests if we move it to a common place. (BTW: I like this lightweight test).

I removed TestRootedOzoneFileSystemWithMocks in HDDS-3767 since TestOzoneFileSystemWithMocks is also removed. We can restore this later.

hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java

elek · 2020-06-08T14:24:22Z

hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java

+        if (recursive) {
+          // Delete all buckets first
+          OzoneVolume volume =
+              adapterImpl.getObjectStore().getVolume(volumeName);


The main idea behind the adapter is that we can use only methods on the adapter which provides a clean definition of the used ozone method. I would the usage of getObjectStore() as it leaks the internal methods.

The original goal of adapter to support different classloaders, but still seems to be a good design pattern to use an adapter.getVolume instead.

Yes I agree. When implementing OFS volume and bucket deletion I tried to put the logic in adapter, but I realized if I want to do it the easy way (using recursion) I simply can't put the logic in adapter. Hence the hacky one you see here.

I will refactor this chunk of code and remove adapterImpl in another refactoring jira later. But I think this shouldn't affect the merge.

smengcl · 2020-06-09T16:26:43Z

Thanks to drive this effort @smengcl. Overall it looks good to me. I agree that we are in the last step before the merge.

I have some questions about the code.

(And I feel myself guilty of the conflict, I can explain the changes on the master what I did, or I can help to rebase it.)

Thanks for the comment @elek .

The merge conflict comes from HDDS-3627 (commit) as far as I can tell. Shouldn't be a big problem. It is always a delight to see good refactoring. :)

A question though. I notice TestOzoneFileSystemWithMocks being removed in HDDS-3627, where in OFS I forked it to create TestRootedOzoneFileSystemWithMocks. Should I relocate the latter to somewhere else or just remove it as well? For now I will move it under hadoop-ozone/ozonefs-common/src/test/java/org/apache/hadoop/fs/ozone/ to be in the same place with TestOzoneFSInputStream.java. The merge conflict resolution work is being done in HDDS-3767.

Conflicts: hadoop-ozone/ozonefs-hadoop2/src/main/java/org/apache/hadoop/fs/ozone/TestOFSPath.java hadoop-ozone/ozonefs-hadoop2/src/main/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystemWithMocks.java hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/OzoneClientAdapterFactory.java

smengcl · 2020-06-09T17:33:12Z

Some of the bugs that showed up in SonarCloud doesn't relate to OFS (e.g. BasicOzoneFileSystem/BasicOzoneClientAdapterImpl).

~~I will only be addressing the ones in OFS. i.e. BasicRootedOzoneClientAdapterImpl in HDDS-3767.~~

A closer look reveals that the bug isn't really a bug. If I "address" it by closing ozoneOutputStream in finally, the returning OzoneFSOutputStream will contain a stream that is already closed.

Closes #1046

codecov-commenter · 2020-06-11T17:46:53Z

Codecov Report

Merging #1021 into master will increase coverage by 0.08%.
The diff coverage is 71.82%.

@@             Coverage Diff              @@
##             master    #1021      +/-   ##
============================================
+ Coverage     70.48%   70.57%   +0.08%     
- Complexity     9260     9405     +145     
============================================
  Files           961      965       +4     
  Lines         48177    48979     +802     
  Branches       4678     4790     +112     
============================================
+ Hits          33959    34565     +606     
- Misses        11968    12104     +136     
- Partials       2250     2310      +60

Impacted Files	Coverage Δ	Complexity Δ
...main/java/org/apache/hadoop/ozone/OzoneConsts.java	`84.21% <ø> (ø)`	`1.00 <0.00> (ø)`
...e/hadoop/fs/ozone/BasicOzoneClientAdapterImpl.java	`70.05% <0.00%> (-0.38%)`	`28.00 <0.00> (ø)`
...g/apache/hadoop/fs/ozone/BasicOzoneFileSystem.java	`75.24% <ø> (ø)`	`51.00 <0.00> (ø)`
.../hadoop/fs/ozone/RootedOzoneClientAdapterImpl.java	`41.66% <41.66%> (ø)`	`2.00 <2.00> (?)`
...op/fs/ozone/BasicRootedOzoneClientAdapterImpl.java	`68.45% <68.45%> (ø)`	`47.00 <47.00> (?)`
...he/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java	`74.40% <74.40%> (ø)`	`50.00 <50.00> (?)`
.../main/java/org/apache/hadoop/fs/ozone/OFSPath.java	`79.59% <79.59%> (ø)`	`37.00 <37.00> (?)`
...hdds/scm/container/common/helpers/ExcludeList.java	`83.67% <0.00%> (-14.29%)`	`20.00% <0.00%> (-4.00%)`
...che/hadoop/hdds/scm/pipeline/PipelineStateMap.java	`83.04% <0.00%> (-4.10%)`	`45.00% <0.00%> (-3.00%)`
...ent/algorithms/SCMContainerPlacementRackAware.java	`76.69% <0.00%> (-3.01%)`	`31.00% <0.00%> (-2.00%)`
... and 23 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c5b0ba6...04dc11c. Read the comment docs.

smengcl · 2020-06-12T16:53:59Z

The only test failure after HDDS-3767 merge is it-hdds-om TestOzoneManagerHAWithData#testOMRestart, which is a flaky test also seen on master branch: https://elek.github.io/ozone-build-results/
Thanks Marton for this useful page.

smengcl · 2020-06-15T16:11:10Z

Thanks @elek for kicking off another 2 runs. I just pushed another empty commit for a new run.

We might want to exclude (by rebasing) the empty commits from feature branch when we merge is to master? @elek
My plan is to merge the HDDS-2665-ofs branch to master by running

git merge --no-ff HDDS-2665-ofs

when on master branch, as described in the Merging a feature branch section on [this Hadoop Wiki page|https://cwiki.apache.org/confluence/display/HADOOP2/HowToCommit].

sonarcloud · 2020-06-16T18:21:12Z

SonarCloud Quality Gate failed.

2 Bugs
0 Vulnerabilities (and 1 Security Hotspot to review)
38 Code Smells

76.1% Coverage
16.9% Duplication

The version of Java (1.8.0_232) you have used to run this analysis is deprecated and we will stop accepting it from October 2020. Please update to at least Java 11.
Read more here

smengcl · 2020-06-16T18:39:41Z

The last run (merge commit) is good. Will manually merge feature branch HDDS-2665-ofs shortly and close this PR.

smengcl · 2020-06-16T19:18:46Z

I'm going to force push to the feature branch to remove the empty commits. All 7 runs, for the record:

smengcl and others added 20 commits January 30, 2020 12:45

HDDS-2840. Implement ofs://: mkdir (#415)

20bb5b5

HDDS-2665. Merge master to HDDS-2665-ofs branch (#511)

79c0c1c

Revert "HDDS-2665. Merge master to HDDS-2665-ofs branch (#511)"

3d39723

This reverts commit 79c0c1c.

Merge branch 'master' into HDDS-2665

ec9986b

HDDS-2665. Addendum fix for merge master due to HDDS-2188 revert. Con…

87288a2

…tributed by Siyao Meng.

HDDS-2979. Implement ofs://: Fix getFileStatus for mkdir volume (#528)

3e4782d

HDDS-2928. Implement ofs://: listStatus (#547)

7c35d76

HDDS-3073. Implement ofs://: Fix listStatus continuation (#606)

0610ce3

HDDS-2929. Implement ofs://: temp directory mount (#610)

c410c4d

HDDS-2945. Implement ofs://: Add robot tests for mkdir. (#703)

5616dc3

Merge branch 'master' into HDDS-2665-ofs

3c5ec6a

HDDS-3279. Rebase OFS branch (#731)

3eaec50

HDDS-3390. Adapt OFSPath to master branch changes (#847)

8356a4a

Merge remote-tracking branch 'asf/master' into HDDS-2665-ofs

dd99223

Conflicts: hadoop-ozone/dist/src/main/smoketest/ozonefs/ozonefs.robot

HDDS-3390. Rebase OFS branch - 2. Adapt OFS classes to HDDS-3101 (#822)

9ed2e15

HDDS-3494. Implement ofs://: Support volume and bucket deletion (#906)

0773a4a

HDDS-3574. Implement ofs://: Override getTrashRoot (#941)

28540cb

HDDS-2969. Implement ofs://: Add contract test (#865)

d6d31c6

Merge remote-tracking branch 'asf/master' into HDDS-2665-ofs

a00dc18

Conflicts: (HDDS-3385) hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneShellHA.java

HDDS-3709. Rebase OFS branch - 3. Adapt to HDDS-3501 (#1015)

b9dac41

nandakumar131 assigned smengcl Jun 4, 2020

nandakumar131 added client enhancement New feature or request labels Jun 4, 2020

elek reviewed Jun 8, 2020

View reviewed changes

smengcl added a commit to smengcl/hadoop-ozone that referenced this pull request Jun 9, 2020

Uncomment for HDDS-3054. apache#1021 (comment)

4019e34

HDDS-3767. [OFS] Address merge conflicts after HDDS-3627

f64315f

Closes #1046

Merge remote-tracking branch 'origin/master' into HDDS-2665-ofs

2eb3181

asfgit force-pushed the HDDS-2665-ofs branch from 04dc11c to 2eb3181 Compare June 16, 2020 19:19

asfgit merged commit 75de083 into master Jun 16, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HDDS-2665. Implement new Ozone Filesystem scheme ofs:// #1021

HDDS-2665. Implement new Ozone Filesystem scheme ofs:// #1021

smengcl commented Jun 4, 2020

smengcl commented Jun 5, 2020

elek left a comment

elek Jun 8, 2020

smengcl Jun 9, 2020

elek Jun 8, 2020

smengcl Jun 9, 2020 •

edited

Loading

smengcl commented Jun 9, 2020 •

edited

Loading

smengcl commented Jun 9, 2020 •

edited

Loading

codecov-commenter commented Jun 11, 2020 •

edited

Loading

smengcl commented Jun 12, 2020 •

edited

Loading

smengcl commented Jun 15, 2020

sonarcloud bot commented Jun 16, 2020

smengcl commented Jun 16, 2020

smengcl commented Jun 16, 2020

HDDS-2665. Implement new Ozone Filesystem scheme ofs:// #1021

HDDS-2665. Implement new Ozone Filesystem scheme ofs:// #1021

Conversation

smengcl commented Jun 4, 2020

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

smengcl commented Jun 5, 2020

elek left a comment

Choose a reason for hiding this comment

elek Jun 8, 2020

Choose a reason for hiding this comment

smengcl Jun 9, 2020

Choose a reason for hiding this comment

elek Jun 8, 2020

Choose a reason for hiding this comment

smengcl Jun 9, 2020 • edited Loading

Choose a reason for hiding this comment

smengcl commented Jun 9, 2020 • edited Loading

smengcl commented Jun 9, 2020 • edited Loading

codecov-commenter commented Jun 11, 2020 • edited Loading

Codecov Report

smengcl commented Jun 12, 2020 • edited Loading

smengcl commented Jun 15, 2020

sonarcloud bot commented Jun 16, 2020

smengcl commented Jun 16, 2020

smengcl commented Jun 16, 2020

smengcl Jun 9, 2020 •

edited

Loading

smengcl commented Jun 9, 2020 •

edited

Loading

smengcl commented Jun 9, 2020 •

edited

Loading

codecov-commenter commented Jun 11, 2020 •

edited

Loading

smengcl commented Jun 12, 2020 •

edited

Loading