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 2022/10/06 16:06:33 UTC
[GitHub] [ozone] devmadhuu opened a new pull request, #3803: HDDS-7290. provide a config to increase the list batch size in OzoneFileSystem
devmadhuu opened a new pull request, #3803:
URL: https://github.com/apache/ozone/pull/3803
## What changes were proposed in this pull request?
Currently the value of list page size for listing items of ozone fs sub-commands output is set as hardcoded value 1024.
A new configuration "ozone.client.fs.default.listing.page.size" will be provided to configure the value.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-7290?filter=-1
## How was this patch tested?
Manual testing and configured value is reflecting.
<img width="753" alt="image" src="https://user-images.githubusercontent.com/20607992/194363123-9133a669-154d-4ef1-8062-5f54a102e915.png">
<img width="469" alt="image" src="https://user-images.githubusercontent.com/20607992/194363312-e444decc-1091-49c0-bd70-3c59c0651b8f.png">
--
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] adoroszlai commented on a diff in pull request #3803: HDDS-7290. provide a config to increase the list batch size in OzoneFileSystem
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on code in PR #3803:
URL: https://github.com/apache/ozone/pull/3803#discussion_r989271276
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneFileSystem.java:
##########
@@ -91,6 +94,8 @@ public class BasicOzoneFileSystem extends FileSystem {
static final Logger LOG =
LoggerFactory.getLogger(BasicOzoneFileSystem.class);
+ private static int listingPageSize = 1024;
Review Comment:
I recommend setting it to `OZONE_CLIENT_FS_LISTING_PAGE_SIZE_DEFAULT` to avoid duplication.
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java:
##########
@@ -405,4 +405,17 @@ private OMConfigKeys() {
public static final TimeDuration OZONE_OM_CONTAINER_LOCATION_CACHE_TTL_DEFAULT
= TimeDuration.valueOf(360, TimeUnit.MINUTES);
+
+ // Items listing page size for fs client sub-commands output
+ public static final String OZONE_CLIENT_FS_MAX_LISTING_PAGE_SIZE =
+ "ozone.client.fs.max.listing.page.size";
+
+ public static final String
+ OZONE_CLIENT_FS_LISTING_PAGE_SIZE =
+ "ozone.client.fs.default.listing.page.size";
+ public static final int
+ OZONE_CLIENT_FS_LISTING_PAGE_SIZE_DEFAULT = 1024;
Review Comment:
`OMConfigKeys` is for OM server-side settings. Since this setting is for the client, I think `OzoneConfigKeys` would be a better place for it.
##########
hadoop-hdds/common/src/main/resources/ozone-default.xml:
##########
@@ -3358,5 +3358,22 @@
</description>
</property>
+ <property>
+ <name>ozone.client.fs.default.listing.page.size</name>
+ <value>1024</value>
+ <tag>OZONE, CLIENT</tag>
+ <description>
+ Default listing page size value used for listing items on fs related sub-commands.
+ </description>
+ </property>
+
+ <property>
+ <name>ozone.client.fs.max.listing.page.size</name>
Review Comment:
I think this config property is unnecessary. Consider the case where a user wants to use page size P, which is considered too large. They can just set `ozone.client.fs.max.listing.page.size` to P+1 and bypass the limit.
##########
hadoop-hdds/common/src/main/resources/ozone-default.xml:
##########
@@ -3358,5 +3358,22 @@
</description>
</property>
+ <property>
+ <name>ozone.client.fs.default.listing.page.size</name>
+ <value>1024</value>
+ <tag>OZONE, CLIENT</tag>
+ <description>
+ Default listing page size value used for listing items on fs related sub-commands.
Review Comment:
Can users override this config any way (e.g. by some command-line option)? If not, then I think "default" is unnecessary in this name/description.
--
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] devmadhuu commented on a diff in pull request #3803: HDDS-7290. provide a config to increase the list batch size in OzoneFileSystem
Posted by GitBox <gi...@apache.org>.
devmadhuu commented on code in PR #3803:
URL: https://github.com/apache/ozone/pull/3803#discussion_r989673096
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneFileSystem.java:
##########
@@ -91,6 +94,8 @@ public class BasicOzoneFileSystem extends FileSystem {
static final Logger LOG =
LoggerFactory.getLogger(BasicOzoneFileSystem.class);
+ private static int listingPageSize = 1024;
Review Comment:
yes, resolved.
--
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] devmadhuu commented on a diff in pull request #3803: HDDS-7290. provide a config to increase the list batch size in OzoneFileSystem
Posted by GitBox <gi...@apache.org>.
devmadhuu commented on code in PR #3803:
URL: https://github.com/apache/ozone/pull/3803#discussion_r990147652
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java:
##########
@@ -405,4 +405,17 @@ private OMConfigKeys() {
public static final TimeDuration OZONE_OM_CONTAINER_LOCATION_CACHE_TTL_DEFAULT
= TimeDuration.valueOf(360, TimeUnit.MINUTES);
+
+ // Items listing page size for fs client sub-commands output
+ public static final String OZONE_CLIENT_FS_MAX_LISTING_PAGE_SIZE =
+ "ozone.client.fs.max.listing.page.size";
+
+ public static final String
+ OZONE_CLIENT_FS_LISTING_PAGE_SIZE =
+ "ozone.client.fs.default.listing.page.size";
+ public static final int
+ OZONE_CLIENT_FS_LISTING_PAGE_SIZE_DEFAULT = 1024;
Review Comment:
@adoroszlai , pls re-review, comment is fixed.
--
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] devmadhuu commented on a diff in pull request #3803: HDDS-7290. provide a config to increase the list batch size in OzoneFileSystem
Posted by GitBox <gi...@apache.org>.
devmadhuu commented on code in PR #3803:
URL: https://github.com/apache/ozone/pull/3803#discussion_r991825052
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java:
##########
@@ -92,6 +94,9 @@ public class BasicRootedOzoneFileSystem extends FileSystem {
static final Logger LOG =
LoggerFactory.getLogger(BasicRootedOzoneFileSystem.class);
+ private static int listingPageSize =
+ OZONE_CLIENT_FS_LISTING_PAGE_SIZE_DEFAULT;
Review Comment:
Thanks @adoroszlai for reviewing this PR. I'll run all checks locally now before pushing. I didn't aware of this earlier. Thanks for guiding me. Please re-review. Its fixed and passing all checks
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneFileSystem.java:
##########
@@ -91,6 +93,9 @@ public class BasicOzoneFileSystem extends FileSystem {
static final Logger LOG =
LoggerFactory.getLogger(BasicOzoneFileSystem.class);
+ private static int listingPageSize =
+ OZONE_CLIENT_FS_LISTING_PAGE_SIZE_DEFAULT;
Review Comment:
Thanks @adoroszlai for reviewing this PR. I'll run all checks locally now before pushing. I didn't aware of this earlier. Thanks for guiding me. Please re-review. Its fixed and passing all checks
--
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] devmadhuu commented on pull request #3803: HDDS-7290. provide a config to increase the list batch size in OzoneFileSystem
Posted by GitBox <gi...@apache.org>.
devmadhuu commented on PR #3803:
URL: https://github.com/apache/ozone/pull/3803#issuecomment-1270335999
@nandakumar131 @adoroszlai , pls review the change.
--
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] devmadhuu commented on a diff in pull request #3803: HDDS-7290. provide a config to increase the list batch size in OzoneFileSystem
Posted by GitBox <gi...@apache.org>.
devmadhuu commented on code in PR #3803:
URL: https://github.com/apache/ozone/pull/3803#discussion_r989673863
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java:
##########
@@ -405,4 +405,17 @@ private OMConfigKeys() {
public static final TimeDuration OZONE_OM_CONTAINER_LOCATION_CACHE_TTL_DEFAULT
= TimeDuration.valueOf(360, TimeUnit.MINUTES);
+
+ // Items listing page size for fs client sub-commands output
+ public static final String OZONE_CLIENT_FS_MAX_LISTING_PAGE_SIZE =
+ "ozone.client.fs.max.listing.page.size";
+
+ public static final String
+ OZONE_CLIENT_FS_LISTING_PAGE_SIZE =
+ "ozone.client.fs.default.listing.page.size";
+ public static final int
+ OZONE_CLIENT_FS_LISTING_PAGE_SIZE_DEFAULT = 1024;
Review Comment:
Ok
--
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] adoroszlai commented on a diff in pull request #3803: HDDS-7290. provide a config to increase the list batch size in OzoneFileSystem
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on code in PR #3803:
URL: https://github.com/apache/ozone/pull/3803#discussion_r991524180
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneFileSystem.java:
##########
@@ -91,6 +93,9 @@ public class BasicOzoneFileSystem extends FileSystem {
static final Logger LOG =
LoggerFactory.getLogger(BasicOzoneFileSystem.class);
+ private static int listingPageSize =
+ OZONE_CLIENT_FS_LISTING_PAGE_SIZE_DEFAULT;
Review Comment:
Please change this to not be `static`, as findbugs complains about it.
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java:
##########
@@ -92,6 +94,9 @@ public class BasicRootedOzoneFileSystem extends FileSystem {
static final Logger LOG =
LoggerFactory.getLogger(BasicRootedOzoneFileSystem.class);
+ private static int listingPageSize =
+ OZONE_CLIENT_FS_LISTING_PAGE_SIZE_DEFAULT;
Review Comment:
Please change this to not be `static`, as findbugs complains about 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.
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] devmadhuu commented on pull request #3803: HDDS-7290. provide a config to increase the list batch size in OzoneFileSystem
Posted by GitBox <gi...@apache.org>.
devmadhuu commented on PR #3803:
URL: https://github.com/apache/ozone/pull/3803#issuecomment-1280244916
>
@adoroszlai , valid point, however before doing this change I noticed that same hard coded variable value was being used for all fs operations , I just made it configurable that variable. Pls let me know if I am missing something.
--
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] devmadhuu commented on a diff in pull request #3803: HDDS-7290. provide a config to increase the list batch size in OzoneFileSystem
Posted by GitBox <gi...@apache.org>.
devmadhuu commented on code in PR #3803:
URL: https://github.com/apache/ozone/pull/3803#discussion_r990146889
##########
hadoop-hdds/common/src/main/resources/ozone-default.xml:
##########
@@ -3358,5 +3358,22 @@
</description>
</property>
+ <property>
+ <name>ozone.client.fs.default.listing.page.size</name>
+ <value>1024</value>
+ <tag>OZONE, CLIENT</tag>
+ <description>
+ Default listing page size value used for listing items on fs related sub-commands.
Review Comment:
@adoroszlai , pls re-review. Comment is fixed.
--
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] devmadhuu commented on a diff in pull request #3803: HDDS-7290. provide a config to increase the list batch size in OzoneFileSystem
Posted by GitBox <gi...@apache.org>.
devmadhuu commented on code in PR #3803:
URL: https://github.com/apache/ozone/pull/3803#discussion_r989672198
##########
hadoop-hdds/common/src/main/resources/ozone-default.xml:
##########
@@ -3358,5 +3358,22 @@
</description>
</property>
+ <property>
+ <name>ozone.client.fs.default.listing.page.size</name>
+ <value>1024</value>
+ <tag>OZONE, CLIENT</tag>
+ <description>
+ Default listing page size value used for listing items on fs related sub-commands.
+ </description>
+ </property>
+
+ <property>
+ <name>ozone.client.fs.max.listing.page.size</name>
Review Comment:
@nandakumar131 , pls suggest on this comment. I already had discussion with @adoroszlai on this comment.
--
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] devmadhuu commented on a diff in pull request #3803: HDDS-7290. provide a config to increase the list batch size in OzoneFileSystem
Posted by GitBox <gi...@apache.org>.
devmadhuu commented on code in PR #3803:
URL: https://github.com/apache/ozone/pull/3803#discussion_r990147240
##########
hadoop-hdds/common/src/main/resources/ozone-default.xml:
##########
@@ -3358,5 +3358,22 @@
</description>
</property>
+ <property>
+ <name>ozone.client.fs.default.listing.page.size</name>
+ <value>1024</value>
+ <tag>OZONE, CLIENT</tag>
+ <description>
+ Default listing page size value used for listing items on fs related sub-commands.
+ </description>
+ </property>
+
+ <property>
+ <name>ozone.client.fs.max.listing.page.size</name>
Review Comment:
@adoroszlai , pls re-review, comment is fixed.
--
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] adoroszlai commented on pull request #3803: HDDS-7290. provide a config to increase the list batch size in OzoneFileSystem
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on PR #3803:
URL: https://github.com/apache/ozone/pull/3803#issuecomment-1280392957
> owever before doing this change I noticed that same hard coded variable value was being used for all fs operations , I just made it configurable that variable. Pls let me know if I am missing something.
Previously there was no precondition check, since the value was hard-coded. Adding a check this way affects all operations, even those that to which page size does not apply.
Instead of throwing exception if the page size config exceeds max. allowed, how about limiting it (and maybe logging a warning)?
--
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] devmadhuu closed pull request #3803: HDDS-7290. provide a config to increase the list batch size in OzoneFileSystem
Posted by GitBox <gi...@apache.org>.
devmadhuu closed pull request #3803: HDDS-7290. provide a config to increase the list batch size in OzoneFileSystem
URL: https://github.com/apache/ozone/pull/3803
--
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