You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ozone.apache.org by um...@apache.org on 2021/10/07 23:33:56 UTC

[ozone] branch HDDS-3816-ec updated: HDDS-5832. EC: ECKeyOutputStream persists blocks in random order (#2717)

This is an automated email from the ASF dual-hosted git repository.

umamahesh pushed a commit to branch HDDS-3816-ec
in repository https://gitbox.apache.org/repos/asf/ozone.git


The following commit(s) were added to refs/heads/HDDS-3816-ec by this push:
     new f478b96  HDDS-5832. EC: ECKeyOutputStream persists blocks in random order (#2717)
f478b96 is described below

commit f478b96360e6eb1daf2f55d6b6c06bd1338c66ac
Author: Stephen O'Donnell <st...@gmail.com>
AuthorDate: Fri Oct 8 00:33:45 2021 +0100

    HDDS-5832. EC: ECKeyOutputStream persists blocks in random order (#2717)
---
 .../client/io/ECBlockOutputStreamEntryPool.java    |  3 +-
 .../hadoop/ozone/client/io/ECKeyOutputStream.java  |  1 -
 .../hadoop/ozone/client/MockOmTransport.java       |  4 --
 .../client/MultiNodePipelineBlockAllocator.java    | 16 +++++++-
 .../ozone/client/SinglePipelineBlockAllocator.java | 15 ++++++--
 .../hadoop/ozone/client/TestOzoneClient.java       | 15 +++++---
 .../hadoop/ozone/client/TestOzoneECClient.java     | 44 ++++++++++++++++------
 7 files changed, 70 insertions(+), 28 deletions(-)

diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/ECBlockOutputStreamEntryPool.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/ECBlockOutputStreamEntryPool.java
index 2e59650..aa27a72 100644
--- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/ECBlockOutputStreamEntryPool.java
+++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/ECBlockOutputStreamEntryPool.java
@@ -33,6 +33,7 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.Iterator;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 
@@ -120,7 +121,7 @@ public class ECBlockOutputStreamEntryPool extends BlockOutputStreamEntryPool {
       List<BlockOutputStreamEntry> streams) {
     List<OmKeyLocationInfo> locationInfoList = new ArrayList<>();
     Map<BlockID, ArrayList<ECBlockOutputStreamEntry>> blkIdVsStream =
-        new HashMap<>();
+        new LinkedHashMap<>();
 
     for (BlockOutputStreamEntry streamEntry : streams) {
       BlockID blkID = streamEntry.getBlockID();
diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/ECKeyOutputStream.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/ECKeyOutputStream.java
index 02c3ec3..ec9402c 100644
--- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/ECKeyOutputStream.java
+++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/ECKeyOutputStream.java
@@ -195,7 +195,6 @@ public class ECKeyOutputStream extends KeyOutputStream {
         currentWriterChunkLenToWrite,
         currentChunkBufferLen + currentWriterChunkLenToWrite == ecChunkSize);
     checkAndWriteParityCells(pos);
-
     int remLen = len - currentWriterChunkLenToWrite;
     int iters = remLen / ecChunkSize;
     int lastCellSize = remLen % ecChunkSize;
diff --git a/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/MockOmTransport.java b/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/MockOmTransport.java
index 5904d14..0a2fc41 100644
--- a/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/MockOmTransport.java
+++ b/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/MockOmTransport.java
@@ -83,10 +83,6 @@ public class MockOmTransport implements OmTransport {
     this.blockAllocator = allocator;
   }
 
-  public MockOmTransport() {
-    this.blockAllocator = new SinglePipelineBlockAllocator();
-  }
-
   @Override
   public OMResponse submitRequest(OMRequest payload) throws IOException {
     switch (payload.getCmdType()) {
diff --git a/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/MultiNodePipelineBlockAllocator.java b/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/MultiNodePipelineBlockAllocator.java
index e3f1d78..f5d2de3 100644
--- a/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/MultiNodePipelineBlockAllocator.java
+++ b/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/MultiNodePipelineBlockAllocator.java
@@ -18,7 +18,11 @@
 
 package org.apache.hadoop.ozone.client;
 
+import org.apache.hadoop.hdds.conf.ConfigurationSource;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.conf.StorageUnit;
 import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.ozone.OzoneConfigKeys;
 import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
 
 import java.util.ArrayList;
@@ -31,9 +35,12 @@ public class MultiNodePipelineBlockAllocator implements MockBlockAllocator {
   private long blockId;
   private HddsProtos.Pipeline pipeline;
   private int requiredNodes;
+  private final ConfigurationSource conf;
 
-  public MultiNodePipelineBlockAllocator(int requiredNodes) {
+  public MultiNodePipelineBlockAllocator(OzoneConfiguration conf,
+      int requiredNodes) {
     this.requiredNodes = requiredNodes;
+    this.conf = conf;
   }
 
   @Override
@@ -61,6 +68,11 @@ public class MultiNodePipelineBlockAllocator implements MockBlockAllocator {
       pipeline = builder.build();
     }
 
+    long blockSize = (long)conf.getStorageSize(
+        OzoneConfigKeys.OZONE_SCM_BLOCK_SIZE,
+        OzoneConfigKeys.OZONE_SCM_BLOCK_SIZE_DEFAULT,
+        StorageUnit.BYTES);
+
     List<OzoneManagerProtocolProtos.KeyLocation> results = new ArrayList<>();
     results.add(OzoneManagerProtocolProtos.KeyLocation.newBuilder()
         .setPipeline(pipeline).setBlockID(
@@ -68,7 +80,7 @@ public class MultiNodePipelineBlockAllocator implements MockBlockAllocator {
                 .setContainerBlockID(
                     HddsProtos.ContainerBlockID.newBuilder().setContainerID(1L)
                         .setLocalID(blockId++).build()).build()).setOffset(0L)
-        .setLength(keyArgs.getDataSize()).build());
+        .setLength(blockSize).build());
     return results;
   }
 }
diff --git a/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/SinglePipelineBlockAllocator.java b/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/SinglePipelineBlockAllocator.java
index 46ac98b..9d88e8c 100644
--- a/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/SinglePipelineBlockAllocator.java
+++ b/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/SinglePipelineBlockAllocator.java
@@ -17,6 +17,8 @@
  */
 package org.apache.hadoop.ozone.client;
 
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.conf.StorageUnit;
 import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
 import org.apache.hadoop.hdds.protocol.proto.HddsProtos.BlockID;
 import org.apache.hadoop.hdds.protocol.proto.HddsProtos.ContainerBlockID;
@@ -25,6 +27,7 @@ import org.apache.hadoop.hdds.protocol.proto.HddsProtos.Pipeline;
 import org.apache.hadoop.hdds.protocol.proto.HddsProtos.PipelineID;
 import org.apache.hadoop.hdds.protocol.proto.HddsProtos.Port;
 import org.apache.hadoop.hdds.protocol.proto.HddsProtos.UUID;
+import org.apache.hadoop.ozone.OzoneConfigKeys;
 import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyArgs;
 import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyLocation;
 
@@ -39,9 +42,10 @@ public class SinglePipelineBlockAllocator
 
   private long blockId;
   private Pipeline pipeline;
+  private OzoneConfiguration conf;
 
-  public SinglePipelineBlockAllocator() {
-
+  public SinglePipelineBlockAllocator(OzoneConfiguration conf) {
+    this.conf = conf;
   }
 
   @Override
@@ -76,6 +80,11 @@ public class SinglePipelineBlockAllocator
       pipeline = bldr.build();
     }
 
+    long blockSize =  (long)conf.getStorageSize(
+        OzoneConfigKeys.OZONE_SCM_BLOCK_SIZE,
+        OzoneConfigKeys.OZONE_SCM_BLOCK_SIZE_DEFAULT,
+        StorageUnit.BYTES);
+
     List<KeyLocation> results = new ArrayList<>();
     results.add(KeyLocation.newBuilder()
         .setPipeline(pipeline)
@@ -87,7 +96,7 @@ public class SinglePipelineBlockAllocator
                 .build())
             .build())
         .setOffset(0L)
-        .setLength(keyArgs.getDataSize())
+        .setLength(blockSize)
         .build());
     return results;
   }
diff --git a/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/TestOzoneClient.java b/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/TestOzoneClient.java
index 4dc47c3..e7bc9d7 100644
--- a/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/TestOzoneClient.java
+++ b/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/TestOzoneClient.java
@@ -18,11 +18,13 @@
 
 package org.apache.hadoop.ozone.client;
 
+import org.apache.hadoop.conf.StorageUnit;
 import org.apache.hadoop.hdds.client.ECReplicationConfig;
 import org.apache.hadoop.hdds.client.ReplicationType;
 import org.apache.hadoop.hdds.conf.ConfigurationSource;
-import org.apache.hadoop.hdds.conf.InMemoryConfiguration;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
 import org.apache.hadoop.hdds.scm.XceiverClientFactory;
+import org.apache.hadoop.ozone.OzoneConfigKeys;
 import org.apache.hadoop.ozone.OzoneConsts;
 import org.apache.hadoop.ozone.client.io.OzoneInputStream;
 import org.apache.hadoop.ozone.client.io.OzoneOutputStream;
@@ -71,8 +73,8 @@ public class TestOzoneClient {
 
   @Before
   public void init() throws IOException {
-    ConfigurationSource config = new InMemoryConfiguration();
-    createNewClient(config, new SinglePipelineBlockAllocator());
+    OzoneConfiguration config = new OzoneConfiguration();
+    createNewClient(config, new SinglePipelineBlockAllocator(config));
   }
 
   private void createNewClient(ConfigurationSource config,
@@ -190,11 +192,14 @@ public class TestOzoneClient {
   @Test
   public void testPutKeyWithECReplicationConfig() throws IOException {
     close();
-    ConfigurationSource config = new InMemoryConfiguration();
+    OzoneConfiguration config = new OzoneConfiguration();
+    config.setStorageSize(OzoneConfigKeys.OZONE_SCM_BLOCK_SIZE, 2,
+        StorageUnit.KB);
     int data = 3;
     int parity = 2;
     int chunkSize = 1024;
-    createNewClient(config, new MultiNodePipelineBlockAllocator(data + parity));
+    createNewClient(config, new MultiNodePipelineBlockAllocator(
+        config, data + parity));
     String value = new String(new byte[chunkSize], UTF_8);
     OzoneBucket bucket = getOzoneBucket();
 
diff --git a/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/TestOzoneECClient.java b/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/TestOzoneECClient.java
index 6eee2ab..1ba25f3 100644
--- a/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/TestOzoneECClient.java
+++ b/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/TestOzoneECClient.java
@@ -18,11 +18,10 @@
 
 package org.apache.hadoop.ozone.client;
 
+import org.apache.hadoop.conf.StorageUnit;
 import org.apache.hadoop.hdds.client.DefaultReplicationConfig;
 import org.apache.hadoop.hdds.client.ECReplicationConfig;
 import org.apache.hadoop.hdds.client.ReplicationType;
-import org.apache.hadoop.hdds.conf.ConfigurationSource;
-import org.apache.hadoop.hdds.conf.InMemoryConfiguration;
 import org.apache.hadoop.hdds.conf.OzoneConfiguration;
 import org.apache.hadoop.hdds.protocol.DatanodeDetails;
 import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
@@ -33,6 +32,7 @@ import org.apache.hadoop.io.erasurecode.ECSchema;
 import org.apache.hadoop.io.erasurecode.ErasureCodecOptions;
 import org.apache.hadoop.io.erasurecode.codec.RSErasureCodec;
 import org.apache.hadoop.io.erasurecode.rawcoder.RawErasureEncoder;
+import org.apache.hadoop.ozone.OzoneConfigKeys;
 import org.apache.hadoop.ozone.client.io.ECKeyOutputStream;
 import org.apache.hadoop.ozone.client.io.OzoneInputStream;
 import org.apache.hadoop.ozone.client.io.OzoneOutputStream;
@@ -74,8 +74,7 @@ public class TestOzoneECClient {
   private byte[][] inputChunks = new byte[dataBlocks][chunkSize];
   private final XceiverClientFactory factoryStub =
       new MockXceiverClientFactory();
-  private final MockOmTransport transportStub = new MockOmTransport(
-      new MultiNodePipelineBlockAllocator(dataBlocks + parityBlocks));
+  private MockOmTransport transportStub = null;
   private ECSchema schema = new ECSchema("rs", dataBlocks, parityBlocks);
   private ErasureCodecOptions options = new ErasureCodecOptions(schema);
   private OzoneConfiguration conf = new OzoneConfiguration();
@@ -86,8 +85,12 @@ public class TestOzoneECClient {
 
   @Before
   public void init() throws IOException {
-    ConfigurationSource config = new InMemoryConfiguration();
-    client = new OzoneClient(config, new RpcClient(config, null) {
+    conf.setStorageSize(OzoneConfigKeys.OZONE_SCM_BLOCK_SIZE, 2,
+        StorageUnit.KB);
+    transportStub = new MockOmTransport(
+        new MultiNodePipelineBlockAllocator(conf, dataBlocks + parityBlocks));
+
+    client = new OzoneClient(conf, new RpcClient(conf, null) {
 
       @Override
       protected OmTransport createOmTransport(String omServiceId)
@@ -307,17 +310,34 @@ public class TestOzoneECClient {
 
     // create key without mentioning replication config. Since we set EC
     // replication in bucket, key should be EC key.
-    try (OzoneOutputStream out = bucket.createKey("mykey", 2000)) {
+    try (OzoneOutputStream out = bucket.createKey("mykey", 1024)) {
       Assert.assertTrue(out.getOutputStream() instanceof ECKeyOutputStream);
-      for (int i = 0; i < inputChunks.length; i++) {
-        out.write(inputChunks[i]);
+      // Block Size is 2kb, so to create 3 blocks we need 6 iterations here
+      for (int j = 0; j < 6; j++) {
+        for (int i = 0; i < inputChunks.length; i++) {
+          out.write(inputChunks[i]);
+        }
       }
     }
+    OzoneManagerProtocolProtos.KeyLocationList blockList =
+        transportStub.getKeys().get(volumeName).get(bucketName).get("mykey")
+            .getKeyLocationListList().get(0);
+
+    Assert.assertEquals(3, blockList.getKeyLocationsCount());
+    // As the mock allocator allocates block with id's increasing sequentially
+    // from 1. Therefore the block should be in the order with id starting 1, 2
+    // and then 3.
+    for (int i = 0; i < 3; i++) {
+      long localId = blockList.getKeyLocationsList().get(i).getBlockID()
+          .getContainerBlockID().getLocalID();
+      Assert.assertEquals(i + 1, localId);
+    }
+
     Assert.assertEquals(1,
-        transportStub.getKeys().get(volumeName).get(bucketName).get(keyName)
+        transportStub.getKeys().get(volumeName).get(bucketName).get("mykey")
             .getKeyLocationListCount());
-    Assert.assertEquals(inputChunks[0].length * 3,
-        transportStub.getKeys().get(volumeName).get(bucketName).get(keyName)
+    Assert.assertEquals(inputChunks[0].length * 3 * 6,
+        transportStub.getKeys().get(volumeName).get(bucketName).get("mykey")
             .getDataSize());
   }
 

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@ozone.apache.org
For additional commands, e-mail: commits-help@ozone.apache.org