You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2021/08/02 05:57:34 UTC
[GitHub] [ozone] sadanand48 opened a new pull request #2463: HDDS-5483. Validate block file length during put block
sadanand48 opened a new pull request #2463:
URL: https://github.com/apache/ozone/pull/2463
## What changes were proposed in this pull request?
Validate block file length during put block .
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-5483
## How was this patch tested?
Only a validation check.
--
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] szetszwo commented on pull request #2463: HDDS-5483. Validate block file length during put block
Posted by GitBox <gi...@apache.org>.
szetszwo commented on pull request #2463:
URL: https://github.com/apache/ozone/pull/2463#issuecomment-897281571
Should this be merged to the master branch instead the HDDS-4454 branch? This dose not look like specific to HDDS-4454.
--
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] szetszwo commented on pull request #2463: HDDS-5483. Validate block file length during put block
Posted by GitBox <gi...@apache.org>.
szetszwo commented on pull request #2463:
URL: https://github.com/apache/ozone/pull/2463#issuecomment-900207476
Thanks for the response. Just have merged this to the master 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.
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] mukul1987 merged pull request #2463: HDDS-5483. Validate block file length during put block
Posted by GitBox <gi...@apache.org>.
mukul1987 merged pull request #2463:
URL: https://github.com/apache/ozone/pull/2463
--
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 change in pull request #2463: HDDS-5483. Validate block file length during put block
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #2463:
URL: https://github.com/apache/ozone/pull/2463#discussion_r690283532
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java
##########
@@ -460,6 +463,20 @@ ContainerCommandResponseProto handlePutBlock(
ContainerProtos.BlockData data = request.getPutBlock().getBlockData();
BlockData blockData = BlockData.getFromProtoBuf(data);
Preconditions.checkNotNull(blockData);
+ ChunkLayOutVersion layoutVersion =
+ ChunkLayOutVersion.getConfiguredVersion(conf);
+ if (layoutVersion == FILE_PER_BLOCK &&
+ // ChunkManagerDummyImpl doesn't persist to disk, don't check here
+ !chunkManager.getClass()
+ .isAssignableFrom(ChunkManagerDummyImpl.class)) {
Review comment:
I think layout-specific code would be better added to each `ChunkManager` implementation instead of such `==` and `isAssignableFrom` checks. Take a look at `finishWriteChunks()` and `shutdown()`.
--
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] szetszwo commented on pull request #2463: HDDS-5483. Validate block file length during put block
Posted by GitBox <gi...@apache.org>.
szetszwo commented on pull request #2463:
URL: https://github.com/apache/ozone/pull/2463#issuecomment-899998832
@sadanand48 , @mukul1987 if there is no objection, I will merge this to the master branch. Thanks.
--
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] sadanand48 commented on a change in pull request #2463: HDDS-5483. Validate block file length during put block
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on a change in pull request #2463:
URL: https://github.com/apache/ozone/pull/2463#discussion_r680680029
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java
##########
@@ -427,6 +429,17 @@ ContainerCommandResponseProto handlePutBlock(
ContainerProtos.BlockData data = request.getPutBlock().getBlockData();
BlockData blockData = BlockData.getFromProtoBuf(data);
Preconditions.checkNotNull(blockData);
+ ChunkLayOutVersion layoutVersion =
+ ChunkLayOutVersion.getConfiguredVersion(conf);
+ if (layoutVersion == FILE_PER_BLOCK) {
+ File blockFile = layoutVersion
+ .getChunkFile(kvContainer.getContainerData(),
+ blockData.getBlockID(), null);
+ // ensure that the putBlock length <= blockFile length
+ Preconditions.checkArgument(blockFile.length() >= blockData.getSize(),
Review comment:
@smengcl This is just a safety check. The ozone streaming write path will not use the current ratisAsyncApi which ensures sequential transactions i.e putBlock would always occur after writeChunk. To be on a safer side, this check is added. We may remove this after the complete streaming implementation if this check is redundant
cc : @mukul1987
--
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] mukul1987 commented on pull request #2463: HDDS-5483. Validate block file length during put block
Posted by GitBox <gi...@apache.org>.
mukul1987 commented on pull request #2463:
URL: https://github.com/apache/ozone/pull/2463#issuecomment-900059651
@szetszwo, sure lets do that
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a change in pull request #2463: HDDS-5483.Validate block file length during put block
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2463:
URL: https://github.com/apache/ozone/pull/2463#discussion_r677896200
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java
##########
@@ -427,6 +429,17 @@ ContainerCommandResponseProto handlePutBlock(
ContainerProtos.BlockData data = request.getPutBlock().getBlockData();
BlockData blockData = BlockData.getFromProtoBuf(data);
Preconditions.checkNotNull(blockData);
+ ChunkLayOutVersion layoutVersion =
+ ChunkLayOutVersion.getConfiguredVersion(conf);
+ if (layoutVersion == FILE_PER_BLOCK) {
+ File blockFile = layoutVersion
+ .getChunkFile(kvContainer.getContainerData(),
+ blockData.getBlockID(), null);
+ // ensure that the putBlock length <= blockFile length
+ Preconditions.checkArgument(blockFile.length() >= blockData.getSize(),
Review comment:
Under what conditions could this be violated?
--
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] szetszwo commented on pull request #2463: HDDS-5483. Validate block file length during put block
Posted by GitBox <gi...@apache.org>.
szetszwo commented on pull request #2463:
URL: https://github.com/apache/ozone/pull/2463#issuecomment-901568571
@adoroszlai , it is a good idea to have a PR. I will revert the commit from the master.
--
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 change in pull request #2463: HDDS-5483. Validate block file length during put block
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #2463:
URL: https://github.com/apache/ozone/pull/2463#discussion_r690283532
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java
##########
@@ -460,6 +463,20 @@ ContainerCommandResponseProto handlePutBlock(
ContainerProtos.BlockData data = request.getPutBlock().getBlockData();
BlockData blockData = BlockData.getFromProtoBuf(data);
Preconditions.checkNotNull(blockData);
+ ChunkLayOutVersion layoutVersion =
+ ChunkLayOutVersion.getConfiguredVersion(conf);
+ if (layoutVersion == FILE_PER_BLOCK &&
+ // ChunkManagerDummyImpl doesn't persist to disk, don't check here
+ !chunkManager.getClass()
+ .isAssignableFrom(ChunkManagerDummyImpl.class)) {
Review comment:
I think layout-specific code would be better added to each `ChunkManager` implementation instead of such `if` and `isAssignableFrom`. Take a look at `finishWriteChunks()` and `shutdown()`.
--
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] szetszwo commented on pull request #2463: HDDS-5483. Validate block file length during put block
Posted by GitBox <gi...@apache.org>.
szetszwo commented on pull request #2463:
URL: https://github.com/apache/ozone/pull/2463#issuecomment-906888776
@sadanand48 , as suggested by @adoroszlai , please submit a PR for the master branch. Similar to other JIRAs, we will rebase this to the HDDS-4454 branch after this has been committed to the master branch. Thanks.
--
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] szetszwo commented on pull request #2463: HDDS-5483. Validate block file length during put block
Posted by GitBox <gi...@apache.org>.
szetszwo commented on pull request #2463:
URL: https://github.com/apache/ozone/pull/2463#issuecomment-901569039
```
commit a595d06450d6788237bd459a1ac4f36fe5582992 (HEAD -> master, origin/master, origin/HEAD)
Author: Tsz-Wo Nicholas Sze <sz...@apache.org>
Date: Thu Aug 19 10:53:26 2021 +0800
Revert "HDDS-5483. Validate block file length during put block (#2463)"
This reverts commit 375f3c4f02a46e12fcfc1acc127ecdc5badd359d.
```
--
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] sadanand48 closed pull request #2463: HDDS-5483. Validate block file length during put block
Posted by GitBox <gi...@apache.org>.
sadanand48 closed pull request #2463:
URL: https://github.com/apache/ozone/pull/2463
--
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 #2463: HDDS-5483. Validate block file length during put block
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #2463:
URL: https://github.com/apache/ozone/pull/2463#issuecomment-900221279
> Should this be merged to the master branch instead the HDDS-4454 branch?
Although I don't expect any problems for this specific change, I would prefer going via PR for `master` next time, to make sure all checks pass.
--
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] szetszwo edited a comment on pull request #2463: HDDS-5483. Validate block file length during put block
Posted by GitBox <gi...@apache.org>.
szetszwo edited a comment on pull request #2463:
URL: https://github.com/apache/ozone/pull/2463#issuecomment-897281571
@smengcl , @mukul1987 , Should this be merged to the master branch instead the HDDS-4454 branch? This dose not look like specific to HDDS-4454.
--
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] ferhui commented on pull request #2463: HDDS-5483. Validate block file length during put block
Posted by GitBox <gi...@apache.org>.
ferhui commented on pull request #2463:
URL: https://github.com/apache/ozone/pull/2463#issuecomment-890642172
Maybe this should rebase on HDDS-4454
--
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] szetszwo edited a comment on pull request #2463: HDDS-5483. Validate block file length during put block
Posted by GitBox <gi...@apache.org>.
szetszwo edited a comment on pull request #2463:
URL: https://github.com/apache/ozone/pull/2463#issuecomment-897281571
@sadanand48 , @mukul1987 , Should this be merged to the master branch instead the HDDS-4454 branch? This dose not look like specific to HDDS-4454.
--
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