Skip to content

Commit

Permalink
HDDS-11132. Revert client version bump done as part of HDDS-10983 (ap…
Browse files Browse the repository at this point in the history
  • Loading branch information
swamirishi authored Oct 23, 2024
1 parent ea5cbff commit e7bf154
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 26 deletions.
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

0 comments on commit e7bf154

Please sign in to comment.