HDDS-3955. Unable to list intermediate paths on keys created using S3G. #1196

bharatviswa504 · 2020-07-10T22:59:57Z

What changes were proposed in this pull request?

Keys created via the S3 Gateway currently use the createKey OM API to create the ozone key. Hence, when using a hdfs client to list intermediate directories in the key, OM returns key not found error. This was encountered while using fluentd to write Hive logs to Ozone via the s3 gateway.

Added a new config in OM to control the behavior of key create to create intermediate directories or not. In this way, keys created via ozone S3G can be accessed through fs API.

What is the link to the Apache JIRA

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

How was this patch tested?

Added UT and IT.

arp7 · 2020-07-13T16:06:42Z

hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java

used via FileSystem API (reword comment).

arp7 · 2020-07-13T16:24:47Z

Can you update the PR template a bit to describe the high-level approach (was fix made in OM or S3G)? 🙂

arp7 · 2020-07-13T16:26:30Z

...e/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java

Let's make the exception message slightly more descriptive to explain what is really going on. We can mention that createIntermediateDirs behavior is enabled and hence / has special interpretation.

...e/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java

arp7 · 2020-07-13T16:30:04Z

The approach looks really good. It is surprisingly concise and elegant because I was expecting it to be a lot more code! Added a few review comments.

mukul1987 · 2020-07-13T16:34:48Z

...one-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCreateRequest.java

Can we change this to fs.getFileStatus(keypath) so that returns status.isDir() ?

avijayanhwx

Thanks for working on this @bharatviswa504. Minor comments inline.

...op-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java

...ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/TestNormalizePaths.java

arp7 · 2020-07-16T00:47:00Z

hadoop-hdds/common/src/main/resources/ozone-default.xml

I would rephrase first sentence a bit to state that key names will be interpreted as file system paths. / will be treated as a special character and paths will be normalized and must follow Unix filesystem path naming conventions.

...op-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java

arp7 · 2020-07-16T17:42:19Z

...ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java

Leading / in path should be dropped at this point, right?

Yes. This change was required, when we are allowing leading / in keyName. I will revert the change

arp7 · 2020-07-16T17:43:56Z

...ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java

I am a little bit worried about changes to OMFileRequest. There is a risk of changing the behavior and invalidating the app-compat testing we have done so far.

This change can be reverted as now we don't allow leading / in path names when OMConfig enable.filesystems is set to true. Will updated in my next PR update.

bharatviswa504 · 2020-07-16T21:56:20Z

Thank You @arp7 and @avijayanhwx for the review.
I have addressed review comments.

This PR only modifies behavior of KeyCreate when ozone.om.enable.filesystem.paths is enabled. (Not changed the default behavior of actual KeyCreate)

bharatviswa504 · 2020-07-17T17:44:16Z

Rebased with latest apache main branch.

avijayanhwx

Thanks for working on this @bharatviswa504. I have tested the latest version of the patch against a cluster where s3g is writing to OM with ozone.om.enable.filesystem.paths enabled.

...op-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java

...ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java

...e/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java

bharatviswa504 · 2020-07-17T22:02:30Z

Few minor comments, and couple of questions on the expected normalizations.

Overall the change looks good. Thanks for adding comprehensive test cases.

Thank You @arp7 for the review. addressed review comments.

arp7

+1

* master: HDDS-3984. Support filter and search the columns in recon UI (apache#1218) HDDS-3806. Support recognize aws v2 Authorization header. (apache#1098) HDDS-3955. Unable to list intermediate paths on keys created using S3G. (apache#1196) HDDS-3741. Reload old OM state if Install Snapshot from Leader fails (apache#1129) HDDS-3965. SCM failed to start up for duplicated pipeline detected. (apache#1210) HDDS-3855. Add upgrade smoketest (apache#1142) HDDS-3964. Ratis config key mismatch (apache#1204) HDDS-3612. Allow mounting bucket under other volume (apache#1104) HDDS-3926. OM Token Identifier table should use in-house serialization. (apache#1182) HDDS-3824: OM read requests should make SCM#refreshPipeline outside BUCKET_LOCK (apache#1164) HDDS-3966. Disable flaky TestOMRatisSnapshots

* HDDS-3869: HDDS-3984. Support filter and search the columns in recon UI (apache#1218) HDDS-3806. Support recognize aws v2 Authorization header. (apache#1098) HDDS-3955. Unable to list intermediate paths on keys created using S3G. (apache#1196) HDDS-3741. Reload old OM state if Install Snapshot from Leader fails (apache#1129) HDDS-3965. SCM failed to start up for duplicated pipeline detected. (apache#1210)

* master: HDDS-3984. Support filter and search the columns in recon UI (apache#1218) HDDS-3806. Support recognize aws v2 Authorization header. (apache#1098) HDDS-3955. Unable to list intermediate paths on keys created using S3G. (apache#1196) HDDS-3741. Reload old OM state if Install Snapshot from Leader fails (apache#1129) HDDS-3965. SCM failed to start up for duplicated pipeline detected. (apache#1210)

…G. (apache#1196)

* add-deleted-block-table: (63 commits) Make block iterator tests use deleted blocks table, and remove the now unused #deleted# Replace uses of #deleted# key prefix with access to new deleted blocks table Add deleted blocks table to base level DB wrappers Have block deleting service test look for #deleted# keys in metadata table Move block delete to correct table and remove debugging print statement Import schema version when importing container data from export HDDS-3984. Support filter and search the columns in recon UI (apache#1218) HDDS-3806. Support recognize aws v2 Authorization header. (apache#1098) HDDS-3955. Unable to list intermediate paths on keys created using S3G. (apache#1196) HDDS-3741. Reload old OM state if Install Snapshot from Leader fails (apache#1129) Move new key value block iterator implementation and tests to new interface Fix checkstyle violations HDDS-3965. SCM failed to start up for duplicated pipeline detected. (apache#1210) Update comments Add comments on added helper method Remove seekToLast() from iterator interface, implementation, and tests Add more robust unit test with alternating key matches All unit tests pass after allowing keys with deleted and deleting prefixes to be made HDDS-3855. Add upgrade smoketest (apache#1142) HDDS-3964. Ratis config key mismatch (apache#1204) ...

…G. (#1196) (cherry picked from commit 715aed2)

…G. (#1196)

…G. (apache#1196)

arp7 reviewed Jul 13, 2020

View reviewed changes

...e/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java Outdated Show resolved Hide resolved

mukul1987 reviewed Jul 13, 2020

View reviewed changes

bharatviswa504 force-pushed the keyput branch 2 times, most recently from bc2ca69 to 51c6a90 Compare July 14, 2020 04:47

bharatviswa504 changed the title ~~Keyput~~ HDDS-3955. Unable to list intermediate paths on keys created using S3G. Jul 14, 2020

avijayanhwx added the 0.6.0 label Jul 14, 2020

bharatviswa504 force-pushed the keyput branch from aeae1bb to d173db0 Compare July 15, 2020 00:23

bharatviswa504 marked this pull request as ready for review July 15, 2020 00:24

bharatviswa504 added the om label Jul 15, 2020

avijayanhwx reviewed Jul 15, 2020

View reviewed changes

bharatviswa504 force-pushed the keyput branch from 9028b91 to bff5c4a Compare July 15, 2020 22:55

arp7 reviewed Jul 16, 2020

View reviewed changes

...op-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java Outdated Show resolved Hide resolved

arp7 reviewed Jul 16, 2020

View reviewed changes

bharatviswa504 force-pushed the keyput branch from 7e45327 to 6e0cd7a Compare July 16, 2020 21:55

bharatviswa504 requested review from arp7, avijayanhwx and mukul1987 July 16, 2020 21:57

bharatviswa504 force-pushed the keyput branch from c9194af to 285d0db Compare July 17, 2020 17:43

avijayanhwx approved these changes Jul 17, 2020

View reviewed changes

arp7 reviewed Jul 17, 2020

View reviewed changes

...op-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java Outdated Show resolved Hide resolved

arp7 reviewed Jul 17, 2020

View reviewed changes

...ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java Outdated Show resolved Hide resolved

arp7 reviewed Jul 17, 2020

View reviewed changes

...e/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java Outdated Show resolved Hide resolved

bharatviswa504 added 17 commits July 17, 2020 14:05

address review comments

afb0f5c

cs

da4ed94

filesystem api

010da1b

cs

d916ee4

address leading / to follow fs semantics

c020f4a

add disable test

28b7d7e

cs and fb

8c85c11

fb

711343a

some corner case tests address review comments and handle empty keyName

1e55f1e

add / test

edd38aa

make empty key normal to key create also

8458f67

remove file request code

75780f0

add validatecheck and add tests

bcd5b78

address review comment

5f8e72a

cs

083c224

remove spurious change

301b6d0

add test

f0389ac

bharatviswa504 force-pushed the keyput branch from 629563c to f0389ac Compare July 17, 2020 21:06

fix bug during rebase

15b9e5c

arp7 approved these changes Jul 18, 2020

View reviewed changes

arp7 merged commit 715aed2 into apache:master Jul 18, 2020

errose28 pushed a commit to errose28/ozone that referenced this pull request Jul 21, 2020

HDDS-3955. Unable to list intermediate paths on keys created using S3…

7acfb12

…G. (apache#1196)

ChenSammi pushed a commit that referenced this pull request Jul 22, 2020

HDDS-3955. Unable to list intermediate paths on keys created using S3…

ba8966d

…G. (#1196) (cherry picked from commit 715aed2)

timmylicheng pushed a commit that referenced this pull request Aug 6, 2020

HDDS-3955. Unable to list intermediate paths on keys created using S3…

51a3582

…G. (#1196)

rakeshadr pushed a commit to rakeshadr/hadoop-ozone that referenced this pull request Sep 3, 2020

HDDS-3955. Unable to list intermediate paths on keys created using S3…

cb349ec

…G. (apache#1196)

HDDS-3955. Unable to list intermediate paths on keys created using S3G. #1196

HDDS-3955. Unable to list intermediate paths on keys created using S3G. #1196

Uh oh!

Conversation

bharatviswa504 commented Jul 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

arp7 Jul 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arp7 commented Jul 13, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

arp7 commented Jul 13, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

avijayanhwx left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bharatviswa504 commented Jul 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bharatviswa504 commented Jul 17, 2020

Uh oh!

avijayanhwx left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bharatviswa504 commented Jul 17, 2020

Uh oh!

arp7 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

bharatviswa504 commented Jul 10, 2020 •

edited

Loading

arp7 Jul 13, 2020 •

edited

Loading

bharatviswa504 commented Jul 16, 2020 •

edited

Loading