[
https://issues.apache.org/jira/browse/ZOOKEEPER-1214?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13180512#comment-13180512
]
[email protected] commented on ZOOKEEPER-1214:
----------------------------------------------------------
bq. On 2011-12-14 23:36:06, Patrick Hunt wrote:
bq. > /src/java/main/org/apache/zookeeper/jmx/MBeanRegistry.java, lines 126-127
bq. > <https://reviews.apache.org/r/2208/diff/1/?file=48123#file48123line126>
bq. >
bq. > cleanup this typo (2 returns)
Done.
bq. On 2011-12-14 23:36:06, Patrick Hunt wrote:
bq. > /src/java/main/org/apache/zookeeper/jmx/MBeanRegistry.java, line 130
bq. > <https://reviews.apache.org/r/2208/diff/1/?file=48123#file48123line130>
bq. >
bq. > let's call this
bq. >
bq. > getRegisteredBeans()
bq. >
Ok.
bq. On 2011-12-14 23:36:06, Patrick Hunt wrote:
bq. > /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java, lines
757-767
bq. > <https://reviews.apache.org/r/2208/diff/1/?file=48124#file48124line757>
bq. >
bq. > Add back the try/catch protection. This was necessary in some cases
(I can't remember why off the top of my head though).
I've enhanced the exception handling during "public void unregister(ZKMBeanInfo
bean)" method invocation since JMException was caught and logged two times: 1)
private void unregister(String path,ZKMBeanInfo bean) throws JMException, 2)
public void unregister(ZKMBeanInfo bean).
I've also added a "catch(Throwable t)" to "public void unregister(ZKMBeanInfo
bean)" method that will generate an error log instead of a warn.
bq. On 2011-12-14 23:36:06, Patrick Hunt wrote:
bq. > /src/java/test/org/apache/zookeeper/test/QuorumUtilTest.java, line 1
bq. > <https://reviews.apache.org/r/2208/diff/1/?file=48126#file48126line1>
bq. >
bq. > license header is missing. add the std boilerplate found on any
other java file in the project.
As you can see I'm a newbie contributor ;-)
bq. On 2011-12-14 23:36:06, Patrick Hunt wrote:
bq. > /src/java/test/org/apache/zookeeper/test/QuorumUtilTest.java, line 15
bq. > <https://reviews.apache.org/r/2208/diff/1/?file=48126#file48126line15>
bq. >
bq. > add a comment to the class describing what this set of tests is for
Done.
bq. On 2011-12-14 23:36:06, Patrick Hunt wrote:
bq. > /src/java/test/org/apache/zookeeper/test/QuorumUtilTest.java, line 16
bq. > <https://reviews.apache.org/r/2208/diff/1/?file=48126#file48126line16>
bq. >
bq. > no tabs, reformat with spaces only. (you'll probably need to cleanup
some of your other changes at the same time)
Done. It would be nice that the eclipse ant task will create the .setting files
to set all formatting rules and useful save actions (like apply formatting).
I'll investigate it and upload as a new jira if success.
bq. On 2011-12-14 23:36:06, Patrick Hunt wrote:
bq. > /src/java/test/org/apache/zookeeper/test/QuorumUtilTest.java, line 19
bq. > <https://reviews.apache.org/r/2208/diff/1/?file=48126#file48126line19>
bq. >
bq. > it's fine to ref a jira here, however there should also be some
details on what this test is doing. (short summary esp as the test itself is
pretty clear)
Done.
- César
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2208/#review3914
-----------------------------------------------------------
On 2011-10-05 11:59:30, César Álvarez Núñez wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/2208/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-10-05 11:59:30)
bq.
bq.
bq. Review request for zookeeper.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. See https://issues.apache.org/jira/browse/ZOOKEEPER-1214
bq.
bq.
bq. This addresses bug ZOOKEEPER-1214.
bq. https://issues.apache.org/jira/browse/ZOOKEEPER-1214
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. /src/java/test/org/apache/zookeeper/test/QuorumUtil.java 1179165
bq. /src/java/test/org/apache/zookeeper/test/QuorumUtilTest.java
PRE-CREATION
bq. /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java
1179166
bq. /src/java/main/org/apache/zookeeper/jmx/MBeanRegistry.java 1169669
bq.
bq. Diff: https://reviews.apache.org/r/2208/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. -
bq.
bq.
bq. Thanks,
bq.
bq. César
bq.
bq.
> QuorumPeer should unregister only its previsously registered MBeans instead
> of use MBeanRegistry.unregisterAll() method.
> ------------------------------------------------------------------------------------------------------------------------
>
> Key: ZOOKEEPER-1214
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1214
> Project: ZooKeeper
> Issue Type: Bug
> Components: quorum
> Reporter: César Álvarez Núñez
> Assignee: César Álvarez Núñez
> Fix For: 3.5.0
>
> Attachments: ZOOKEEPER-1214.patch, ZOOKEEPER-1214.patch,
> ZOOKEEPER-1214.patch
>
>
> When a QuorumPeer thread dies, it is unregistering *all* ZKMBeanInfo MBeans
> previously registered on its java process; including those that has not been
> registered by itself.
> It does not cause any side effect in production environment where each server
> is running on a separate java process; but fails when using
> "org.apache.zookeeper.test.QuorumUtil" to programmatically start up a
> zookeeper server ensemble and use its provided methods to force Disconnected,
> SyncConnected or SessionExpired events; in order to perform some
> basic/functional testing.
> Scenario:
> * QuorumUtil qU = new QuorumUtil(1); // It creates a 3 servers ensemble.
> * qU.startAll(); // Startup all servers: 1 Leader + 2 Followers
> * qU.shutdown\(i\); // i is a number from 1 to 3. It shutdown one server.
> The last method causes that a QuorumPeer will die, invoking the
> MBeanRegistry.unregisterAll() method.
> As a result, *all* ZKMBeanInfo MBeans are unregistered; including those
> belonging to the other QuorumPeer instances.
> When trying to restart previous server (qU.restart\(i\)) an AssertionError is
> thrown at MBeanRegistry.register(ZKMBeanInfo bean, ZKMBeanInfo parent)
> method, causing the QuorumPeer thread dead.
> To solve it:
> * MBeanRegistry.unregisterAll() method has been removed.
> * QuorumPeer only unregister its ZKMBeanInfo MBeans.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
