You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by GitBox <gi...@apache.org> on 2022/05/02 19:02:59 UTC
[GitHub] [hadoop] jojochuang commented on a diff in pull request #4252: HDFS-16566 Erasure Coding: Recovery may causes excess replicas when busy DN exsits
jojochuang commented on code in PR #4252:
URL: https://github.com/apache/hadoop/pull/4252#discussion_r863097191
##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestReconstructStripedBlocks.java:
##########
@@ -430,4 +433,86 @@ public void testReconstructionWork() throws Exception {
dfsCluster.shutdown();
}
}
+ private byte[] writeStripedFile(DistributedFileSystem fs, Path ecFile,
+ int writeBytes) throws Exception {
+ byte[] bytes = StripedFileTestUtil.generateBytes(writeBytes);
+ DFSTestUtil.writeFile(fs, ecFile, new String(bytes));
+ StripedFileTestUtil.waitBlockGroupsReported(fs, ecFile.toString());
+
+ return bytes;
+ }
+ @Test
+ public void testReconstrutionWithBusyBlock1() throws Exception {
+ //When the index of busy block is smaller than the missing block
+ //[0(busy),1(busy),3,4,5,6,7,8]
+ int busyNodeIndex1=0;
Review Comment:
```suggestion
int busyNodeIndex1 = 0;
```
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/BlockECReconstructionCommand.java:
##########
@@ -78,28 +78,31 @@ public static class BlockECReconstructionInfo {
private String[] targetStorageIDs;
private StorageType[] targetStorageTypes;
private final byte[] liveBlockIndices;
+ private final byte[] excludeReconstructedIndices;
private final ErasureCodingPolicy ecPolicy;
public BlockECReconstructionInfo(ExtendedBlock block,
DatanodeInfo[] sources, DatanodeStorageInfo[] targetDnStorageInfo,
- byte[] liveBlockIndices, ErasureCodingPolicy ecPolicy) {
+ byte[] liveBlockIndices, byte[] excludeReconstructedIndices, ErasureCodingPolicy ecPolicy) {
this(block, sources, DatanodeStorageInfo
.toDatanodeInfos(targetDnStorageInfo), DatanodeStorageInfo
.toStorageIDs(targetDnStorageInfo), DatanodeStorageInfo
- .toStorageTypes(targetDnStorageInfo), liveBlockIndices, ecPolicy);
+ .toStorageTypes(targetDnStorageInfo), liveBlockIndices,
+ excludeReconstructedIndices, ecPolicy);
}
public BlockECReconstructionInfo(ExtendedBlock block,
DatanodeInfo[] sources, DatanodeInfo[] targets,
String[] targetStorageIDs, StorageType[] targetStorageTypes,
- byte[] liveBlockIndices, ErasureCodingPolicy ecPolicy) {
+ byte[] liveBlockIndices, byte[] excludeReconstructedIndices, ErasureCodingPolicy ecPolicy) {
this.block = block;
this.sources = sources;
this.targets = targets;
this.targetStorageIDs = targetStorageIDs;
this.targetStorageTypes = targetStorageTypes;
this.liveBlockIndices = liveBlockIndices == null ?
new byte[]{} : liveBlockIndices;
+ this.excludeReconstructedIndices=excludeReconstructedIndices;
Review Comment:
```suggestion
this.excludeReconstructedIndices = excludeReconstructedIndices;
```
##########
hadoop-hdfs-project/hadoop-hdfs-client/src/main/proto/erasurecoding.proto:
##########
@@ -107,7 +107,8 @@ message BlockECReconstructionInfoProto {
required StorageUuidsProto targetStorageUuids = 4;
required StorageTypesProto targetStorageTypes = 5;
required bytes liveBlockIndices = 6;
- required ErasureCodingPolicyProto ecPolicy = 7;
+ required bytes excludeReconstructedIndices = 7;
Review Comment:
This is going to be incompatible with older versions. Please restore the order.
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/erasurecode/StripedWriter.java:
##########
@@ -123,13 +123,14 @@ void init() throws IOException {
private void initTargetIndices() {
BitSet bitset = reconstructor.getLiveBitSet();
+ BitSet excludebitset=reconstructor.getExcludeBitSet();
int m = 0;
hasValidTargets = false;
for (int i = 0; i < dataBlkNum + parityBlkNum; i++) {
if (!bitset.get(i)) {
if (reconstructor.getBlockLen(i) > 0) {
- if (m < targets.length) {
+ if (m < targets.length&& !excludebitset.get(i)) {
Review Comment:
check style: please add space between targets.length and &&
```suggestion
if (m < targets.length && !excludebitset.get(i)) {
```
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/erasurecode/StripedReconstructionInfo.java:
##########
@@ -41,26 +41,28 @@ public class StripedReconstructionInfo {
private final DatanodeInfo[] targets;
private final StorageType[] targetStorageTypes;
private final String[] targetStorageIds;
+ private final byte[] excludeReconstructedIndices;
public StripedReconstructionInfo(ExtendedBlock blockGroup,
ErasureCodingPolicy ecPolicy, byte[] liveIndices, DatanodeInfo[] sources,
byte[] targetIndices) {
this(blockGroup, ecPolicy, liveIndices, sources, targetIndices, null,
- null, null);
+ null, null, new byte[0]);
}
StripedReconstructionInfo(ExtendedBlock blockGroup,
ErasureCodingPolicy ecPolicy, byte[] liveIndices, DatanodeInfo[] sources,
DatanodeInfo[] targets, StorageType[] targetStorageTypes,
- String[] targetStorageIds) {
+ String[] targetStorageIds, byte[] excludeReconstructedIndices) {
Review Comment:
I don't understand this.
No new code exercises this code path. Why make this change?
##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestReconstructStripedBlocks.java:
##########
@@ -430,4 +433,86 @@ public void testReconstructionWork() throws Exception {
dfsCluster.shutdown();
}
}
+ private byte[] writeStripedFile(DistributedFileSystem fs, Path ecFile,
+ int writeBytes) throws Exception {
+ byte[] bytes = StripedFileTestUtil.generateBytes(writeBytes);
+ DFSTestUtil.writeFile(fs, ecFile, new String(bytes));
+ StripedFileTestUtil.waitBlockGroupsReported(fs, ecFile.toString());
+
+ return bytes;
+ }
+ @Test
+ public void testReconstrutionWithBusyBlock1() throws Exception {
+ //When the index of busy block is smaller than the missing block
+ //[0(busy),1(busy),3,4,5,6,7,8]
+ int busyNodeIndex1=0;
+ int busyNodeIndex2=1;
+ int deadNodeIndex=2;
+ final Path ecDir = new Path("/" + this.getClass().getSimpleName());
Review Comment:
Use GenericTestUtils.getRandomizedTestDir() instead
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org