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/17 06:03:52 UTC
[GitHub] [ozone] bshashikant opened a new pull request #2542: HDDS-5623. Add data validation checks on datanode.
bshashikant opened a new pull request #2542:
URL: https://github.com/apache/ozone/pull/2542
## What changes were proposed in this pull request?
https://issues.apache.org/jira/browse/HDDS-5619 , details a data corruption issue on the datanode side. The patch is here adds the necessary data validation checks on the server side so as to catch such data corruptions on datanode side as early in the cycle as possible.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-5623
## How was this patch tested?
Existing 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] avijayanhwx merged pull request #2542: HDDS-5623. Add data validation checks on datanode.
Posted by GitBox <gi...@apache.org>.
avijayanhwx merged pull request #2542:
URL: https://github.com/apache/ozone/pull/2542
--
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] kerneltime commented on a change in pull request #2542: HDDS-5623. Add data validation checks on datanode.
Posted by GitBox <gi...@apache.org>.
kerneltime commented on a change in pull request #2542:
URL: https://github.com/apache/ozone/pull/2542#discussion_r690800501
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java
##########
@@ -595,6 +602,12 @@ ContainerCommandResponseProto handleReadChunk(
data = chunkManager.readChunk(kvContainer, blockID, chunkInfo,
dispatcherContext);
+ // Validate data only if the read chunk issued as a result of
+ // ratis call internally. For client reads, validation happens
+ // on the client
Review comment:
```suggestion
// Validate data only if the read chunk is issued by Ratis for its internal logic.
// For client reads, the client is expected to validate.
```
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java
##########
@@ -664,6 +677,18 @@ ContainerCommandResponseProto handleDeleteChunk(
return getSuccessResponse(request);
}
+ private void validateChunkData(ChunkBuffer data, ChunkInfo info)
Review comment:
There can be many kinds of validations. This makes the code more readable in terms of what will be done.
```suggestion
private void validateChunkChecksumData(ChunkBuffer data, ChunkInfo info)
```
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeConfiguration.java
##########
@@ -214,6 +216,16 @@ public void setBlockDeletionLimit(int limit) {
)
private long diskCheckTimeout = DISK_CHECK_TIMEOUT_DEFAULT;
+ @Config(key = "chunk.data.validation.check",
+ defaultValue = "false",
+ type = ConfigType.BOOLEAN,
+ tags = { DATANODE },
+ description = "Enable safety checks to ensure chunk offset equal "
+ + "to block file length"
Review comment:
```suggestion
description = "Enable safety checks such as checksum validation for Ratis calls."
```
--
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] kerneltime commented on a change in pull request #2542: HDDS-5623. Add data validation checks on datanode.
Posted by GitBox <gi...@apache.org>.
kerneltime commented on a change in pull request #2542:
URL: https://github.com/apache/ozone/pull/2542#discussion_r690801802
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeConfiguration.java
##########
@@ -214,6 +216,16 @@ public void setBlockDeletionLimit(int limit) {
)
private long diskCheckTimeout = DISK_CHECK_TIMEOUT_DEFAULT;
+ @Config(key = "chunk.data.validation.check",
+ defaultValue = "false",
+ type = ConfigType.BOOLEAN,
+ tags = { DATANODE },
+ description = "Enable safety checks to ensure chunk offset equal "
+ + "to block file length"
Review comment:
```suggestion
description = "Enable safety checks such as checksum validation for Ratis calls."
```
--
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] kerneltime commented on a change in pull request #2542: HDDS-5623. Add data validation checks on datanode.
Posted by GitBox <gi...@apache.org>.
kerneltime commented on a change in pull request #2542:
URL: https://github.com/apache/ozone/pull/2542#discussion_r690800501
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java
##########
@@ -595,6 +602,12 @@ ContainerCommandResponseProto handleReadChunk(
data = chunkManager.readChunk(kvContainer, blockID, chunkInfo,
dispatcherContext);
+ // Validate data only if the read chunk issued as a result of
+ // ratis call internally. For client reads, validation happens
+ // on the client
Review comment:
```suggestion
// Validate data only if the read chunk is issued by Ratis for its internal logic.
// For client reads, the client is expected to validate.
```
--
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] kerneltime commented on a change in pull request #2542: HDDS-5623. Add data validation checks on datanode.
Posted by GitBox <gi...@apache.org>.
kerneltime commented on a change in pull request #2542:
URL: https://github.com/apache/ozone/pull/2542#discussion_r690801296
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java
##########
@@ -664,6 +677,18 @@ ContainerCommandResponseProto handleDeleteChunk(
return getSuccessResponse(request);
}
+ private void validateChunkData(ChunkBuffer data, ChunkInfo info)
Review comment:
There can be many kinds of validations. This makes the code more readable in terms of what will be done.
```suggestion
private void validateChunkChecksumData(ChunkBuffer data, ChunkInfo info)
```
--
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] avijayanhwx commented on pull request #2542: HDDS-5623. Add data validation checks on datanode.
Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on pull request #2542:
URL: https://github.com/apache/ozone/pull/2542#issuecomment-906631842
Thanks for the contribution @bshashikant, and the reviews @mukul1987 , @szetszwo , @swagle, @kerneltime & @JacksonYao287!
--
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 #2542: HDDS-5623. Add data validation checks on datanode.
Posted by GitBox <gi...@apache.org>.
mukul1987 commented on pull request #2542:
URL: https://github.com/apache/ozone/pull/2542#issuecomment-904689742
@bshashikant can we fix the errors in the CI here ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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