[jira] [Comment Edited] (HDDS-265) Move numPendingDeletionBlocks and deleteTransactionId from ContainerData to KeyValueContainerData

[LiXin Ge (JIRA)]

    [ 
https://issues.apache.org/jira/browse/HDDS-265?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16574746#comment-16574746
 ] 

LiXin Ge edited comment on HDDS-265 at 8/11/18 8:38 AM:
--------------------------------------------------------

[~ljain] Thanks for reviewing this! patch 002 addressed all your comments 
except the one :
{quote}3.We can add a default implementation for isValidContainerType in 
ContainerDeletionChoosingPolicy.
{quote}
That's because _ContainerDeletionChoosingPolicy_ is interface that not allowed 
to have body.
  
 BTW, I have tried to add a test like below, but find that the test will fail 
even in the *trunk branch*. I'm not sure wether this is a bug, if it is, maybe 
we should have another jira to trace it.
 ----2018/08/11: OK, this is not a bug, maybe something by design. 
ContainerSet#getContainerReport didn't send {{DeleteTransactionId}} and 
ContainerMapping#reconcileState didn't update this field based on 
ContainerReport, too. Actually, SCM updates the {{DeleteTransactionId}} based 
on the DeletedBlockLogImpl now.
{code:java|title=TestContainerMapping.java|borderStyle=solid}
@@ -243,7 +243,7 @@ public void testContainerCloseWithContainerReport() throws 
IOException {
         .setReadBytes(5368705120L)
         .setWriteBytes(5368705120L)
         .setContainerID(info.getContainerID())
-        .setDeleteTransactionId(0);
+        .setDeleteTransactionId(3592468L);

     reports.add(ciBuilder.build());

@@ -258,6 +258,7 @@ public void testContainerCloseWithContainerReport() throws 
IOException {
     Assert.assertEquals(500000000L,
         updatedContainer.getNumberOfKeys());
     Assert.assertEquals(5368705120L, updatedContainer.getUsedBytes());
+    Assert.assertEquals(3592468L, updatedContainer.getDeleteTransactionId());
     NavigableSet<ContainerID> pendingCloseContainers = 
mapping.getStateManager()
         .getMatchingContainerIDs(
             containerOwner,
{code}


was (Author: gelixin):
[~ljain] Thanks for reviewing this! patch 002 addressed all your comments 
except the one :
bq. 3.We can add a default implementation for isValidContainerType in 
ContainerDeletionChoosingPolicy.
That's because _ContainerDeletionChoosingPolicy_ is interface that not allowed 
to have body.
 
BTW, I have tried to add a test like below, but find that the test will fail 
even in the *trunk branch*. I'm not sure wether this is a bug, if it is, maybe 
we should have another jira to trace it.

{code:java|title=TestContainerMapping.java|borderStyle=solid}
@@ -243,7 +243,7 @@ public void testContainerCloseWithContainerReport() throws 
IOException {
         .setReadBytes(5368705120L)
         .setWriteBytes(5368705120L)
         .setContainerID(info.getContainerID())
-        .setDeleteTransactionId(0);
+        .setDeleteTransactionId(3592468L);

     reports.add(ciBuilder.build());

@@ -258,6 +258,7 @@ public void testContainerCloseWithContainerReport() throws 
IOException {
     Assert.assertEquals(500000000L,
         updatedContainer.getNumberOfKeys());
     Assert.assertEquals(5368705120L, updatedContainer.getUsedBytes());
+    Assert.assertEquals(3592468L, updatedContainer.getDeleteTransactionId());
     NavigableSet<ContainerID> pendingCloseContainers = 
mapping.getStateManager()
         .getMatchingContainerIDs(
             containerOwner,
{code}

> Move numPendingDeletionBlocks and deleteTransactionId from ContainerData to 
> KeyValueContainerData
> -------------------------------------------------------------------------------------------------
>
>                 Key: HDDS-265
>                 URL: https://issues.apache.org/jira/browse/HDDS-265
>             Project: Hadoop Distributed Data Store
>          Issue Type: Bug
>    Affects Versions: 0.2.1
>            Reporter: Hanisha Koneru
>            Assignee: LiXin Ge
>            Priority: Major
>             Fix For: 0.2.1
>
>         Attachments: HDDS-265.000.patch, HDDS-265.001.patch, 
> HDDS-265.002.patch
>
>
> "numPendingDeletionBlocks" and "deleteTransactionId" fields are specific to 
> KeyValueContainers. As such they should be moved to KeyValueContainerData 
> from ContainerData.
> ContainerReport should also be refactored to take in this change. 
> Please refer to [~ljain]'s comment in HDDS-250.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org