[jira] [Comment Edited] (HDDS-228) Add the ReplicaMaps to ContainerStateManager
[
https://issues.apache.org/jira/browse/HDDS-228?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16539160#comment-16539160
]
Ajay Kumar edited comment on HDDS-228 at 7/10/18 9:59 PM:
--
[~nandakumar131] thanks for review. Addressed most of you suggestions, except
one.
{quote}ContainerStateManager#getContainerReplicas javadoc is incorrect.
ContainerStateMap#getContainerReplicas javadoc is incorrect
ContainerStateMap#removeContainerReplica:
For the third case, the call should throw Exception.{quote}
Done
{quote}ContainerStateMap#addContainerReplica: If the container is not already
present, this method adds the container to the contReplicaMap and updates the
set with the given datanode. It would be easy to detect inconsistency if we
throw Exception in case if the container is not present in the contReplicaMap.
A container should be added to the contReplicaMap when addContainer is called.
This way we can maintain consistency between containerMap and
contReplicaMap.{quote}
IMO adding a new replica even if one doesn't exist do not violate any
condition. So we should add it silently. Also there is little value in adding
open containers to replicaMap as there replication will be handled by RATIS. So
only container report handler should add or remove entries from this instead of
adding it via addContainer. Let me know if you still think throwing exception
is right way there.
was (Author: ajayydv):
[~nandakumar131] thanks for review. Addressed most of you suggestions, except
one.
{quote}ContainerStateManager#getContainerReplicas javadoc is incorrect.
ContainerStateMap#getContainerReplicas javadoc is incorrect
ContainerStateMap#removeContainerReplica:
For the third case, the call should throw Exception.{quote}
Done
{quote}ContainerStateMap#addContainerReplica: If the container is not already
present, this method adds the container to the contReplicaMap and updates the
set with the given datanode. It would be easy to detect inconsistency if we
throw Exception in case if the container is not present in the contReplicaMap.
A container should be added to the contReplicaMap when addContainer is called.
This way we can maintain consistency between containerMap and
contReplicaMap.{quote}
IMO adding a new replica even if one doesn't exist do not violate any
condition. So we should add it silently. Also there is little value in adding
open containers to replicaMap as there replication will be handled by RATIS. So
only contrainer report handler should add or remove entries from this instead
of adding it via addContainer.
> Add the ReplicaMaps to ContainerStateManager
>
>
> Key: HDDS-228
> URL: https://issues.apache.org/jira/browse/HDDS-228
> Project: Hadoop Distributed Data Store
> Issue Type: Improvement
> Components: SCM
>Reporter: Anu Engineer
>Assignee: Ajay Kumar
>Priority: Major
> Fix For: 0.2.1
>
> Attachments: HDDS-228.00.patch, HDDS-228.01.patch, HDDS-228.02.patch,
> HDDS-228.03.patch, HDDS-228.04.patch
>
>
> We need to maintain a list of data nodes in the SCM that tells us where a
> container is located. This created from the container reports. The HDDS-175
> refactored the class to make this separation easy and this JIRA is a followup
> that keeps a hash table to track this information.
--
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
[jira] [Comment Edited] (HDDS-228) Add the ReplicaMaps to ContainerStateManager
[
https://issues.apache.org/jira/browse/HDDS-228?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16534283#comment-16534283
]
Ajay Kumar edited comment on HDDS-228 at 7/6/18 4:20 AM:
-
[~nandakumar131] thanks for review.
{quote}Instead of adding container entry in contReplicaMap inside
addContainerReplica call, can we do it in addContainer call. This way
containerMap and contReplicaMap will be consistent.
{quote}
addContainer is called from ContainerStateManager#allocateContainer as well.
This means Container is probably in open state and we don't want
replicationManager to handle it until it is closed. Instead of adding an entry
right away we can wait for DataNodes to report this container via
ContainerReport and then add it. Having separate API will allow
ContainerReportHandler to do this mutation and send a message to
ReplicationManager accordingly. Let us know if you think otherwise.
{quote}It would be nice to have DatanodeDetails as varargs in
addContainerReplica, so that it will be easy to update/add multiple datanodes
at the same time.{quote}
Done
{quote}Inside ContainerStateManager#allocateContainer method we should update
the container replica after we add the container to ContainerStateMap.{quote}
same as above.
Failed test case timed out in jenkins run, passes locally.
was (Author: ajayydv):
[~nandakumar131] thanks for review.
{quote}Instead of adding container entry in contReplicaMap inside
addContainerReplica call, can we do it in addContainer call. This way
containerMap and contReplicaMap will be consistent.
{quote}
addContainer is called from ContainerStateManager#allocateContainer as well.
This means Container is probably in open state and we don't want
replicationManager to handle it until it is closed. Instead of adding an entry
right away we can wait for DataNodes to report this container via
ContainerReport and then add it. Having separate API will allow
ContainerReportHandler to do this mutation and send a message to
ReplicationManager accordingly.
{quote}It would be nice to have DatanodeDetails as varargs in
addContainerReplica, so that it will be easy to update/add multiple datanodes
at the same time.{quote}
Done
{quote}Inside ContainerStateManager#allocateContainer method we should update
the container replica after we add the container to ContainerStateMap.{quote}
same as above.
Failed test case timed out in jenkins run, passes locally.
> Add the ReplicaMaps to ContainerStateManager
>
>
> Key: HDDS-228
> URL: https://issues.apache.org/jira/browse/HDDS-228
> Project: Hadoop Distributed Data Store
> Issue Type: Improvement
> Components: SCM
>Reporter: Anu Engineer
>Assignee: Ajay Kumar
>Priority: Major
> Fix For: 0.2.1
>
> Attachments: HDDS-228.00.patch, HDDS-228.01.patch, HDDS-228.02.patch
>
>
> We need to maintain a list of data nodes in the SCM that tells us where a
> container is located. This created from the container reports. The HDDS-175
> refactored the class to make this separation easy and this JIRA is a followup
> that keeps a hash table to track this information.
--
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
[jira] [Comment Edited] (HDDS-228) Add the ReplicaMaps to ContainerStateManager
[
https://issues.apache.org/jira/browse/HDDS-228?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16534283#comment-16534283
]
Ajay Kumar edited comment on HDDS-228 at 7/5/18 11:34 PM:
--
[~nandakumar131] thanks for review.
{quote}Instead of adding container entry in contReplicaMap inside
addContainerReplica call, can we do it in addContainer call. This way
containerMap and contReplicaMap will be consistent.
{quote}
addContainer is called from ContainerStateManager#allocateContainer as well.
This means Container is probably in open state and we don't want
replicationManager to handle it until it is closed. Instead of adding an entry
right away we can wait for DataNodes to report this container via
ContainerReport and then add it. Having separate API will allow
ContainerReportHandler to do this mutation and send a message to
ReplicationManager accordingly.
{quote}It would be nice to have DatanodeDetails as varargs in
addContainerReplica, so that it will be easy to update/add multiple datanodes
at the same time.{quote}
Done
{quote}Inside ContainerStateManager#allocateContainer method we should update
the container replica after we add the container to ContainerStateMap.{quote}
same as above.
Failed test case timed out in jenkins run, passes locally.
was (Author: ajayydv):
[~nandakumar131] thanks for review.
{quote}Instead of adding container entry in contReplicaMap inside
addContainerReplica call, can we do it in addContainer call. This way
containerMap and contReplicaMap will be consistent.
{quote}
addContainer is called from ContainerStateManager#allocateContainer as well.
This means Container is probably in open state and we don't want
replicationManager to handle it until it is closed. Instead of adding an entry
right away we can wait for DataNodes to report this container via
ContainerReport and then add it. Having separate API will allow
ContainerReportHandler to do this mutation and send a message to
ReplicationManager accordingly.
{quote}It would be nice to have DatanodeDetails as varargs in
addContainerReplica, so that it will be easy to update/add multiple datanodes
at the same time.{quote}
Done
{quote}Inside ContainerStateManager#allocateContainer method we should update
the container replica after we add the container to ContainerStateMap.{quote}
same as above.
> Add the ReplicaMaps to ContainerStateManager
>
>
> Key: HDDS-228
> URL: https://issues.apache.org/jira/browse/HDDS-228
> Project: Hadoop Distributed Data Store
> Issue Type: Improvement
> Components: SCM
>Reporter: Anu Engineer
>Assignee: Ajay Kumar
>Priority: Major
> Attachments: HDDS-228.00.patch, HDDS-228.01.patch, HDDS-228.02.patch
>
>
> We need to maintain a list of data nodes in the SCM that tells us where a
> container is located. This created from the container reports. The HDDS-175
> refactored the class to make this separation easy and this JIRA is a followup
> that keeps a hash table to track this information.
--
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
