HDDS-7594. [FSO] Folders created through S3G are created on file system as "files". #4186
Conversation
|
@duongkame @swamirishi can you please take a look at this PR? |
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/S3GatewayConfigKeys.java
Outdated
Show resolved
Hide resolved
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java
Outdated
Show resolved
Hide resolved
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java
Outdated
Show resolved
Hide resolved
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java
Outdated
Show resolved
Hide resolved
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectPut.java
Show resolved
Hide resolved
|
@errose28 thank you for the review! |
mladjan-gadzic
requested review from
errose28
and removed request for
kerneltime
|
@errose28 I've addressed all the comments. Could you please rereview it? |
There was a problem hiding this comment.
Thanks for the updates @mladjan-gadzic I left some minor comments in line.
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/S3GatewayConfigKeys.java
Show resolved
Hide resolved
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectPut.java
Outdated
Show resolved
Hide resolved
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectPut.java
Outdated
Show resolved
Hide resolved
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectPut.java
Outdated
Show resolved
Hide resolved
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/exception/S3ErrorTable.java
Show resolved
Hide resolved
|
@errose28 thank you for re-review! |
There was a problem hiding this comment.
Thanks for the updates @mladjan-gadzic. Changes LGTM but can you investigate the Copy Object acceptance tests? The failures may be related.
I've checked that already. CI passed on the remote but failed on the upstream. I'd say we are on the safe side. |
|
@mladjan-gadzic The same tests have failed twice in a row here. Each run is actually running the same test as part of Can you check the logs once the artifacts for the latest run becomes available to try to root cause the failure so we will know whether it is related or not? Unfortunately I can't merge this with the same test failing consistently on the PR branch. |
|
@errose28 I will check it further. What is a mystery to me right now is that the same tests passed for acceptance (secure) but failed for acceptance (unsecure) and acceptance (HA). What is also interesting is that for one commit before the latest CI was green but it is red for refactor changes on latest one. |
Each acceptance test grouping kind, be it secured, unsecured or HA, run a different set of s3 acceptance tests. Because of this, the failing s3 copy-object tests are occuring for some reason in the unsecured and HA s3 test suites with the source bucket for the copy either an EC bucket (unsecure) or a random bucket (HA). The passing s3 copy-object tests for the secure cluster are using encrypted, link or generated buckets, however these are not tested in the failing unsecure s3 acceptance test suite since it fails on erasure coded buckets. Interested to find the source of the problem here. |
| getClientProtocol() | ||
| .createDirectory(volume.getName(), bucketName, keyPath); | ||
| return Response.ok().status(HttpStatus.SC_OK).build(); | ||
| } |
There was a problem hiding this comment.
Thank @mladjan-gadzic . Minor change. This added block above checking the length, FSO layout and handling the fso directory create (L203-213), should be placed after the block below handling the copy header and CopyObject L217-226. After re-run the CI workflow.
There was a problem hiding this comment.
Thanks @mladjan-gadzic , ran the CI again after your green CI and saw all tests passed.
|
Thank you @neils-dev for suggestions, review and re-running CI! |
|
@errose28 any further comments? Going to merge. Thanks. |
There was a problem hiding this comment.
Thanks for helping debug the failures @neils-dev and for working on the change @mladjan-gadzic
What changes were proposed in this pull request?
Instead of creating 0 byte file, directory is going to be created.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-7594
How was this patch tested?