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/11/24 12:46:20 UTC

[GitHub] [ozone] adoroszlai opened a new pull request #1619: HDDS-4503. Provide info on block size via FileSystem

adoroszlai opened a new pull request #1619:
URL: https://github.com/apache/ozone/pull/1619


   ## What changes were proposed in this pull request?
   
   Provide information about Ozone's configured block size via Hadoop-compatible file system implementations by overriding `getDefaultBlockSize()` from `FileSystem`.
   
   Ozone does not make new files visible until they are closed.  This change allows Impala to query the actual configured block size and use this info during file write, instead of relying on the default block size, which can be overridden in config.
   
   https://issues.apache.org/jira/browse/HDDS-4503
   
   ## How was this patch tested?
   
   Added unit test.


----------------------------------------------------------------
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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] adoroszlai commented on a change in pull request #1619: HDDS-4503. Provide info on block size via FileSystem

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #1619:
URL: https://github.com/apache/ozone/pull/1619#discussion_r541547602



##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneFileSystem.java
##########
@@ -698,6 +701,12 @@ public boolean mkdirs(Path f, FsPermission permission) throws IOException {
     return mkdir(f);
   }
 
+  @Override
+  public long getDefaultBlockSize() {
+    return (long) getConf().getStorageSize(
+        OZONE_SCM_BLOCK_SIZE, OZONE_SCM_BLOCK_SIZE_DEFAULT, StorageUnit.BYTES);

Review comment:
       Thanks @arp7 for taking a look.  Does the above answer your question, or would you like further clarification?




----------------------------------------------------------------
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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] adoroszlai commented on a change in pull request #1619: HDDS-4503. Provide info on block size via FileSystem

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #1619:
URL: https://github.com/apache/ozone/pull/1619#discussion_r530134818



##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneFileSystem.java
##########
@@ -698,6 +701,12 @@ public boolean mkdirs(Path f, FsPermission permission) throws IOException {
     return mkdir(f);
   }
 
+  @Override
+  public long getDefaultBlockSize() {
+    return (long) getConf().getStorageSize(
+        OZONE_SCM_BLOCK_SIZE, OZONE_SCM_BLOCK_SIZE_DEFAULT, StorageUnit.BYTES);

Review comment:
       This is intended to be used by the client that creates the file.  Once the file is closed, the actual block size can be queried via `getBlockSize(Path)`.  Currently this method returns 32MB, regardless of SCM config:
   
   https://github.com/apache/hadoop/blob/a6df05bf5e24d04852a35b096c44e79f843f4776/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java#L2521-L2522




----------------------------------------------------------------
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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ayushtkn commented on a change in pull request #1619: HDDS-4503. Provide info on block size via FileSystem

Posted by GitBox <gi...@apache.org>.
ayushtkn commented on a change in pull request #1619:
URL: https://github.com/apache/ozone/pull/1619#discussion_r529620252



##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java
##########
@@ -723,6 +726,12 @@ public boolean mkdirs(Path f, FsPermission permission) throws IOException {
     return mkdir(f);
   }
 
+  @Override
+  public long getDefaultBlockSize() {
+    return (long) getConf().getStorageSize(
+        OZONE_SCM_BLOCK_SIZE, OZONE_SCM_BLOCK_SIZE_DEFAULT, StorageUnit.BYTES);

Review comment:
       Is this block size used by client while writing? In create and createNonRecursive it tends to ignore the specified block size. 
   So, if it isn't correct, Isn't it same as not implementing it? `FileSystem` will anyway return some value, that can be tweaked as well by `fs.local.block.size`




----------------------------------------------------------------
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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] arp7 commented on a change in pull request #1619: HDDS-4503. Provide info on block size via FileSystem

Posted by GitBox <gi...@apache.org>.
arp7 commented on a change in pull request #1619:
URL: https://github.com/apache/ozone/pull/1619#discussion_r529879618



##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneFileSystem.java
##########
@@ -698,6 +701,12 @@ public boolean mkdirs(Path f, FsPermission permission) throws IOException {
     return mkdir(f);
   }
 
+  @Override
+  public long getDefaultBlockSize() {
+    return (long) getConf().getStorageSize(
+        OZONE_SCM_BLOCK_SIZE, OZONE_SCM_BLOCK_SIZE_DEFAULT, StorageUnit.BYTES);

Review comment:
       What if the client who wrote the block and the client who is querying have a different configuration?




----------------------------------------------------------------
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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] arp7 commented on a change in pull request #1619: HDDS-4503. Provide info on block size via FileSystem

Posted by GitBox <gi...@apache.org>.
arp7 commented on a change in pull request #1619:
URL: https://github.com/apache/ozone/pull/1619#discussion_r541648897



##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneFileSystem.java
##########
@@ -698,6 +701,12 @@ public boolean mkdirs(Path f, FsPermission permission) throws IOException {
     return mkdir(f);
   }
 
+  @Override
+  public long getDefaultBlockSize() {
+    return (long) getConf().getStorageSize(
+        OZONE_SCM_BLOCK_SIZE, OZONE_SCM_BLOCK_SIZE_DEFAULT, StorageUnit.BYTES);

Review comment:
       Yes, thanks. Sorry i missed responding.




----------------------------------------------------------------
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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] adoroszlai commented on a change in pull request #1619: HDDS-4503. Provide info on block size via FileSystem

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #1619:
URL: https://github.com/apache/ozone/pull/1619#discussion_r530134818



##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneFileSystem.java
##########
@@ -698,6 +701,12 @@ public boolean mkdirs(Path f, FsPermission permission) throws IOException {
     return mkdir(f);
   }
 
+  @Override
+  public long getDefaultBlockSize() {
+    return (long) getConf().getStorageSize(
+        OZONE_SCM_BLOCK_SIZE, OZONE_SCM_BLOCK_SIZE_DEFAULT, StorageUnit.BYTES);

Review comment:
       This is intended to be used by the client that creates the file.  Once the file is closed, the actual block size can be queried via `getBlockSize(Path)`.  Currently `getDefaultBlockSize()` returns 32MB, regardless of SCM config:
   
   https://github.com/apache/hadoop/blob/a6df05bf5e24d04852a35b096c44e79f843f4776/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java#L2521-L2522




----------------------------------------------------------------
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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] arp7 merged pull request #1619: HDDS-4503. Provide info on block size via FileSystem

Posted by GitBox <gi...@apache.org>.
arp7 merged pull request #1619:
URL: https://github.com/apache/ozone/pull/1619


   


----------------------------------------------------------------
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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] adoroszlai commented on a change in pull request #1619: HDDS-4503. Provide info on block size via FileSystem

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #1619:
URL: https://github.com/apache/ozone/pull/1619#discussion_r529758307



##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java
##########
@@ -723,6 +726,12 @@ public boolean mkdirs(Path f, FsPermission permission) throws IOException {
     return mkdir(f);
   }
 
+  @Override
+  public long getDefaultBlockSize() {
+    return (long) getConf().getStorageSize(
+        OZONE_SCM_BLOCK_SIZE, OZONE_SCM_BLOCK_SIZE_DEFAULT, StorageUnit.BYTES);

Review comment:
       > In create and createNonRecursive it tends to ignore the specified block size.
   
   Ozone also ignores it and uses uniform block size on server side (configurable via this `OZONE_SCM_BLOCK_SIZE` property).
   
   > can be tweaked as well by `fs.local.block.size`
   
   You are right, we could achieve the same result by forcing this config to the same value as SCM block size.  I think it's simpler and cleaner to override the method than to keep the config in sync.




----------------------------------------------------------------
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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] adoroszlai commented on pull request #1619: HDDS-4503. Provide info on block size via FileSystem

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #1619:
URL: https://github.com/apache/ozone/pull/1619#issuecomment-743882378


   Thanks @arp7 for reviewing and committing it.  Thanks @ayushtkn for the review.


----------------------------------------------------------------
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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] adoroszlai commented on a change in pull request #1619: HDDS-4503. Provide info on block size via FileSystem

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #1619:
URL: https://github.com/apache/ozone/pull/1619#discussion_r541547602



##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneFileSystem.java
##########
@@ -698,6 +701,12 @@ public boolean mkdirs(Path f, FsPermission permission) throws IOException {
     return mkdir(f);
   }
 
+  @Override
+  public long getDefaultBlockSize() {
+    return (long) getConf().getStorageSize(
+        OZONE_SCM_BLOCK_SIZE, OZONE_SCM_BLOCK_SIZE_DEFAULT, StorageUnit.BYTES);

Review comment:
       Thanks @arp7 for taking a look.  Does the above answer your quetion, or would you like further clarification?




----------------------------------------------------------------
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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org