Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,6 @@ public enum ClientVersion implements ComponentVersion {
"This client version has support for Object Store and File " +
"System Optimized Bucket Layouts."),

EC_REPLICA_INDEX_REQUIRED_IN_BLOCK_REQUEST(4,
"This client version enforces replica index is set for fixing read corruption that could occur when " +
"replicaIndex parameter is not validated before EC block reads."),

FUTURE_VERSION(-1, "Used internally when the server side is older and an"
+ " unknown client version has arrived from the client.");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,6 @@
import static org.apache.hadoop.hdds.scm.utils.ClientCommandsUtils.getReadChunkVersion;
import static org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos
.ContainerDataProto.State.RECOVERING;
import static org.apache.hadoop.ozone.ClientVersion.EC_REPLICA_INDEX_REQUIRED_IN_BLOCK_REQUEST;
import static org.apache.hadoop.ozone.OzoneConsts.INCREMENTAL_CHUNK_LIST;
import static org.apache.hadoop.ozone.container.common.interfaces.Container.ScanResult;

Expand Down Expand Up @@ -634,15 +633,6 @@ ContainerCommandResponseProto handleEcho(
return getEchoResponse(request);
}

/**
* Checks if a replicaIndex needs to be checked based on the client version for a request.
* @param request ContainerCommandRequest object.
* @return true if the validation is required for the client version else false.
*/
private boolean replicaIndexCheckRequired(ContainerCommandRequestProto request) {
return request.hasVersion() && request.getVersion() >= EC_REPLICA_INDEX_REQUIRED_IN_BLOCK_REQUEST.toProtoValue();
}

/**
* Handle Get Block operation. Calls BlockManager to process the request.
*/
Expand All @@ -661,9 +651,7 @@ ContainerCommandResponseProto handleGetBlock(
try {
BlockID blockID = BlockID.getFromProtobuf(
request.getGetBlock().getBlockID());
if (replicaIndexCheckRequired(request)) {
BlockUtils.verifyReplicaIdx(kvContainer, blockID);
}
BlockUtils.verifyReplicaIdx(kvContainer, blockID);
responseData = blockManager.getBlock(kvContainer, blockID).getProtoBufMessage();
final long numBytes = responseData.getSerializedSize();
metrics.incContainerBytesStats(Type.GetBlock, numBytes);
Expand Down Expand Up @@ -786,9 +774,7 @@ ContainerCommandResponseProto handleReadChunk(
ChunkInfo chunkInfo = ChunkInfo.getFromProtoBuf(request.getReadChunk()
.getChunkData());
Preconditions.checkNotNull(chunkInfo);
if (replicaIndexCheckRequired(request)) {
BlockUtils.verifyReplicaIdx(kvContainer, blockID);
}
BlockUtils.verifyReplicaIdx(kvContainer, blockID);
BlockUtils.verifyBCSId(kvContainer, blockID);

if (dispatcherContext == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,9 @@ public static void verifyBCSId(Container container, BlockID blockID)
public static void verifyReplicaIdx(Container container, BlockID blockID)
throws IOException {
Integer containerReplicaIndex = container.getContainerData().getReplicaIndex();
if (containerReplicaIndex > 0 && !containerReplicaIndex.equals(blockID.getReplicaIndex())) {
Integer blockReplicaIndex = blockID.getReplicaIndex();
if (containerReplicaIndex > 0 && blockReplicaIndex != null && blockReplicaIndex != 0 &&
!containerReplicaIndex.equals(blockReplicaIndex)) {
throw new StorageContainerException(
"Unable to find the Container with replicaIdx " + blockID.getReplicaIndex() + ". Container "
+ container.getContainerData().getContainerID() + " replicaIdx is "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,7 @@ public void testGetBlockWithReplicaIndexMismatch(ClientVersion clientVersion, in
handler.handleGetBlock(
getDummyCommandRequestProto(clientVersion, ContainerProtos.Type.GetBlock, rid),
container);
assertEquals((replicaIndex > 0 && rid != replicaIndex && clientVersion.toProtoValue() >=
ClientVersion.EC_REPLICA_INDEX_REQUIRED_IN_BLOCK_REQUEST.toProtoValue()) ?
assertEquals((replicaIndex > 0 && rid != 0 && rid != replicaIndex) ?
ContainerProtos.Result.CONTAINER_NOT_FOUND : UNKNOWN_BCSID,
response.getResult());
}
Expand Down Expand Up @@ -167,8 +166,7 @@ public void testReadChunkWithReplicaIndexMismatch(ClientVersion clientVersion, i
ContainerProtos.ContainerCommandResponseProto response =
handler.handleReadChunk(getDummyCommandRequestProto(clientVersion, ContainerProtos.Type.ReadChunk, rid),
container, null);
assertEquals((replicaIndex > 0 && rid != replicaIndex &&
clientVersion.toProtoValue() >= ClientVersion.EC_REPLICA_INDEX_REQUIRED_IN_BLOCK_REQUEST.toProtoValue()) ?
assertEquals((replicaIndex > 0 && rid != 0 && rid != replicaIndex) ?
ContainerProtos.Result.CONTAINER_NOT_FOUND : UNKNOWN_BCSID,
response.getResult());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,7 @@ public void testCreateRecoveryContainer() throws Exception {
int replicaIndex = 4;
XceiverClientSpi dnClient = xceiverClientManager.acquireClient(
createSingleNodePipeline(newPipeline, newPipeline.getNodes().get(0),
replicaIndex));
2));
try {
// To create the actual situation, container would have been in closed
// state at SCM.
Expand Down