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/07 21:14:54 UTC
[GitHub] [ozone] DaveTeng0 opened a new pull request, #3810: HDDS-7260. Fix the issue that the "-du" command does not return correct disk consumed with replica for both ratis and EC
DaveTeng0 opened a new pull request, #3810:
URL: https://github.com/apache/ozone/pull/3810
## What changes were proposed in this pull request?
Fix the issue that the "-du" command does not return correct disk consumed with replica for both ratis and EC
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-7260
## How was this patch tested?
Manual tests. (WIP)
Unit tests. (WIP)
Robot tests. (WIP)
--
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] DaveTeng0 commented on pull request #3810: HDDS-7260. Fix the issue that the "-du" command does not return correct disk consumed with replica for both ratis and EC
Posted by GitBox <gi...@apache.org>.
DaveTeng0 commented on PR #3810:
URL: https://github.com/apache/ozone/pull/3810#issuecomment-1275121207
>
Sure!! will take a look the failed tests!
--
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] DaveTeng0 commented on pull request #3810: HDDS-7260. Fix the issue that the "-du" command does not return correct disk consumed with replica for both ratis and EC
Posted by GitBox <gi...@apache.org>.
DaveTeng0 commented on PR #3810:
URL: https://github.com/apache/ozone/pull/3810#issuecomment-1275120629
>
Sure!! will resolve the conflict and update unit tests!
--
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] DaveTeng0 commented on pull request #3810: HDDS-7260. dfs -du should return correct disk space consumed by all replicas
Posted by GitBox <gi...@apache.org>.
DaveTeng0 commented on PR #3810:
URL: https://github.com/apache/ozone/pull/3810#issuecomment-1315761153
>
sure! will create a separate ticket!
--
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] DaveTeng0 commented on a diff in pull request #3810: HDDS-7260. dfs -du should return correct disk space consumed by all replicas
Posted by GitBox <gi...@apache.org>.
DaveTeng0 commented on code in PR #3810:
URL: https://github.com/apache/ozone/pull/3810#discussion_r1027708709
##########
hadoop-ozone/dist/src/main/compose/ozonesecure/test.sh:
##########
@@ -27,7 +27,7 @@ export SECURITY_ENABLED=true
: ${OZONE_BUCKET_KEY_NAME:=key1}
-start_docker_env
+start_docker_env 5
Review Comment:
ohh!! thanks Atila for the hint!! (I did encounter that test failure frequently, now that make sense!
--
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] DaveTeng0 commented on a diff in pull request #3810: HDDS-7260. dfs -du should return correct disk space consumed by all replicas
Posted by GitBox <gi...@apache.org>.
DaveTeng0 commented on code in PR #3810:
URL: https://github.com/apache/ozone/pull/3810#discussion_r1020999057
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java:
##########
@@ -732,20 +733,33 @@ private boolean o3Exists(final Path f) throws IOException {
@Override
public FileStatus[] listStatus(Path f) throws IOException {
+ return convertFileStatusArr(listStatusAdapter(f));
+ }
+
+ private FileStatus[] convertFileStatusArr(FileStatusAdapter[] adapterArr) {
+ FileStatus[] fileStatuses = new FileStatus[adapterArr.length];
+ int index = 0;
+ for (FileStatusAdapter statusAdapter : adapterArr) {
+ fileStatuses[index++] = convertFileStatus(statusAdapter);
+ }
+ return fileStatuses;
+ }
+
+
+ public FileStatusAdapter[] listStatusAdapter(Path f) throws IOException {
Review Comment:
Yes, will do!
--
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] DaveTeng0 commented on a diff in pull request #3810: HDDS-7260. dfs -du should return correct disk space consumed by all replicas
Posted by GitBox <gi...@apache.org>.
DaveTeng0 commented on code in PR #3810:
URL: https://github.com/apache/ozone/pull/3810#discussion_r1020998970
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java:
##########
@@ -450,7 +451,7 @@ private class DeleteIterator extends OzoneListingIterator {
throws IOException {
super(f);
this.recursive = recursive;
- if (getStatus().isDirectory()
+ if (getStatus().isDir()
Review Comment:
Yes! it's because Ozone uses a class called FileStatusAdapter, which is similar with the class FileStatus in hdfs. The main difference is that FileStatusAdapter class is built in with attribute of disk usage of file, which we'll use in the 'du' command!
--
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] sodonnel commented on a diff in pull request #3810: HDDS-7260. Fix the issue that the "-du" command does not return correct disk consumed with replica for both ratis and EC
Posted by GitBox <gi...@apache.org>.
sodonnel commented on code in PR #3810:
URL: https://github.com/apache/ozone/pull/3810#discussion_r991171706
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneClientAdapterImpl.java:
##########
@@ -521,8 +521,12 @@ private FileStatusAdapter toFileStatusAdapter(OzoneFileStatus status,
OmKeyInfo keyInfo = status.getKeyInfo();
short replication = (short) keyInfo.getReplicationConfig()
.getRequiredNodes();
+ //calculate the number of bytes required to store the dataSize with replication
+ long diskConsumed = keyInfo.getReplicatedSize();
Review Comment:
I don't think we need this variable - just use the `keyInfo.getReplicatedSize()` call inline.
--
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] jojochuang merged pull request #3810: HDDS-7260. dfs -du should return correct disk space consumed by all replicas
Posted by GitBox <gi...@apache.org>.
jojochuang merged PR #3810:
URL: https://github.com/apache/ozone/pull/3810
--
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] DaveTeng0 commented on a diff in pull request #3810: HDDS-7260. Fix the issue that the "-du" command does not return correct disk consumed with replica for both ratis and EC
Posted by GitBox <gi...@apache.org>.
DaveTeng0 commented on code in PR #3810:
URL: https://github.com/apache/ozone/pull/3810#discussion_r992667257
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java:
##########
@@ -944,8 +944,11 @@ private FileStatusAdapter toFileStatusAdapter(OzoneFileStatus status,
OmKeyInfo keyInfo = status.getKeyInfo();
short replication = (short) keyInfo.getReplicationConfig()
.getRequiredNodes();
+ //calculate the number of bytes required to store the dataSize with replication
+ long diskConsumed = keyInfo.getReplicatedSize();
Review Comment:
yes!!
--
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] DaveTeng0 commented on a diff in pull request #3810: HDDS-7260. dfs -du should return correct disk space consumed by all replicas
Posted by GitBox <gi...@apache.org>.
DaveTeng0 commented on code in PR #3810:
URL: https://github.com/apache/ozone/pull/3810#discussion_r1023170987
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java:
##########
@@ -773,7 +787,7 @@ numEntries, uri, workingDir, getUsername())
// exhausted.
} while (tmpStatusList.size() == numEntries);
- return statuses.toArray(new FileStatus[0]);
+ return statuses.toArray(new FileStatusAdapter[0]);
Review Comment:
ohh!! yes!!
--
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] jojochuang commented on a diff in pull request #3810: HDDS-7260. dfs -du should return correct disk space consumed by all replicas
Posted by GitBox <gi...@apache.org>.
jojochuang commented on code in PR #3810:
URL: https://github.com/apache/ozone/pull/3810#discussion_r1019492094
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java:
##########
@@ -1157,19 +1175,19 @@ public ConfigurationSource getConfSource() {
*/
private abstract class OzoneListingIterator {
private final Path path;
- private final FileStatus status;
+ private final FileStatusAdapter status;
private String pathKey;
private Iterator<BasicKeyInfo> keyIterator = null;
private boolean isFSO;
OzoneListingIterator(Path path, boolean isFSO)
throws IOException {
this.path = path;
- this.status = getFileStatus(path);
+ this.status = getFileStatusAdapter(path);
this.pathKey = pathToKey(path);
this.isFSO = isFSO;
if (!isFSO) {
- if (status.isDirectory()) {
+ if (status.isDir()) {
Review Comment:
ditto
##########
hadoop-ozone/dist/src/main/compose/ozonesecure/test.sh:
##########
@@ -27,7 +27,7 @@ export SECURITY_ENABLED=true
: ${OZONE_BUCKET_KEY_NAME:=key1}
-start_docker_env
+start_docker_env 5
Review Comment:
Is this change required? Just curious
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java:
##########
@@ -450,7 +451,7 @@ private class DeleteIterator extends OzoneListingIterator {
throws IOException {
super(f);
this.recursive = recursive;
- if (getStatus().isDirectory()
+ if (getStatus().isDir()
Review Comment:
Isn't this a deprecated API and which is the same as isDirectory()?
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java:
##########
@@ -1210,15 +1228,15 @@ boolean iterate() throws IOException {
List<String> keyPathList = new ArrayList<>();
int batchSize = getConf().getInt(OZONE_FS_ITERATE_BATCH_SIZE,
OZONE_FS_ITERATE_BATCH_SIZE_DEFAULT);
- if (status.isDirectory()) {
+ if (status.isDir()) {
Review Comment:
ditto
--
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] DaveTeng0 commented on a diff in pull request #3810: HDDS-7260. dfs -du should return correct disk space consumed by all replicas
Posted by GitBox <gi...@apache.org>.
DaveTeng0 commented on code in PR #3810:
URL: https://github.com/apache/ozone/pull/3810#discussion_r1020998031
##########
hadoop-ozone/dist/src/main/compose/ozonesecure/test.sh:
##########
@@ -27,7 +27,7 @@ export SECURITY_ENABLED=true
: ${OZONE_BUCKET_KEY_NAME:=key1}
-start_docker_env
+start_docker_env 5
Review Comment:
Yes! I added a unit test to verify the EC file's disk usage, I found the minimum number of datanode for EC is 5! (if not specify, the default number of datanode launched during the test is 3.
--
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] umamaheswararao commented on pull request #3810: HDDS-7260. dfs -du should return correct disk space consumed by all replicas
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on PR #3810:
URL: https://github.com/apache/ozone/pull/3810#issuecomment-1314277490
@jojochuang and @sodonnel, could you please check once if we can proceed with this? ( Pending CI)
--
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] DaveTeng0 commented on a diff in pull request #3810: HDDS-7260. dfs -du should return correct disk space consumed by all replicas
Posted by GitBox <gi...@apache.org>.
DaveTeng0 commented on code in PR #3810:
URL: https://github.com/apache/ozone/pull/3810#discussion_r1023172406
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/FileStatusAdapter.java:
##########
@@ -131,4 +142,27 @@ public BlockLocation[] getBlockLocations() {
return blockLocations;
}
+ @Override
+ public String toString() {
+ StringBuilder sb = new StringBuilder();
+ sb.append(getClass().getSimpleName())
+ .append("{")
+ .append("path=" + path)
+ .append("; isDirectory=" + isdir);
+ if (isFile()) {
+ sb.append("; length=" + length)
+ .append("; diskConsumed= " + getDiskConsumed())
+ .append("; blockReplication=" + blockReplication)
+ .append("; blocksize=" + blocksize);
+ }
+ sb.append("; accessTime=" + accessTime)
+ .append("; owner=" + owner)
+ .append("; group=" + group)
+ .append("; permission=" + permission)
+ .append("; isSymlink=" + getSymlink())
+ .append("}");
Review Comment:
yes!! good catch!
--
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] jojochuang commented on a diff in pull request #3810: HDDS-7260. dfs -du should return correct disk space consumed by all replicas
Posted by GitBox <gi...@apache.org>.
jojochuang commented on code in PR #3810:
URL: https://github.com/apache/ozone/pull/3810#discussion_r1019504563
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java:
##########
@@ -732,20 +733,33 @@ private boolean o3Exists(final Path f) throws IOException {
@Override
public FileStatus[] listStatus(Path f) throws IOException {
+ return convertFileStatusArr(listStatusAdapter(f));
+ }
+
+ private FileStatus[] convertFileStatusArr(FileStatusAdapter[] adapterArr) {
+ FileStatus[] fileStatuses = new FileStatus[adapterArr.length];
+ int index = 0;
+ for (FileStatusAdapter statusAdapter : adapterArr) {
+ fileStatuses[index++] = convertFileStatus(statusAdapter);
+ }
+ return fileStatuses;
+ }
+
+
+ public FileStatusAdapter[] listStatusAdapter(Path f) throws IOException {
Review Comment:
I think we should rewrite it to return List<FileStatusAdapter>.
There's no point in creating a temp array that is going to be discarded right away.
--
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] jojochuang commented on a diff in pull request #3810: HDDS-7260. dfs -du should return correct disk space consumed by all replicas
Posted by GitBox <gi...@apache.org>.
jojochuang commented on code in PR #3810:
URL: https://github.com/apache/ozone/pull/3810#discussion_r1019504563
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java:
##########
@@ -732,20 +733,33 @@ private boolean o3Exists(final Path f) throws IOException {
@Override
public FileStatus[] listStatus(Path f) throws IOException {
+ return convertFileStatusArr(listStatusAdapter(f));
+ }
+
+ private FileStatus[] convertFileStatusArr(FileStatusAdapter[] adapterArr) {
+ FileStatus[] fileStatuses = new FileStatus[adapterArr.length];
+ int index = 0;
+ for (FileStatusAdapter statusAdapter : adapterArr) {
+ fileStatuses[index++] = convertFileStatus(statusAdapter);
+ }
+ return fileStatuses;
+ }
+
+
+ public FileStatusAdapter[] listStatusAdapter(Path f) throws IOException {
Review Comment:
I think we should rewrite it to return `List<FileStatusAdapter>`. That is, get rid of `statuses`.
There's no point in creating a temp array that is going to be discarded right away.
--
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] jojochuang commented on a diff in pull request #3810: HDDS-7260. dfs -du should return correct disk space consumed by all replicas
Posted by GitBox <gi...@apache.org>.
jojochuang commented on code in PR #3810:
URL: https://github.com/apache/ozone/pull/3810#discussion_r1022177934
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/FileStatusAdapter.java:
##########
@@ -131,4 +142,27 @@ public BlockLocation[] getBlockLocations() {
return blockLocations;
}
+ @Override
+ public String toString() {
+ StringBuilder sb = new StringBuilder();
+ sb.append(getClass().getSimpleName())
+ .append("{")
+ .append("path=" + path)
+ .append("; isDirectory=" + isdir);
+ if (isFile()) {
+ sb.append("; length=" + length)
+ .append("; diskConsumed= " + getDiskConsumed())
+ .append("; blockReplication=" + blockReplication)
+ .append("; blocksize=" + blocksize);
+ }
+ sb.append("; accessTime=" + accessTime)
+ .append("; owner=" + owner)
+ .append("; group=" + group)
+ .append("; permission=" + permission)
+ .append("; isSymlink=" + getSymlink())
+ .append("}");
Review Comment:
IMO better to rewrite by calling more append rather than string concatenation.
e.g.
```suggestion
.append("path=").append(path)
...
```
--
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] jojochuang commented on a diff in pull request #3810: HDDS-7260. dfs -du should return correct disk space consumed by all replicas
Posted by GitBox <gi...@apache.org>.
jojochuang commented on code in PR #3810:
URL: https://github.com/apache/ozone/pull/3810#discussion_r1022220935
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java:
##########
@@ -773,7 +787,7 @@ numEntries, uri, workingDir, getUsername())
// exhausted.
} while (tmpStatusList.size() == numEntries);
- return statuses.toArray(new FileStatus[0]);
+ return statuses.toArray(new FileStatusAdapter[0]);
Review Comment:
is it possible to return statuses instead? Save one temporary array allocation.
--
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] DaveTeng0 commented on a diff in pull request #3810: HDDS-7260. Fix the issue that the "-du" command does not return correct disk consumed with replica for both ratis and EC
Posted by GitBox <gi...@apache.org>.
DaveTeng0 commented on code in PR #3810:
URL: https://github.com/apache/ozone/pull/3810#discussion_r992667596
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneClientAdapterImpl.java:
##########
@@ -521,8 +521,12 @@ private FileStatusAdapter toFileStatusAdapter(OzoneFileStatus status,
OmKeyInfo keyInfo = status.getKeyInfo();
short replication = (short) keyInfo.getReplicationConfig()
.getRequiredNodes();
+ //calculate the number of bytes required to store the dataSize with replication
+ long diskConsumed = keyInfo.getReplicatedSize();
Review Comment:
yes!! definitely!
--
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 #3810: HDDS-7260. dfs -du should return correct disk space consumed by all replicas
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on code in PR #3810:
URL: https://github.com/apache/ozone/pull/3810#discussion_r1027139834
##########
hadoop-ozone/dist/src/main/compose/ozonesecure/test.sh:
##########
@@ -27,7 +27,7 @@ export SECURITY_ENABLED=true
: ${OZONE_BUCKET_KEY_NAME:=key1}
-start_docker_env
+start_docker_env 5
Review Comment:
This causes intermittent, but frequent failure in the replication test.
https://github.com/apache/ozone/blob/85e7cd1867ec9000df798c74ab0f9cf153936a5d/hadoop-ozone/dist/src/main/compose/ozonesecure/test.sh#L57-L61
The test scales datanodes to 2, waits for container replica count = 2, then scales to 3, and waits for replica count = 3.
With 3 initial datanodes, the test can assume all nodes have the container, so scaling to 2 then 3 datanodes, the container count always matches expectations, if replication works correctly.
Now with 5 initial datanodes, when the test scales datanodes to 2, the container may have 0, 1 or 2 healthy replicas left, depending on where the original 3 replicas were stored. And when datanodes are scaled to 3, the container may have 1, 2 or 3 replicas.
Thus the test fails frequently, but not in 100% of runs.
* https://github.com/adoroszlai/ozone-build-results/tree/master/2022/11/18/18547/acceptance-secure/
* https://github.com/adoroszlai/ozone-build-results/tree/master/2022/11/18/18566/acceptance-secure/
* https://github.com/adoroszlai/ozone-build-results/tree/master/2022/11/18/18559/acceptance-secure/
* https://github.com/adoroszlai/ozone-build-results/tree/master/2022/11/18/18554/acceptance-secure/
* https://github.com/adoroszlai/ozone-build-results/tree/master/2022/11/19/18570/acceptance-secure/
Filed HDDS-7518 for 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
[GitHub] [ozone] adoroszlai commented on a diff in pull request #3810: HDDS-7260. dfs -du should return correct disk space consumed by all replicas
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on code in PR #3810:
URL: https://github.com/apache/ozone/pull/3810#discussion_r1027139834
##########
hadoop-ozone/dist/src/main/compose/ozonesecure/test.sh:
##########
@@ -27,7 +27,7 @@ export SECURITY_ENABLED=true
: ${OZONE_BUCKET_KEY_NAME:=key1}
-start_docker_env
+start_docker_env 5
Review Comment:
This causes intermittent, but frequent failure in the replication test.
https://github.com/apache/ozone/blob/85e7cd1867ec9000df798c74ab0f9cf153936a5d/hadoop-ozone/dist/src/main/compose/ozonesecure/test.sh#L57-L61
The test scales datanodes to 2, waits for container replica count = 2, then scales to 3, and waits for replica count = 3.
With 3 initial datanodes, the test can assume all nodes have the container, so scaling to 2 then 3 datanodes, the container count always matches expectations, if replication works correctly.
Now with 5 initial datanodes, when the test scales datanodes to 2, the container may have 0, 1 or 2 healthy replicas left, depending on where the original 3 replicas were stored. And when datanodes are scaled to 3, the container may have 1, 2 or 3 replicas.
Thus the test fails frequently, but not in 100% of runs.
* https://github.com/adoroszlai/ozone-build-results/tree/master/2022/11/18/18547/acceptance-secure/
* https://github.com/adoroszlai/ozone-build-results/tree/master/2022/11/18/18566/acceptance-secure/
* https://github.com/adoroszlai/ozone-build-results/tree/master/2022/11/18/18559/acceptance-secure/
* https://github.com/adoroszlai/ozone-build-results/tree/master/2022/11/18/18554/acceptance-secure/
* https://github.com/adoroszlai/ozone-build-results/tree/master/2022/11/19/18570/acceptance-secure/
--
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 #3810: HDDS-7260. dfs -du should return correct disk space consumed by all replicas
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on code in PR #3810:
URL: https://github.com/apache/ozone/pull/3810#discussion_r1027139834
##########
hadoop-ozone/dist/src/main/compose/ozonesecure/test.sh:
##########
@@ -27,7 +27,7 @@ export SECURITY_ENABLED=true
: ${OZONE_BUCKET_KEY_NAME:=key1}
-start_docker_env
+start_docker_env 5
Review Comment:
This causes intermittent, but frequent failure in the replication test.
https://github.com/apache/ozone/blob/85e7cd1867ec9000df798c74ab0f9cf153936a5d/hadoop-ozone/dist/src/main/compose/ozonesecure/test.sh#L57-L61
The test scales datanodes to 2, waits for container replica count = 2, then scales to 3, and waits for replica count = 3.
With 3 initial datanodes, the test can assume all nodes have the container, so scaling to 2 then 3 datanodes, the container count always matches expectations, if replication works correctly.
Now with 5 initial datanodes, when the test scales datanodes to 2, the container may have 0, 1 or 2 healthy replicas left, depending on where the original 3 replicas were stored. And when datanodes are scaled to 3, the container may have 1, 2 or 3 replicas.
Thus the test fails frequently, but not in 100% of runs.
--
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] DaveTeng0 commented on a diff in pull request #3810: HDDS-7260. dfs -du should return correct disk space consumed by all replicas
Posted by GitBox <gi...@apache.org>.
DaveTeng0 commented on code in PR #3810:
URL: https://github.com/apache/ozone/pull/3810#discussion_r1027708709
##########
hadoop-ozone/dist/src/main/compose/ozonesecure/test.sh:
##########
@@ -27,7 +27,7 @@ export SECURITY_ENABLED=true
: ${OZONE_BUCKET_KEY_NAME:=key1}
-start_docker_env
+start_docker_env 5
Review Comment:
ohh!! thanks Atila for the hint!! (I did encounter that test failure frequently, now that makes sense!
--
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] jojochuang commented on a diff in pull request #3810: HDDS-7260. dfs -du should return correct disk space consumed by all replicas
Posted by GitBox <gi...@apache.org>.
jojochuang commented on code in PR #3810:
URL: https://github.com/apache/ozone/pull/3810#discussion_r1019504563
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java:
##########
@@ -732,20 +733,33 @@ private boolean o3Exists(final Path f) throws IOException {
@Override
public FileStatus[] listStatus(Path f) throws IOException {
+ return convertFileStatusArr(listStatusAdapter(f));
+ }
+
+ private FileStatus[] convertFileStatusArr(FileStatusAdapter[] adapterArr) {
+ FileStatus[] fileStatuses = new FileStatus[adapterArr.length];
+ int index = 0;
+ for (FileStatusAdapter statusAdapter : adapterArr) {
+ fileStatuses[index++] = convertFileStatus(statusAdapter);
+ }
+ return fileStatuses;
+ }
+
+
+ public FileStatusAdapter[] listStatusAdapter(Path f) throws IOException {
Review Comment:
I think we should rewrite it to return `List<FileStatusAdapter>`.
There's no point in creating a temp array that is going to be discarded right away.
--
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] sodonnel commented on a diff in pull request #3810: HDDS-7260. Fix the issue that the "-du" command does not return correct disk consumed with replica for both ratis and EC
Posted by GitBox <gi...@apache.org>.
sodonnel commented on code in PR #3810:
URL: https://github.com/apache/ozone/pull/3810#discussion_r991172063
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java:
##########
@@ -944,8 +944,11 @@ private FileStatusAdapter toFileStatusAdapter(OzoneFileStatus status,
OmKeyInfo keyInfo = status.getKeyInfo();
short replication = (short) keyInfo.getReplicationConfig()
.getRequiredNodes();
+ //calculate the number of bytes required to store the dataSize with replication
+ long diskConsumed = keyInfo.getReplicatedSize();
Review Comment:
Same as above, I think it would be cleaner to just use the `keyInfo.getReplicatedSize()` call inline.
--
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