[
https://issues.apache.org/jira/browse/HDDS-325?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16590357#comment-16590357
]
Elek, Marton commented on HDDS-325:
-----------------------------------
Thanks you very much [~ljain] the patch. It's much more simple and clean for me.
* Over it looks good to me, but I am not very familiar with the deletion part.
Additional comments would be good from somebody else. As far as I understood
it's good.
* Adding ContainerBlocksDeletionACKProto to the status field looks strange to
me, but to be honest I have no better idea. I guess with proto 3 we will have
better tools to use OOP polymorphism.
* I am not sure but I think in RetriableDatanodeEventWatcher.onTimeout we need
to send the message to SCMEvents.DATANODE_COMMAND and not
SCMEvents.RETRIABLE_DATANODE_COMMAND (A unit test would help to decide this
question...)
* About EventWatcher. I don't understand exactly your example. I think one
EventWatcher (or more preciously one RetriableDatanodeEventWatcher) would be
enough.
Let's say we have two kind of commands :
new CommandForDatanode<>(dnId, new DeleteBlocksCommand)
new CommandForDatanode<>(dnId, new EatBananaCommand)
Both could be sent to the SCMEvents.RETRIABLE_DATANODE_COMMAND for
RetriableDatanodeEventWatcher (and for the scmNodeManager) and they could
handle both of them. With listening on the CMD_STATUS_REPORT event
RetriableDatanodeEventWatcher could handle all of the retries in one place.
More formerly I have two problems with the changes in EventWatcher:
1) we don't need it in this patch (the new EventWatcher.watchEvents method is
unused). I would add it when it required.
2) I think the main problem is that the addHandler calls are part of the
EventWatcher (this is my fault, but since the original creation all of the
wiring logic moved to the StorageContainerManager). I think we can either
remove the wiring logic from the EventHandler and move to the
StorageContainerManager (where we can easily add EventWatcher as a listener of
multiple events) or we can create a builder (EventHandler.watchEvents is almost
like a builder). Or both! But I propose to do it in a separated jira (as it's
not strictly required now as I understood, the watchEvents is still unused)
One minor nit: I would follow the naming convention in SCMEvents: L92 (using
underscores).
> Add event watcher for delete blocks command
> -------------------------------------------
>
> Key: HDDS-325
> URL: https://issues.apache.org/jira/browse/HDDS-325
> Project: Hadoop Distributed Data Store
> Issue Type: Bug
> Components: Ozone Datanode, SCM
> Reporter: Lokesh Jain
> Assignee: Lokesh Jain
> Priority: Major
> Fix For: 0.2.1
>
> Attachments: HDDS-325.001.patch, HDDS-325.002.patch,
> HDDS-325.003.patch, HDDS-325.004.patch, HDDS-325.005.patch
>
>
> This Jira aims to add watcher for deleteBlocks command. It removes the
> current rpc call required for datanode to send the acknowledgement for
> deleteBlocks.
--
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
