[
https://issues.apache.org/jira/browse/HDDS-245?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16569890#comment-16569890
]
Elek, Marton commented on HDDS-245:
-----------------------------------
Thank you very much the review of [~anu], [~ajayydv], [~nandakumar131]. I fixed
all of the trivial fixes (doubled, replication set, etc.). I will mention only
the problem which are not clear for me.
1. new HashSet() in ReportResult:
It was required as the missingContainers/newContainers (which are created all
the time by Node2ContainerMap.processReport) were not always propagated:
{code}
if (!newContainers.isEmpty() && missingContainers.isEmpty()) {
return ReportResult.ReportResultBuilder.newBuilder()
.setStatus(ReportStatus.NEW_CONTAINERS_FOUND)
.setNewContainers(newContainers)
.build();
}
{code}
Here the setMissingContainers are not set. I can modify the processReport to
always set both the sets, but it's against the builder pattern. The advantage
of the builder pattern is that it can create object with setting only a few of
the field values.
Finally I removed the new HashSet part and modified the builder to use a
Collections.emptySet (thanks [~nandakumar131]) in case of null missing/new
container set.
2. Thanks to share [~anu] the historic reason to separated the
node2ContainerMap.updateDatanodeMap and node2ContainerMap.updateDatanodeMap.
Unfortunately I don't understand what should I fix. The current code seems to
be ok for me:
{code}
//update state in container db and trigger close container events
containerMapping.processContainerReports(datanodeOrigin, containerReport);
.
..
ReportResult reportResult = node2ContainerMap
.processReport(datanodeOrigin.getUuid(), containerIds);
//we have the report, so we can update the states for the next iteration.
node2ContainerMap
.updateDatanodeMap(datanodeOrigin.getUuid(), containerIds);
{code}
If I understood well the main thing here is to update the node2ContainerMap at
last and this seems to be done (My only concern was in the previous comments
that I wouldn't like to update state *after* publisher.fireEvent as I would
like to guarantee that the state in the maps are up-to-date at the time of the
event sending. (In any other case the event could be delivered faster than the
state update))
3. I created HDDS-326 for sending the enable/disable replication event.
4. [~ajayydv] Could you please share the the details where Streams are slow? I
prefer to use it as it 's more expressive and I would like to check the
problems.
5. [~ajayydv] I renamed Node2ContainerMap#updateDatanodeMap to
setContainersForDatanode (but feel free to suggest better name if you have). I
think it's more clean that the whole set will be updated for a specific
datanode.
> Handle ContainerReports in the SCM
> ----------------------------------
>
> Key: HDDS-245
> URL: https://issues.apache.org/jira/browse/HDDS-245
> Project: Hadoop Distributed Data Store
> Issue Type: Improvement
> Components: SCM
> Reporter: Elek, Marton
> Assignee: Elek, Marton
> Priority: Major
> Fix For: 0.2.1
>
> Attachments: HDDS-245.001.patch, HDDS-245.002.patch,
> HDDS-245.003.patch, HDDS-245.004.patch
>
>
> HDDS-242 provides a new class ContainerReportHandler which could handle the
> ContainerReports from the SCMHeartbeatDispatchere.
> HDDS-228 introduces a new map to store the container -> datanode[] mapping
> HDDS-199 implements the ReplicationManager which could send commands to the
> datanodes to copy the datanode.
> To wire all these components, we need to put implementation to the
> ContainerReportHandler (created in HDDS-242).
> The ContainerReportHandler should process the new ContainerReportForDatanode
> events, update the containerStateMap and node2ContainerMap and calculate the
> missing/duplicate containers and send the ReplicateCommand to the
> ReplicateManager.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
