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 2020/02/27 01:37:02 UTC
[GitHub] [hadoop-ozone] smengcl opened a new pull request #610: HDDS-2929.
Implement ofs://: temp directory mount
smengcl opened a new pull request #610: HDDS-2929. Implement ofs://: temp directory mount
URL: https://github.com/apache/hadoop-ozone/pull/610
## What changes were proposed in this pull request?
Support client-side /tmp/ mount for legacy applications. Each user will have their own temp bucket in this implementation.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-2929
## How was this patch tested?
Added new test `testTempMount`.
## NOTES
1. An admin must create volume "tmp" with world access ACL before any user can use /tmp/
2. When a user is trying to write under /tmp/, a bucket with that user's name will be automatically created under volume "tmp"
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl edited a comment on issue #610: HDDS-2929.
Implement ofs://: temp directory mount
Posted by GitBox <gi...@apache.org>.
smengcl edited a comment on issue #610: HDDS-2929. Implement ofs://: temp directory mount
URL: https://github.com/apache/hadoop-ozone/pull/610#issuecomment-592226679
Note this might conflict with https://github.com/apache/hadoop-ozone/pull/606 so I'd rebase after that one get in.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #610:
HDDS-2929. Implement ofs://: temp directory mount
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #610: HDDS-2929. Implement ofs://: temp directory mount
URL: https://github.com/apache/hadoop-ozone/pull/610#discussion_r385418855
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java
##########
@@ -113,12 +125,12 @@ public void teardown() {
@Test
public void testOzoneFsServiceLoader() throws IOException {
- OzoneConfiguration conf = new OzoneConfiguration();
+ OzoneConfiguration cfg = new OzoneConfiguration();
Review comment:
Done in 28cedc4db0820b84e8d2af151525d66ec2af944b
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request
#610: HDDS-2929. Implement ofs://: temp directory mount
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #610: HDDS-2929. Implement ofs://: temp directory mount
URL: https://github.com/apache/hadoop-ozone/pull/610#discussion_r389175589
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/OFSPath.java
##########
@@ -44,7 +48,7 @@
* /vol1/buc2/dir3/key4 vol1 buc2 (empty) dir3/key4
* /vol1/buc2 vol1 buc2 (empty) (empty)
* /vol1 vol1 (empty) (empty) (empty)
- * /tmp/dir3/key4 tempVolume tempBucket tmp dir3/key4
+ * /tmp/dir3/key4 tmp <username> tmp dir3/key4
Review comment:
NIT: should we update the comment to match with the code?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #610:
HDDS-2929. Implement ofs://: temp directory mount
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #610: HDDS-2929. Implement ofs://: temp directory mount
URL: https://github.com/apache/hadoop-ozone/pull/610#discussion_r389182311
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/OFSPath.java
##########
@@ -44,7 +48,7 @@
* /vol1/buc2/dir3/key4 vol1 buc2 (empty) dir3/key4
* /vol1/buc2 vol1 buc2 (empty) (empty)
* /vol1 vol1 (empty) (empty) (empty)
- * /tmp/dir3/key4 tempVolume tempBucket tmp dir3/key4
+ * /tmp/dir3/key4 tmp <username> tmp dir3/key4
Review comment:
As discussed, will resolve in a follow up jira.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #610:
HDDS-2929. Implement ofs://: temp directory mount
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #610: HDDS-2929. Implement ofs://: temp directory mount
URL: https://github.com/apache/hadoop-ozone/pull/610#discussion_r389183849
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/OFSPath.java
##########
@@ -174,4 +185,27 @@ public boolean isRoot() {
public boolean isVolume() {
return this.getBucketName().isEmpty() && !this.getVolumeName().isEmpty();
}
+
+ /**
+ * Get the bucket name of temp for given username.
+ * @param username Input user name String. Mustn't be null.
+ * @return Username MD5 hash in hex digits.
+ */
+ @VisibleForTesting
+ static String getTempMountBucketName(String username) {
+ Preconditions.checkNotNull(username);
+ // TODO: Improve this to "slugify(username)-md5(username)" for better
+ // readability?
+ return DigestUtils.md5Hex(username);
Review comment:
As discussed, since other users' `tmp` buckets aren't visible to current user via `ofs://`. This could be addressed when the requirement of accessing other users' temp bucket happen.
And there are several related future improvements. Like:
1. allowing users to access all volumes (that they have access to) via a special reserved path (e.g. `ofs://om/.volumes/<volume_name>/..` or `ofs://om/.reserved/..`). Also, admins should be able to list all volumes, not just the ones they own.
2. Provide a utility in Ozone Shell to access the users' temp buckets conveniently for admins.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao merged pull request #610: HDDS-2929.
Implement ofs://: temp directory mount
Posted by GitBox <gi...@apache.org>.
xiaoyuyao merged pull request #610: HDDS-2929. Implement ofs://: temp directory mount
URL: https://github.com/apache/hadoop-ozone/pull/610
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request
#610: HDDS-2929. Implement ofs://: temp directory mount
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #610: HDDS-2929. Implement ofs://: temp directory mount
URL: https://github.com/apache/hadoop-ozone/pull/610#discussion_r386687730
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/OFSPath.java
##########
@@ -174,4 +181,32 @@ public boolean isRoot() {
public boolean isVolume() {
return this.getBucketName().isEmpty() && !this.getVolumeName().isEmpty();
}
+
+ /**
+ * Get the bucket name of temp for given username.
+ * @param username Input user name String. Mustn't be null.
+ * @return Username MD5 hash in hex digits.
+ */
+ @VisibleForTesting
+ static String getTempMountBucketName(String username) {
+ Preconditions.checkNotNull(username);
+ // TODO: Improve this to "slugify(username)-md5(username)" for better
+ // readability?
+ return DigestUtils.md5Hex(username);
+ }
+
+ /**
+ * Get the bucket name of temp for the current user from UserGroupInformation.
+ * @return Username MD5 hash in hex digits.
+ */
+ @VisibleForTesting
+ static String getTempMountBucketNameOfCurrentUser() {
+ String username;
+ try {
+ username = UserGroupInformation.getCurrentUser().getUserName();
+ } catch (IOException ex) {
Review comment:
why do we need to catch the exception here? This should not fail, if it does, the exception can bubble up directly.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] timmylicheng commented on a change in pull request
#610: HDDS-2929. Implement ofs://: temp directory mount
Posted by GitBox <gi...@apache.org>.
timmylicheng commented on a change in pull request #610: HDDS-2929. Implement ofs://: temp directory mount
URL: https://github.com/apache/hadoop-ozone/pull/610#discussion_r385064711
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/OFSPath.java
##########
@@ -67,10 +75,9 @@ private void initOFSPath(String pathStr) {
// TODO: Compare a keyword list instead for future expansion.
if (firstToken.equals(OFS_MOUNT_NAME_TMP)) {
mountName = firstToken;
- // TODO: Retrieve volume and bucket of the mount from user protobuf.
- // Leave them hard-coded just for now. Will be addressed in HDDS-2929
- volumeName = "tempVolume";
- bucketName = "tempBucket";
+ // Future: retrieve volume and bucket from UserVolumeInfo
Review comment:
Do you mean to comment a TODO 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #610:
HDDS-2929. Implement ofs://: temp directory mount
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #610: HDDS-2929. Implement ofs://: temp directory mount
URL: https://github.com/apache/hadoop-ozone/pull/610#discussion_r386692432
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -213,7 +213,9 @@ private OzoneBucket getBucket(String volumeStr, String bucketStr,
try {
bucket = proxy.getBucketDetails(volumeStr, bucketStr);
} catch (OMException ex) {
- if (createIfNotExist) {
+ // Note: always create bucket if volumeStr matches "tmp" so -put works
+ if (createIfNotExist ||
+ volumeStr.equals(OFSPath.OFS_MOUNT_TMP_VOLUMENAME)) {
Review comment:
That's correct. This is my implementation decision.
I initially only create temp bucket for write, as you would expect. Then I found Hadoop FS file write operations calls `getFileStatus` beforehand.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #610:
HDDS-2929. Implement ofs://: temp directory mount
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #610: HDDS-2929. Implement ofs://: temp directory mount
URL: https://github.com/apache/hadoop-ozone/pull/610#discussion_r386723497
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/OFSPath.java
##########
@@ -44,7 +48,7 @@
* /vol1/buc2/dir3/key4 vol1 buc2 (empty) dir3/key4
* /vol1/buc2 vol1 buc2 (empty) (empty)
* /vol1 vol1 (empty) (empty) (empty)
- * /tmp/dir3/key4 tempVolume tempBucket tmp dir3/key4
+ * /tmp/dir3/key4 tmp <username> tmp dir3/key4
Review comment:
As of now userA can't access userB's temp with ofs:// . Because OFSPath parses `ofs://tmp/dir1/dir2` into
```
volume=tmp
bucketname=<USER_TEMP_BUCKET_NAME>
key(in bucket)=dir1/dir2
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #610:
HDDS-2929. Implement ofs://: temp directory mount
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #610: HDDS-2929. Implement ofs://: temp directory mount
URL: https://github.com/apache/hadoop-ozone/pull/610#discussion_r386723497
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/OFSPath.java
##########
@@ -44,7 +48,7 @@
* /vol1/buc2/dir3/key4 vol1 buc2 (empty) dir3/key4
* /vol1/buc2 vol1 buc2 (empty) (empty)
* /vol1 vol1 (empty) (empty) (empty)
- * /tmp/dir3/key4 tempVolume tempBucket tmp dir3/key4
+ * /tmp/dir3/key4 tmp <username> tmp dir3/key4
Review comment:
As of now userA can't access userB's temp with ofs:// . Because OFSPath parses `ofs://tmp/dir1/dir2` into `volume=tmp bucketname=<USER_TEMP_BUCKET_NAME>, key(in bucket)=dir1/dir2`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #610:
HDDS-2929. Implement ofs://: temp directory mount
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #610: HDDS-2929. Implement ofs://: temp directory mount
URL: https://github.com/apache/hadoop-ozone/pull/610#discussion_r385422979
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/OFSPath.java
##########
@@ -67,10 +75,9 @@ private void initOFSPath(String pathStr) {
// TODO: Compare a keyword list instead for future expansion.
if (firstToken.equals(OFS_MOUNT_NAME_TMP)) {
mountName = firstToken;
- // TODO: Retrieve volume and bucket of the mount from user protobuf.
- // Leave them hard-coded just for now. Will be addressed in HDDS-2929
- volumeName = "tempVolume";
- bucketName = "tempBucket";
+ // Future: retrieve volume and bucket from UserVolumeInfo
Review comment:
Done in 5ed5cfa523cb6a0ff1d2c5179ef3f0f04d8c7ae9
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on issue #610: HDDS-2929.
Implement ofs://: temp directory mount
Posted by GitBox <gi...@apache.org>.
smengcl commented on issue #610: HDDS-2929. Implement ofs://: temp directory mount
URL: https://github.com/apache/hadoop-ozone/pull/610#issuecomment-592226679
Note this might conflict with https://github.com/apache/hadoop-ozone/pull/606 so I'd so a rebase after that one get in.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #610:
HDDS-2929. Implement ofs://: temp directory mount
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #610: HDDS-2929. Implement ofs://: temp directory mount
URL: https://github.com/apache/hadoop-ozone/pull/610#discussion_r385418333
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java
##########
@@ -113,12 +125,12 @@ public void teardown() {
@Test
public void testOzoneFsServiceLoader() throws IOException {
- OzoneConfiguration conf = new OzoneConfiguration();
+ OzoneConfiguration cfg = new OzoneConfiguration();
Review comment:
This is due to I put another `conf` in global. So IntelliJ is complaining this `conf` inside shadows the global `conf` in this function.
Anyway, will rename it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] timmylicheng commented on a change in pull request
#610: HDDS-2929. Implement ofs://: temp directory mount
Posted by GitBox <gi...@apache.org>.
timmylicheng commented on a change in pull request #610: HDDS-2929. Implement ofs://: temp directory mount
URL: https://github.com/apache/hadoop-ozone/pull/610#discussion_r385064366
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/OFSPath.java
##########
@@ -67,10 +75,9 @@ private void initOFSPath(String pathStr) {
// TODO: Compare a keyword list instead for future expansion.
if (firstToken.equals(OFS_MOUNT_NAME_TMP)) {
mountName = firstToken;
- // TODO: Retrieve volume and bucket of the mount from user protobuf.
- // Leave them hard-coded just for now. Will be addressed in HDDS-2929
- volumeName = "tempVolume";
- bucketName = "tempBucket";
+ // Future: retrieve volume and bucket from UserVolumeInfo
+ volumeName = OFS_MOUNT_TMP_VOLUMENAME;
+ bucketName = getTempMountBucketName(null);
Review comment:
Is it better to have a 'DEFAULT_USER' string to replace a null input?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #610:
HDDS-2929. Implement ofs://: temp directory mount
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #610: HDDS-2929. Implement ofs://: temp directory mount
URL: https://github.com/apache/hadoop-ozone/pull/610#discussion_r386723017
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/OFSPath.java
##########
@@ -174,4 +181,32 @@ public boolean isRoot() {
public boolean isVolume() {
return this.getBucketName().isEmpty() && !this.getVolumeName().isEmpty();
}
+
+ /**
+ * Get the bucket name of temp for given username.
+ * @param username Input user name String. Mustn't be null.
+ * @return Username MD5 hash in hex digits.
+ */
+ @VisibleForTesting
+ static String getTempMountBucketName(String username) {
+ Preconditions.checkNotNull(username);
+ // TODO: Improve this to "slugify(username)-md5(username)" for better
+ // readability?
+ return DigestUtils.md5Hex(username);
+ }
+
+ /**
+ * Get the bucket name of temp for the current user from UserGroupInformation.
+ * @return Username MD5 hash in hex digits.
+ */
+ @VisibleForTesting
+ static String getTempMountBucketNameOfCurrentUser() {
+ String username;
+ try {
+ username = UserGroupInformation.getCurrentUser().getUserName();
+ } catch (IOException ex) {
Review comment:
Done in 1d4164b153386c5e476f27df6420512b60894d5d
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #610:
HDDS-2929. Implement ofs://: temp directory mount
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #610: HDDS-2929. Implement ofs://: temp directory mount
URL: https://github.com/apache/hadoop-ozone/pull/610#discussion_r386722947
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -213,7 +213,9 @@ private OzoneBucket getBucket(String volumeStr, String bucketStr,
try {
bucket = proxy.getBucketDetails(volumeStr, bucketStr);
} catch (OMException ex) {
- if (createIfNotExist) {
+ // Note: always create bucket if volumeStr matches "tmp" so -put works
+ if (createIfNotExist ||
+ volumeStr.equals(OFSPath.OFS_MOUNT_TMP_VOLUMENAME)) {
Review comment:
Addressed in 1d4164b153386c5e476f27df6420512b60894d5d
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request
#610: HDDS-2929. Implement ofs://: temp directory mount
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #610: HDDS-2929. Implement ofs://: temp directory mount
URL: https://github.com/apache/hadoop-ozone/pull/610#discussion_r387353833
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/OFSPath.java
##########
@@ -44,7 +48,7 @@
* /vol1/buc2/dir3/key4 vol1 buc2 (empty) dir3/key4
* /vol1/buc2 vol1 buc2 (empty) (empty)
* /vol1 vol1 (empty) (empty) (empty)
- * /tmp/dir3/key4 tempVolume tempBucket tmp dir3/key4
+ * /tmp/dir3/key4 tmp <username> tmp dir3/key4
Review comment:
agree, can you add the proposal to the design doc in the umbrella JIRA?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #610:
HDDS-2929. Implement ofs://: temp directory mount
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #610: HDDS-2929. Implement ofs://: temp directory mount
URL: https://github.com/apache/hadoop-ozone/pull/610#discussion_r386723869
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/OFSPath.java
##########
@@ -44,7 +48,7 @@
* /vol1/buc2/dir3/key4 vol1 buc2 (empty) dir3/key4
* /vol1/buc2 vol1 buc2 (empty) (empty)
* /vol1 vol1 (empty) (empty) (empty)
- * /tmp/dir3/key4 tempVolume tempBucket tmp dir3/key4
+ * /tmp/dir3/key4 tmp <username> tmp dir3/key4
Review comment:
I'm thinking of adding some `.reserved` like HDFS did to allow user to bypass the mount or add some options, if there is a need.
Anyway this may not fit in this jira.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #610:
HDDS-2929. Implement ofs://: temp directory mount
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #610: HDDS-2929. Implement ofs://: temp directory mount
URL: https://github.com/apache/hadoop-ozone/pull/610#discussion_r385422940
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/OFSPath.java
##########
@@ -67,10 +75,9 @@ private void initOFSPath(String pathStr) {
// TODO: Compare a keyword list instead for future expansion.
if (firstToken.equals(OFS_MOUNT_NAME_TMP)) {
mountName = firstToken;
- // TODO: Retrieve volume and bucket of the mount from user protobuf.
- // Leave them hard-coded just for now. Will be addressed in HDDS-2929
- volumeName = "tempVolume";
- bucketName = "tempBucket";
+ // Future: retrieve volume and bucket from UserVolumeInfo
+ volumeName = OFS_MOUNT_TMP_VOLUMENAME;
+ bucketName = getTempMountBucketName(null);
Review comment:
Thanks for the review and comment.
Yeah good point. `null` here doesn't have have good readability.
Added new function `getTempMountBucketNameOfCurrentUser()` in 5ed5cfa523cb6a0ff1d2c5179ef3f0f04d8c7ae9
Initially I thought of overriding it but ended up adding a new function with a different name for even better code readability.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on issue #610: HDDS-2929.
Implement ofs://: temp directory mount
Posted by GitBox <gi...@apache.org>.
smengcl commented on issue #610: HDDS-2929. Implement ofs://: temp directory mount
URL: https://github.com/apache/hadoop-ozone/pull/610#issuecomment-592735946
Just rebased on to latest OFS branch.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request
#610: HDDS-2929. Implement ofs://: temp directory mount
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #610: HDDS-2929. Implement ofs://: temp directory mount
URL: https://github.com/apache/hadoop-ozone/pull/610#discussion_r389175764
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/OFSPath.java
##########
@@ -174,4 +185,27 @@ public boolean isRoot() {
public boolean isVolume() {
return this.getBucketName().isEmpty() && !this.getVolumeName().isEmpty();
}
+
+ /**
+ * Get the bucket name of temp for given username.
+ * @param username Input user name String. Mustn't be null.
+ * @return Username MD5 hash in hex digits.
+ */
+ @VisibleForTesting
+ static String getTempMountBucketName(String username) {
+ Preconditions.checkNotNull(username);
+ // TODO: Improve this to "slugify(username)-md5(username)" for better
+ // readability?
+ return DigestUtils.md5Hex(username);
Review comment:
Can we use the name directly to make it more user friendly?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request
#610: HDDS-2929. Implement ofs://: temp directory mount
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #610: HDDS-2929. Implement ofs://: temp directory mount
URL: https://github.com/apache/hadoop-ozone/pull/610#discussion_r389175764
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/OFSPath.java
##########
@@ -174,4 +185,27 @@ public boolean isRoot() {
public boolean isVolume() {
return this.getBucketName().isEmpty() && !this.getVolumeName().isEmpty();
}
+
+ /**
+ * Get the bucket name of temp for given username.
+ * @param username Input user name String. Mustn't be null.
+ * @return Username MD5 hash in hex digits.
+ */
+ @VisibleForTesting
+ static String getTempMountBucketName(String username) {
+ Preconditions.checkNotNull(username);
+ // TODO: Improve this to "slugify(username)-md5(username)" for better
+ // readability?
+ return DigestUtils.md5Hex(username);
Review comment:
NIT: Can we use the name directly to make it more user friendly? Given it is transparent to the ofs client access, I think it is OK as-is to avoid confliction.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #610:
HDDS-2929. Implement ofs://: temp directory mount
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #610: HDDS-2929. Implement ofs://: temp directory mount
URL: https://github.com/apache/hadoop-ozone/pull/610#discussion_r386722258
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -213,7 +213,9 @@ private OzoneBucket getBucket(String volumeStr, String bucketStr,
try {
bucket = proxy.getBucketDetails(volumeStr, bucketStr);
} catch (OMException ex) {
- if (createIfNotExist) {
+ // Note: always create bucket if volumeStr matches "tmp" so -put works
+ if (createIfNotExist ||
+ volumeStr.equals(OFSPath.OFS_MOUNT_TMP_VOLUMENAME)) {
Review comment:
After some discussion, we decide to let user create their temp bucket manually upon first use. i.e. `ozone fs -mkdir ofs://om/tmp/`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #610:
HDDS-2929. Implement ofs://: temp directory mount
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #610: HDDS-2929. Implement ofs://: temp directory mount
URL: https://github.com/apache/hadoop-ozone/pull/610#discussion_r389183849
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/OFSPath.java
##########
@@ -174,4 +185,27 @@ public boolean isRoot() {
public boolean isVolume() {
return this.getBucketName().isEmpty() && !this.getVolumeName().isEmpty();
}
+
+ /**
+ * Get the bucket name of temp for given username.
+ * @param username Input user name String. Mustn't be null.
+ * @return Username MD5 hash in hex digits.
+ */
+ @VisibleForTesting
+ static String getTempMountBucketName(String username) {
+ Preconditions.checkNotNull(username);
+ // TODO: Improve this to "slugify(username)-md5(username)" for better
+ // readability?
+ return DigestUtils.md5Hex(username);
Review comment:
As discussed, since other users' `tmp` buckets aren't visible to current user via `ofs://`. This could be addressed when the requirement of accessing other users' temp bucket happen.
And there are several related future improvements. Like:
1. Allowing users to access all volumes (that they have access to) via a special reserved path (e.g. `ofs://om/.volumes/<volume_name>/..` or `ofs://om/.reserved/..`). Because at the moment, `tmp` mount is masking the existence of volume `tmp` in `ofs://` because of the parsing logic in `OFSPath`.
Also, admins should be able to list all volumes, not just the ones they own.
2. Provide a utility in Ozone Shell to access the users' temp buckets conveniently for admins.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #610:
HDDS-2929. Implement ofs://: temp directory mount
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #610: HDDS-2929. Implement ofs://: temp directory mount
URL: https://github.com/apache/hadoop-ozone/pull/610#discussion_r389182311
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/OFSPath.java
##########
@@ -44,7 +48,7 @@
* /vol1/buc2/dir3/key4 vol1 buc2 (empty) dir3/key4
* /vol1/buc2 vol1 buc2 (empty) (empty)
* /vol1 vol1 (empty) (empty) (empty)
- * /tmp/dir3/key4 tempVolume tempBucket tmp dir3/key4
+ * /tmp/dir3/key4 tmp <username> tmp dir3/key4
Review comment:
Will resolve in a follow up jira.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] timmylicheng commented on a change in pull request
#610: HDDS-2929. Implement ofs://: temp directory mount
Posted by GitBox <gi...@apache.org>.
timmylicheng commented on a change in pull request #610: HDDS-2929. Implement ofs://: temp directory mount
URL: https://github.com/apache/hadoop-ozone/pull/610#discussion_r385065145
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java
##########
@@ -113,12 +125,12 @@ public void teardown() {
@Test
public void testOzoneFsServiceLoader() throws IOException {
- OzoneConfiguration conf = new OzoneConfiguration();
+ OzoneConfiguration cfg = new OzoneConfiguration();
Review comment:
NIT. 'conf' may be a more consistent choice of naming in Ozone repo.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request
#610: HDDS-2929. Implement ofs://: temp directory mount
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #610: HDDS-2929. Implement ofs://: temp directory mount
URL: https://github.com/apache/hadoop-ozone/pull/610#discussion_r386686189
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/OFSPath.java
##########
@@ -44,7 +48,7 @@
* /vol1/buc2/dir3/key4 vol1 buc2 (empty) dir3/key4
* /vol1/buc2 vol1 buc2 (empty) (empty)
* /vol1 vol1 (empty) (empty) (empty)
- * /tmp/dir3/key4 tempVolume tempBucket tmp dir3/key4
+ * /tmp/dir3/key4 tmp <username> tmp dir3/key4
Review comment:
How do we allow userA to access userB's tmp bucket? This may be needed for the ETL workload where an injest process run as user A and the process process run as user B.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request
#610: HDDS-2929. Implement ofs://: temp directory mount
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #610: HDDS-2929. Implement ofs://: temp directory mount
URL: https://github.com/apache/hadoop-ozone/pull/610#discussion_r386685553
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -213,7 +213,9 @@ private OzoneBucket getBucket(String volumeStr, String bucketStr,
try {
bucket = proxy.getBucketDetails(volumeStr, bucketStr);
} catch (OMException ex) {
- if (createIfNotExist) {
+ // Note: always create bucket if volumeStr matches "tmp" so -put works
+ if (createIfNotExist ||
+ volumeStr.equals(OFSPath.OFS_MOUNT_TMP_VOLUMENAME)) {
Review comment:
Do we create the tmp bucket even for read operation?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org