[
https://issues.apache.org/jira/browse/HDDS-364?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16589114#comment-16589114
]
Ajay Kumar edited comment on HDDS-364 at 8/22/18 5:03 PM:
----------------------------------------------------------
[~elek] thanks for review. Please see my response inline:
{quote}What do you think about Node2ContainerMap? Do we need to update it as
well?{quote}
I think we can get rid of processReport by sending missing and new containers
(deltas) directly form DN. This will reduce the SCM-DN communication and will
make DN responsible for sending only changed state. But this can be done in
separate jira.
{quote}2. As I see all the containers will be persisted twice. (First, they
will be imported, after that they will be reconciled.). Don't think it's a big
problem. IMHO later we need to cleanup all the processing path anyway.
One option may be just saving all the initial data to the state map without
processing the reports (checking the required closing state, etc.). The
downside here is some action would be delayed until the first real container
report.{quote}
Not sure which codepath you are referring here. This patch adds containers
reported in register call to replicaMap which is in memory. Everything else
remains same.
{quote} The import part (in case of isRegisterCall=true) is the first part of
processContainerReport method. I think it would be very easy to move to a
separated method and call it independently from
SCMDatanodeProtocolServer.register method. Could be more simple, and maybe it
could be easier to test. Currently (as I understood) there is no specific test
to test the isRegisterCall=true path. But this is not a blocking problem.
Depends from your consideration{quote}
Approach you are suggesting is close to the attached first patch. Had a
discussion regarding this with [~xyao]. Moving it to processContainerReport is
a small optimization to not iterate through whole list. (For a DN with 24 disks
of 12 TB each we can have roughly 57600 containers of 5Gb) iterating through it
and adding it to replicaMap should be quick but a large cluster with large no
of DN may overwhelm the SCM during initial registration process. Moving this
inside processContainerReport results in only one iteration of that list. At
some point we can refactor this along with logic in ContainerCommandHandler and
Node2ContainerMap#processReport.
Updated test in TestContainerMapping checks call to processContainerReport with
isRegisterCall=true.
was (Author: ajayydv):
[~elek] thanks for review. Please see my response inline:
{quote}What do you think about Node2ContainerMap? Do we need to update it as
well?{quote}
I think we can get rid of processReport by sending missing and new containers
(deltas) directly form DN. This will reduce the SCM-DN communication and will
make DN responsible for sending only changed state. But this can be done in
separate jira.
{quote}2. As I see all the containers will be persisted twice. (First, they
will be imported, after that they will be reconciled.). Don't think it's a big
problem. IMHO later we need to cleanup all the processing path anyway.
One option may be just saving all the initial data to the state map without
processing the reports (checking the required closing state, etc.). The
downside here is some action would be delayed until the first real container
report.{quote}
Not sure which codepath you are referring here. This patch adds containers
reported in register call to replicaMap which is in memory. Everything else
remains same.
{quote} The import part (in case of isRegisterCall=true) is the first part of
processContainerReport method. I think it would be very easy to move to a
separated method and call it independently from
SCMDatanodeProtocolServer.register method. Could be more simple, and maybe it
could be easier to test. Currently (as I understood) there is no specific test
to test the isRegisterCall=true path. But this is not a blocking problem.
Depends from your consideration{quote}
Approach you are suggesting is close to the attached first patch. Had a
discussion regarding this with [~xyao]. Moving it to processContainerReport is
a small optimization to not iterate through whole list. (For a DN with 24 disks
of 12 TB each we can have roughly 57600 containers of 5Gb) iterating through it
and adding it to replicaMap should be quick but a large cluster with large no
of DN may overwhelm the SCM during initial registration process. Moving this
inside processContainerReport results in only one iteration of that list. At
some point we can refactor this along with logic in ContainerCommandHandler and
Node2ContainerMap#processReport.
> Update open container replica information in SCM during DN register
> -------------------------------------------------------------------
>
> Key: HDDS-364
> URL: https://issues.apache.org/jira/browse/HDDS-364
> Project: Hadoop Distributed Data Store
> Issue Type: New Feature
> Reporter: Ajay Kumar
> Assignee: Ajay Kumar
> Priority: Major
> Fix For: 0.2.1
>
> Attachments: HDDS-364.00.patch, HDDS-364.01.patch
>
>
> Update open container replica information in SCM during DN register.
--
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
