[
https://issues.apache.org/jira/browse/HDFS-11492?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15932380#comment-15932380
]
Yuanbo Liu edited comment on HDFS-11492 at 3/20/17 9:32 AM:
------------------------------------------------------------
[~anu] Thanks for your patch. Nice pattern to use dispatcher to handler
commands.
Minor comments:
{{DatanodeStateMachine.java}}
{quote}
initCommandHandlerThread(conf);
{quote}
Would it be better to init command handler thread in start method, after
{{container.start()}}
{quote}
Runnable processCommandQueue = () -> {}
{quote}
Suggest to add Thread.sleep here.
{quote}
private long commandshandled;
{quote}
Currently I don't foresee the purpose of counting command number here, would
you please elaborate a bit.
{{Handler.java}}
Would you mind renaming it to CommandHandler.java or something else. There is
already another Handler.java in ozone world.
{{ContainerReportHandler.java}}
{quote}
List<ContainerData> closedContainerList = container.getContainerReports();
for (ContainerData cd : closedContainerList) {
Preconditions.checkArgument(!cd.isOpen(), "Closed containers " +
"expected.");
{quote}
We have filtered the opened container data in
{{container.getContainerReports();}}, so I think there is no need to check
whether it's opened in the loop, right? But I'm ok if you wanna keep it.
{quote}
LOG.error("Unable to process the Container Report command.", ex);
return;
{quote}
'return' is unnecessary here.
{{CommandDispatcher.java}}
{quote}
for (Handler h : handlers) {
if(handlerMap.containsKey(h)){
{quote}
should be handlerMap.containsKey(h.getCommandType())
{quote}
return new CommandDispatcher(this.container, this.connectionManager,
this.context, handlerList.toArray(new Handler\[0\]));
{quote}
Please use new Handler\[handlerList.size()\] instead.
question:
What's your plan of calculating the size of a container?
About test failures:
I tested some in my local env and they're all passed, so I believe the
failures are related to the Apache Jenkins env.
Thanks for your patch, good job!
was (Author: yuanbo):
[~anu] Thanks for your patch. Nice pattern to use dispatcher to handler
commands.
Minor comments:
{{DatanodeStateMachine.java}}
{quote}
initCommandHandlerThread(conf);
{quote}
Would it be better to init command handler thread in start method, after
{{container.start()}}
{quote}
Runnable processCommandQueue = () -> {}
{quote}
Suggest to add Thread.sleep here.
{quote}
private long commandshandled;
{quote}
Currently I don't foresee the purpose of counting command number here, would
you please elaborate a bit.
{{Handler.java}}
Would you mind renaming it to CommandHandler.java or something else. There is
already another Handler.java in ozone world.
{{ContainerReportHandler.java}}
{quote}
List<ContainerData> closedContainerList = container.getContainerReports();
for (ContainerData cd : closedContainerList) {
Preconditions.checkArgument(!cd.isOpen(), "Closed containers " +
"expected.");
{quote}
We have filtered the opened container data in
{{container.getContainerReports();}}, so I think there is no need to check
whether it's opened in the loop, right? But I'm ok if you wanna keep it.
{quote}
LOG.error("Unable to process the Container Report command.", ex);
return;
{quote}
'return' is unnecessary here.
{{CommandDispatcher.java}}
{quote}
for (Handler h : handlers) {
if(handlerMap.containsKey(h)){
{quote}
should be handlerMap.containsKey(h.getCommandType())
{quote}
return new CommandDispatcher(this.container, this.connectionManager,
this.context, handlerList.toArray(new Handler[0]));
{quote}
Please use new Handler\[handlerList.size()\] instead.
> Ozone: Add the ability to handle sendContainerReport Command
> ------------------------------------------------------------
>
> Key: HDFS-11492
> URL: https://issues.apache.org/jira/browse/HDFS-11492
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: ozone
> Affects Versions: HDFS-7240
> Reporter: Anu Engineer
> Assignee: Anu Engineer
> Attachments: HDFS-11492-HDFS-7240.001.patch
>
>
> Once a container report is ready datanodes sends that information to SCM. SCM
> returns a command asking the data node to send container report. Add the
> ability to handle this command on datanode side and send the actual container
> Report.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
