hadoop-common-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Arpit Agarwal (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HADOOP-10376) Refactor refresh*Protocols into a single generic refreshConfigProtocol
Date Mon, 02 Jun 2014 19:15:03 GMT

    [ https://issues.apache.org/jira/browse/HADOOP-10376?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14015753#comment-14015753

Arpit Agarwal commented on HADOOP-10376:

Hi Chris, my comments below. The approach looks fine to me.

* Please consider making {{identifier}} _optional_. _required_ can be a hassle when rev-ing

* Once {{identifier}} is an optional field, please add a corresponding check for {{request.hasIdentifier}}.
For now you can just throw an exception if no identifier was supplied.

* {{userMessage}} is optional, so check for its presence with {{resp.hasUserMessage}}? I realize
the server will pass an empty string if the handler did not return a message but that is a
detail the client doesn't need to know of.

* Do you have a use case in mind for multiple handlers per identifier? If we need to pass
multiple messages, instead of formatting the responses into a string on the server, why not
pass each string to the client in the RPC response? i.e. in GenericRefreshProtocol.proto,
make {{userMessage}} a repeated field. This lets the client choose the presentation.

* Consider using Guava MultiMap for {{handlerTable}}. If you want to continue using the ‘map
of sets’ approach then the following should be fixed:
** {{unregisterAll}} - Why not just remove {{identifier}} and its corresponding value?
** {{getHandlers}} should return an unmodifiableSet. Also the return type should be an abstract

* _All other args after are sent to the host_ - clarify they are sent as uninterpreted strings?
* Does _host:port_ need to be a required argument? None of the other refresh calls require

* {{testMultipleHandlers}} - add verification that there was interaction with both mocks?

> Refactor refresh*Protocols into a single generic refreshConfigProtocol
> ----------------------------------------------------------------------
>                 Key: HADOOP-10376
>                 URL: https://issues.apache.org/jira/browse/HADOOP-10376
>             Project: Hadoop Common
>          Issue Type: Improvement
>            Reporter: Chris Li
>            Assignee: Chris Li
>            Priority: Minor
>         Attachments: HADOOP-10376.patch, HADOOP-10376.patch, RefreshFrameworkProposal.pdf
> See https://issues.apache.org/jira/browse/HADOOP-10285
> There are starting to be too many refresh*Protocols We can refactor them to use a single
protocol with a variable payload to choose what to do.
> Thereafter, we can return an indication of success or failure.

This message was sent by Atlassian JIRA

View raw message