[jira] Commented: (ZOOKEEPER-230) Improvements to FLE

["Mahadev konar (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/ZOOKEEPER-230?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12652927#action_12652927 ] 

Mahadev konar commented on ZOOKEEPER-230:
-----------------------------------------

the patch loooks good. here are some comments :

- 60000 should be declared as a final static int at the beginning of the class and used as a variable inside the code rather than using the value itself. Its easier to look for and replace.
- notimteout is updated twice - once when no new notifications are there and other time when its processing some notifications. Why is it incremented in the second case? also, its not bounded in the second case when notification != null
- LOG.info("Create new connection"); in QuorumCnxnManager does not give any info on which host its creating the connection to. The log message should be more informative so that its easy to find out what happened late. ( I understand its been the same earlier but it will be good if we can make it more informative).
- public void connect() . does this method need to be public? Also, some javadoc comments for developeers, on what the method does would be useful.



> Improvements to FLE
> -------------------
>
>                 Key: ZOOKEEPER-230
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-230
>             Project: Zookeeper
>          Issue Type: Improvement
>          Components: leaderElection
>    Affects Versions: 3.0.0
>            Reporter: Flavio Paiva Junqueira
>            Assignee: Flavio Paiva Junqueira
>             Fix For: 3.1.0
>
>         Attachments: ZOOKEEPER-230.patch, ZOOKEEPER-230.patch, ZOOKEEPER-230.patch
>
>
> I'm about to attach a patch that implements the following modifications:
> . Currently, if a server is on leader election and doesn't receive a notification for some amount of time t, then it sends a new set of notifications if at least one server has delivered a message from the previous set. With this patch, the amount of time a server waits for a notification before sending a new set increases exponentially;
> . I have separated connecting to servers and queuing new notification messages. Before they were all in the same message. The advantage is that now I can tell to an instance of QuorumCnxManager to try to connect to other servers without generating new notification messages;
> . I have changed the logging level of several messages on QuorumCnxManager. They were "warn", but they should really be either "info" or "debug". I've changed them to info.
>  

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.