[GitHub] [ozone] fapifta commented on a change in pull request #2702: HDDS-5755 EC: Refactor ECBlockOutputStreamEntry to accommodate all block group related ECBlockOuputStreams.

[GitBox <gi...@apache.org>]

fapifta commented on a change in pull request #2702:
URL: https://github.com/apache/ozone/pull/2702#discussion_r722005530



##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestECKeyOutputStream.java
##########
@@ -199,64 +191,6 @@ public void testCreateRatisKeyAndWithECBucketDefaults() throws Exception {
     }
   }
 
-

Review comment:
       This test is not feasible in the new code structure.
   This test reaches out to ECBlockOutputStreams that were handling a single BlockOutputStreams earlier, and based on their Pipeline information it acquires a client from their XCeiverClientFactory, then checks if the acquired clients are distinct clients and none of them is equal to each other.
   In the new code sturcture, the ECBlockOutputStreamEntry knows about the full EC Pipeline, and manages the data and parity streams together. Individual pipelines and streams are hidden outside.
   
   The two new tests in the new ECBlockOutputStreamEntryTest class is entitled to test for the same, those tests are there to ensure that the Pipelines that are created by the ECBlockOutputStreamEntry#createSingleECBlockPipeline method are distinct in case the DataNodeDetails defines the same host but different port for EC replication, and I also added a test that ensures that if the port is also the same, then we reuse the client (the second test).
   
   I believe this was important for tests where we need to ensure the tests work in a MiniCluster as well, where the DataNodeDetails differ in just the DataNode port, while in the Ratis replication case it was not really important as we do not differentiate blocks there while in EC we need to know the order of data blocks and parity, and we also need to communicate with all nodes and we can not share a client between them properly.




-- 
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