You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2021/07/13 21:16:59 UTC
[GitHub] [ozone] smengcl opened a new pull request #2412: HDDS-5279. OFS mkdir -p does not work when Volume is not pre-created
smengcl opened a new pull request #2412:
URL: https://github.com/apache/ozone/pull/2412
https://issues.apache.org/jira/browse/HDDS-5279
`ClientProtocol#getBucketDetails` behavior has been changed. It now can throw `VOLUME_NOT_FOUND` which `getBucket()` didn't handle previously.
Thanks @avijayanhwx for the fix.
might post a UT later.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] kerneltime commented on a change in pull request #2412: HDDS-5279. OFS mkdir -p does not work when Volume is not pre-created
Posted by GitBox <gi...@apache.org>.
kerneltime commented on a change in pull request #2412:
URL: https://github.com/apache/ozone/pull/2412#discussion_r671772102
##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -223,31 +223,23 @@ private OzoneBucket getBucket(String volumeStr, String bucketStr,
try {
bucket = proxy.getBucketDetails(volumeStr, bucketStr);
} catch (OMException ex) {
- // Note: always create bucket if volumeStr matches "tmp" so -put works
if (createIfNotExist) {
- // Note: getBucketDetails always throws BUCKET_NOT_FOUND, even if
- // the volume doesn't exist.
- if (ex.getResult().equals(BUCKET_NOT_FOUND)) {
- OzoneVolume volume;
+ // getBucketDetails can throw VOLUME_NOT_FOUND when the parent volume
+ // doesn't exist and ACL is enabled; it can only throw BUCKET_NOT_FOUND
+ // when ACL is disabled. Both exceptions need to be handled.
+ switch (ex.getResult()) {
+ case VOLUME_NOT_FOUND:
+ case BUCKET_NOT_FOUND:
Review comment:
The case where volume is not found should not be as common as bucket not found. Would wrapping the processing in a loop and handling volume and bucket separately be better than always trying volume creation?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] kerneltime commented on a change in pull request #2412: HDDS-5279. OFS mkdir -p does not work when Volume is not pre-created
Posted by GitBox <gi...@apache.org>.
kerneltime commented on a change in pull request #2412:
URL: https://github.com/apache/ozone/pull/2412#discussion_r672584791
##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -223,31 +223,23 @@ private OzoneBucket getBucket(String volumeStr, String bucketStr,
try {
bucket = proxy.getBucketDetails(volumeStr, bucketStr);
} catch (OMException ex) {
- // Note: always create bucket if volumeStr matches "tmp" so -put works
if (createIfNotExist) {
- // Note: getBucketDetails always throws BUCKET_NOT_FOUND, even if
- // the volume doesn't exist.
- if (ex.getResult().equals(BUCKET_NOT_FOUND)) {
- OzoneVolume volume;
+ // getBucketDetails can throw VOLUME_NOT_FOUND when the parent volume
+ // doesn't exist and ACL is enabled; it can only throw BUCKET_NOT_FOUND
+ // when ACL is disabled. Both exceptions need to be handled.
+ switch (ex.getResult()) {
+ case VOLUME_NOT_FOUND:
+ case BUCKET_NOT_FOUND:
Review comment:
Do you want to create a newbie jira to clean up the behavior to send the appropriate error which is the right fix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a change in pull request #2412: HDDS-5279. OFS mkdir -p does not work when Volume is not pre-created
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2412:
URL: https://github.com/apache/ozone/pull/2412#discussion_r674215347
##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -223,31 +223,23 @@ private OzoneBucket getBucket(String volumeStr, String bucketStr,
try {
bucket = proxy.getBucketDetails(volumeStr, bucketStr);
} catch (OMException ex) {
- // Note: always create bucket if volumeStr matches "tmp" so -put works
if (createIfNotExist) {
- // Note: getBucketDetails always throws BUCKET_NOT_FOUND, even if
- // the volume doesn't exist.
- if (ex.getResult().equals(BUCKET_NOT_FOUND)) {
- OzoneVolume volume;
+ // getBucketDetails can throw VOLUME_NOT_FOUND when the parent volume
+ // doesn't exist and ACL is enabled; it can only throw BUCKET_NOT_FOUND
+ // when ACL is disabled. Both exceptions need to be handled.
+ switch (ex.getResult()) {
+ case VOLUME_NOT_FOUND:
+ case BUCKET_NOT_FOUND:
Review comment:
Yes I will file one. but probably not be a newbie one since changing that could involve a series of changes -- throwing a different exception can break other existing behaviors.
##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -223,31 +223,23 @@ private OzoneBucket getBucket(String volumeStr, String bucketStr,
try {
bucket = proxy.getBucketDetails(volumeStr, bucketStr);
} catch (OMException ex) {
- // Note: always create bucket if volumeStr matches "tmp" so -put works
if (createIfNotExist) {
- // Note: getBucketDetails always throws BUCKET_NOT_FOUND, even if
- // the volume doesn't exist.
- if (ex.getResult().equals(BUCKET_NOT_FOUND)) {
- OzoneVolume volume;
+ // getBucketDetails can throw VOLUME_NOT_FOUND when the parent volume
+ // doesn't exist and ACL is enabled; it can only throw BUCKET_NOT_FOUND
+ // when ACL is disabled. Both exceptions need to be handled.
+ switch (ex.getResult()) {
+ case VOLUME_NOT_FOUND:
+ case BUCKET_NOT_FOUND:
Review comment:
Yes I will file one. but probably not a newbie one since changing that could involve a series of changes -- throwing a different exception can break other existing behaviors.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a change in pull request #2412: HDDS-5279. OFS mkdir -p does not work when Volume is not pre-created
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2412:
URL: https://github.com/apache/ozone/pull/2412#discussion_r672513719
##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -223,31 +223,23 @@ private OzoneBucket getBucket(String volumeStr, String bucketStr,
try {
bucket = proxy.getBucketDetails(volumeStr, bucketStr);
} catch (OMException ex) {
- // Note: always create bucket if volumeStr matches "tmp" so -put works
if (createIfNotExist) {
- // Note: getBucketDetails always throws BUCKET_NOT_FOUND, even if
- // the volume doesn't exist.
- if (ex.getResult().equals(BUCKET_NOT_FOUND)) {
- OzoneVolume volume;
+ // getBucketDetails can throw VOLUME_NOT_FOUND when the parent volume
+ // doesn't exist and ACL is enabled; it can only throw BUCKET_NOT_FOUND
+ // when ACL is disabled. Both exceptions need to be handled.
+ switch (ex.getResult()) {
+ case VOLUME_NOT_FOUND:
+ case BUCKET_NOT_FOUND:
Review comment:
Thanks for the comment @kerneltime .
`VOLUME_NOT_FOUND` is actually frequently encountered during testing where we create volume+bucket+directories on one shot with `mkdir -p` when ACL is enabled.
If you think about it, in both cases the action(s) we need to take is the same. i.e. volume needs to be created.
It's just that the exception thrown from the call `proxy.getBucketDetails` is different -- which is bizzare, and we maybe want to fix `getBucketDetails`'s behavior at some point (for example, to always throw `VOLUME_NOT_FOUND` regardless of whether ACL is enabled or not), after which we can throw away `case BUCKET_NOT_FOUND`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a change in pull request #2412: HDDS-5279. OFS mkdir -p does not work when Volume is not pre-created
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2412:
URL: https://github.com/apache/ozone/pull/2412#discussion_r672513719
##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -223,31 +223,23 @@ private OzoneBucket getBucket(String volumeStr, String bucketStr,
try {
bucket = proxy.getBucketDetails(volumeStr, bucketStr);
} catch (OMException ex) {
- // Note: always create bucket if volumeStr matches "tmp" so -put works
if (createIfNotExist) {
- // Note: getBucketDetails always throws BUCKET_NOT_FOUND, even if
- // the volume doesn't exist.
- if (ex.getResult().equals(BUCKET_NOT_FOUND)) {
- OzoneVolume volume;
+ // getBucketDetails can throw VOLUME_NOT_FOUND when the parent volume
+ // doesn't exist and ACL is enabled; it can only throw BUCKET_NOT_FOUND
+ // when ACL is disabled. Both exceptions need to be handled.
+ switch (ex.getResult()) {
+ case VOLUME_NOT_FOUND:
+ case BUCKET_NOT_FOUND:
Review comment:
Thanks for the comment @kerneltime .
`VOLUME_NOT_FOUND` is actually frequently encountered during our testing where we create volume+bucket+directories on one shot with `-p` when ACL is enabled.
If you think about it, in both cases the action(s) we need to take is the same. i.e. volume needs to be created.
It's just that the exception thrown from the call `proxy.getBucketDetails` is different -- which is bizzare, and we maybe want to fix `getBucketDetails`'s behavior at some point (for example, to always throw `VOLUME_NOT_FOUND` regardless of whether ACL is enabled or not).
##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -223,31 +223,23 @@ private OzoneBucket getBucket(String volumeStr, String bucketStr,
try {
bucket = proxy.getBucketDetails(volumeStr, bucketStr);
} catch (OMException ex) {
- // Note: always create bucket if volumeStr matches "tmp" so -put works
if (createIfNotExist) {
- // Note: getBucketDetails always throws BUCKET_NOT_FOUND, even if
- // the volume doesn't exist.
- if (ex.getResult().equals(BUCKET_NOT_FOUND)) {
- OzoneVolume volume;
+ // getBucketDetails can throw VOLUME_NOT_FOUND when the parent volume
+ // doesn't exist and ACL is enabled; it can only throw BUCKET_NOT_FOUND
+ // when ACL is disabled. Both exceptions need to be handled.
+ switch (ex.getResult()) {
+ case VOLUME_NOT_FOUND:
+ case BUCKET_NOT_FOUND:
Review comment:
Thanks for the comment @kerneltime .
`VOLUME_NOT_FOUND` is actually frequently encountered during testing where we create volume+bucket+directories on one shot with `mkdir -p` when ACL is enabled.
If you think about it, in both cases the action(s) we need to take is the same. i.e. volume needs to be created.
It's just that the exception thrown from the call `proxy.getBucketDetails` is different -- which is bizzare, and we maybe want to fix `getBucketDetails`'s behavior at some point (for example, to always throw `VOLUME_NOT_FOUND` regardless of whether ACL is enabled or not), after which we can throw away `case BUCKET_NOT_FOUND`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a change in pull request #2412: HDDS-5279. OFS mkdir -p does not work when Volume is not pre-created
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2412:
URL: https://github.com/apache/ozone/pull/2412#discussion_r672513719
##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -223,31 +223,23 @@ private OzoneBucket getBucket(String volumeStr, String bucketStr,
try {
bucket = proxy.getBucketDetails(volumeStr, bucketStr);
} catch (OMException ex) {
- // Note: always create bucket if volumeStr matches "tmp" so -put works
if (createIfNotExist) {
- // Note: getBucketDetails always throws BUCKET_NOT_FOUND, even if
- // the volume doesn't exist.
- if (ex.getResult().equals(BUCKET_NOT_FOUND)) {
- OzoneVolume volume;
+ // getBucketDetails can throw VOLUME_NOT_FOUND when the parent volume
+ // doesn't exist and ACL is enabled; it can only throw BUCKET_NOT_FOUND
+ // when ACL is disabled. Both exceptions need to be handled.
+ switch (ex.getResult()) {
+ case VOLUME_NOT_FOUND:
+ case BUCKET_NOT_FOUND:
Review comment:
Thanks for the comment @kerneltime .
`VOLUME_NOT_FOUND` is actually frequently encountered during our testing where we create volume+bucket+directories on one shot with `-p` when ACL is enabled.
If you think about it, in both cases the action(s) we need to take is the same. i.e. volume needs to be created.
It's just that the exception thrown from the call `proxy.getBucketDetails` is different -- which is bizzare, and we maybe want to fix `getBucketDetails`'s behavior at some point (for example, to always throw `VOLUME_NOT_FOUND` regardless of whether ACL is enabled or not).
##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -223,31 +223,23 @@ private OzoneBucket getBucket(String volumeStr, String bucketStr,
try {
bucket = proxy.getBucketDetails(volumeStr, bucketStr);
} catch (OMException ex) {
- // Note: always create bucket if volumeStr matches "tmp" so -put works
if (createIfNotExist) {
- // Note: getBucketDetails always throws BUCKET_NOT_FOUND, even if
- // the volume doesn't exist.
- if (ex.getResult().equals(BUCKET_NOT_FOUND)) {
- OzoneVolume volume;
+ // getBucketDetails can throw VOLUME_NOT_FOUND when the parent volume
+ // doesn't exist and ACL is enabled; it can only throw BUCKET_NOT_FOUND
+ // when ACL is disabled. Both exceptions need to be handled.
+ switch (ex.getResult()) {
+ case VOLUME_NOT_FOUND:
+ case BUCKET_NOT_FOUND:
Review comment:
Thanks for the comment @kerneltime .
`VOLUME_NOT_FOUND` is actually frequently encountered during testing where we create volume+bucket+directories on one shot with `mkdir -p` when ACL is enabled.
If you think about it, in both cases the action(s) we need to take is the same. i.e. volume needs to be created.
It's just that the exception thrown from the call `proxy.getBucketDetails` is different -- which is bizzare, and we maybe want to fix `getBucketDetails`'s behavior at some point (for example, to always throw `VOLUME_NOT_FOUND` regardless of whether ACL is enabled or not), after which we can throw away `case BUCKET_NOT_FOUND`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl merged pull request #2412: HDDS-5279. OFS mkdir -p does not work when Volume is not pre-created
Posted by GitBox <gi...@apache.org>.
smengcl merged pull request #2412:
URL: https://github.com/apache/ozone/pull/2412
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] bshashikant commented on pull request #2412: HDDS-5279. OFS mkdir -p does not work when Volume is not pre-created
Posted by GitBox <gi...@apache.org>.
bshashikant commented on pull request #2412:
URL: https://github.com/apache/ozone/pull/2412#issuecomment-884993555
@smengcl , correct if i m wrong. Volumes in ozone are supposed to be created by admins only as per the doc. Are we going to break this notion here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] kerneltime commented on a change in pull request #2412: HDDS-5279. OFS mkdir -p does not work when Volume is not pre-created
Posted by GitBox <gi...@apache.org>.
kerneltime commented on a change in pull request #2412:
URL: https://github.com/apache/ozone/pull/2412#discussion_r672584791
##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -223,31 +223,23 @@ private OzoneBucket getBucket(String volumeStr, String bucketStr,
try {
bucket = proxy.getBucketDetails(volumeStr, bucketStr);
} catch (OMException ex) {
- // Note: always create bucket if volumeStr matches "tmp" so -put works
if (createIfNotExist) {
- // Note: getBucketDetails always throws BUCKET_NOT_FOUND, even if
- // the volume doesn't exist.
- if (ex.getResult().equals(BUCKET_NOT_FOUND)) {
- OzoneVolume volume;
+ // getBucketDetails can throw VOLUME_NOT_FOUND when the parent volume
+ // doesn't exist and ACL is enabled; it can only throw BUCKET_NOT_FOUND
+ // when ACL is disabled. Both exceptions need to be handled.
+ switch (ex.getResult()) {
+ case VOLUME_NOT_FOUND:
+ case BUCKET_NOT_FOUND:
Review comment:
Do you want to create a newbie jira to clean up the behavior to send the appropriate error which is the right fix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] kerneltime commented on a change in pull request #2412: HDDS-5279. OFS mkdir -p does not work when Volume is not pre-created
Posted by GitBox <gi...@apache.org>.
kerneltime commented on a change in pull request #2412:
URL: https://github.com/apache/ozone/pull/2412#discussion_r671772102
##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -223,31 +223,23 @@ private OzoneBucket getBucket(String volumeStr, String bucketStr,
try {
bucket = proxy.getBucketDetails(volumeStr, bucketStr);
} catch (OMException ex) {
- // Note: always create bucket if volumeStr matches "tmp" so -put works
if (createIfNotExist) {
- // Note: getBucketDetails always throws BUCKET_NOT_FOUND, even if
- // the volume doesn't exist.
- if (ex.getResult().equals(BUCKET_NOT_FOUND)) {
- OzoneVolume volume;
+ // getBucketDetails can throw VOLUME_NOT_FOUND when the parent volume
+ // doesn't exist and ACL is enabled; it can only throw BUCKET_NOT_FOUND
+ // when ACL is disabled. Both exceptions need to be handled.
+ switch (ex.getResult()) {
+ case VOLUME_NOT_FOUND:
+ case BUCKET_NOT_FOUND:
Review comment:
The case where volume is not found should not be as common as bucket not found. I think it would be better to avoid the redundant call to create volume.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] kerneltime commented on a change in pull request #2412: HDDS-5279. OFS mkdir -p does not work when Volume is not pre-created
Posted by GitBox <gi...@apache.org>.
kerneltime commented on a change in pull request #2412:
URL: https://github.com/apache/ozone/pull/2412#discussion_r672584791
##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -223,31 +223,23 @@ private OzoneBucket getBucket(String volumeStr, String bucketStr,
try {
bucket = proxy.getBucketDetails(volumeStr, bucketStr);
} catch (OMException ex) {
- // Note: always create bucket if volumeStr matches "tmp" so -put works
if (createIfNotExist) {
- // Note: getBucketDetails always throws BUCKET_NOT_FOUND, even if
- // the volume doesn't exist.
- if (ex.getResult().equals(BUCKET_NOT_FOUND)) {
- OzoneVolume volume;
+ // getBucketDetails can throw VOLUME_NOT_FOUND when the parent volume
+ // doesn't exist and ACL is enabled; it can only throw BUCKET_NOT_FOUND
+ // when ACL is disabled. Both exceptions need to be handled.
+ switch (ex.getResult()) {
+ case VOLUME_NOT_FOUND:
+ case BUCKET_NOT_FOUND:
Review comment:
Do you want to create a newbie jira to clean up the behavior to send the appropriate error which is the right fix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] kerneltime commented on a change in pull request #2412: HDDS-5279. OFS mkdir -p does not work when Volume is not pre-created
Posted by GitBox <gi...@apache.org>.
kerneltime commented on a change in pull request #2412:
URL: https://github.com/apache/ozone/pull/2412#discussion_r671772102
##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -223,31 +223,23 @@ private OzoneBucket getBucket(String volumeStr, String bucketStr,
try {
bucket = proxy.getBucketDetails(volumeStr, bucketStr);
} catch (OMException ex) {
- // Note: always create bucket if volumeStr matches "tmp" so -put works
if (createIfNotExist) {
- // Note: getBucketDetails always throws BUCKET_NOT_FOUND, even if
- // the volume doesn't exist.
- if (ex.getResult().equals(BUCKET_NOT_FOUND)) {
- OzoneVolume volume;
+ // getBucketDetails can throw VOLUME_NOT_FOUND when the parent volume
+ // doesn't exist and ACL is enabled; it can only throw BUCKET_NOT_FOUND
+ // when ACL is disabled. Both exceptions need to be handled.
+ switch (ex.getResult()) {
+ case VOLUME_NOT_FOUND:
+ case BUCKET_NOT_FOUND:
Review comment:
The case where volume is not found should be be as common as bucket not found. Would wrapping the processing in a loop and handling volume and bucket separately be better than always trying volume creation?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on pull request #2412: HDDS-5279. OFS mkdir -p does not work when Volume is not pre-created
Posted by GitBox <gi...@apache.org>.
smengcl commented on pull request #2412:
URL: https://github.com/apache/ozone/pull/2412#issuecomment-884998687
> @smengcl , correct if i m wrong. Volumes in ozone are supposed to be created by admins only as per the doc. Are we going to break this notion here?
Nope. The behavior hasn't changed. This is client code. Server (OM) is responsible for checking if the client has the permission to create a volume. In this case, if a regular user (non-admin) tries to do `mkdir -p ofs://om/vol2/bucket2/` and `vol2` doesn't exist, the client will be rejected.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a change in pull request #2412: HDDS-5279. OFS mkdir -p does not work when Volume is not pre-created
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2412:
URL: https://github.com/apache/ozone/pull/2412#discussion_r672513719
##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -223,31 +223,23 @@ private OzoneBucket getBucket(String volumeStr, String bucketStr,
try {
bucket = proxy.getBucketDetails(volumeStr, bucketStr);
} catch (OMException ex) {
- // Note: always create bucket if volumeStr matches "tmp" so -put works
if (createIfNotExist) {
- // Note: getBucketDetails always throws BUCKET_NOT_FOUND, even if
- // the volume doesn't exist.
- if (ex.getResult().equals(BUCKET_NOT_FOUND)) {
- OzoneVolume volume;
+ // getBucketDetails can throw VOLUME_NOT_FOUND when the parent volume
+ // doesn't exist and ACL is enabled; it can only throw BUCKET_NOT_FOUND
+ // when ACL is disabled. Both exceptions need to be handled.
+ switch (ex.getResult()) {
+ case VOLUME_NOT_FOUND:
+ case BUCKET_NOT_FOUND:
Review comment:
Thanks for the comment @kerneltime .
`VOLUME_NOT_FOUND` is actually frequently encountered during our testing where we create volume+bucket+directories on one shot with `-p` when ACL is enabled.
If you think about it, in both cases the action(s) we need to take is the same. i.e. volume needs to be created.
It's just that the exception thrown from the call `proxy.getBucketDetails` is different -- which is bizzare, and we maybe want to fix `getBucketDetails`'s behavior at some point (for example, to always throw `VOLUME_NOT_FOUND` regardless of whether ACL is enabled or not).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on pull request #2412: HDDS-5279. OFS mkdir -p does not work when Volume is not pre-created
Posted by GitBox <gi...@apache.org>.
smengcl commented on pull request #2412:
URL: https://github.com/apache/ozone/pull/2412#issuecomment-889352218
Thanks @kerneltime and @avijayanhwx for the reviews. I have merged this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org