[jira] Updated: (ZOOKEEPER-472) Making DataNode not instantiate a HashMap when the node is ephmeral
[ https://issues.apache.org/jira/browse/ZOOKEEPER-472?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Erik Holstad updated ZOOKEEPER-472: --- Attachment: zookeeper-472.patch I decided to take a lesser approach and just do what this Jira originally said, so basically creating two new methods, addChild() and removeChild() and making the variable children in DataNode private. The children set in DataNode is not instantiated at node creation but instead the first time addChild() is being called. No synchronization is done one the calls addChild and removeChild since it looks like most other calls are synchronized on the node itself. Not really sure why getChildren is synchronized though, since as far as I can see it all the uses of it are also synchronized on the node. According to the docs it says that 2 spaces should be used instead of 4. Is that a miss in the docs or is that what is really being used? I'm using 2 for this patch. Making DataNode not instantiate a HashMap when the node is ephmeral --- Key: ZOOKEEPER-472 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-472 Project: Zookeeper Issue Type: Improvement Components: server Affects Versions: 3.1.1, 3.2.0 Reporter: Erik Holstad Assignee: Erik Holstad Priority: Minor Fix For: 3.3.0 Attachments: zookeeper-472.patch Looking at the code, there is an overhead of a HashSet object for that nodes children, even though the node might be an ephmeral node and cannot have children. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Created: (ZOOKEEPER-525) Changing children SetString to Setbyte[] in DataNode
Changing children SetString to Setbyte[] in DataNode Key: ZOOKEEPER-525 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-525 Project: Zookeeper Issue Type: Improvement Components: server Reporter: Erik Holstad Priority: Minor Fix For: 3.3.0 For every instance of string there is an overhead of 48B compared to using byte[], on a 64 bit system, that seems unnecessary. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
Accessing internal variables in DataNode and synchronization.
In most places in the server side code, internal variables are accessed directly via something like, node.parent... instead of making the variable private and using getters and setters, what is the purpose of this? Another question is why the synchronization on for example the DataNode is done on the node itself and not on the variables access, it seems reasonable to me that 2 different threads would be allowed to modify for example the data and the children at the same time, or is there something that I'm missing here, watchers? Erik
Re: Accessing internal variables in DataNode and synchronization.
Hi Erik, There is no such reason for accessing variable without acess and getters. We should not be doing that. Its good to have getters and setters. The synchronization on DataNode is a safe lock that would prevent developers being concerned about finer locks and maintaining them. In our performance experiments we havent ever seen any problems with these. So, we did not want to make it more complex with finer granularity locks, Thanks mahadev On 9/11/09 2:14 PM, Erik Holstad erikhols...@gmail.com wrote: In most places in the server side code, internal variables are accessed directly via something like, node.parent... instead of making the variable private and using getters and setters, what is the purpose of this? Another question is why the synchronization on for example the DataNode is done on the node itself and not on the variables access, it seems reasonable to me that 2 different threads would be allowed to modify for example the data and the children at the same time, or is there something that I'm missing here, watchers? Erik
Re: Accessing internal variables in DataNode and synchronization.
Hi Mahadev! Sounds good that you want getters and setters, it makes most things easier to reason about, even though it might add a couple of extra lines of code. The reason that I'm bringing up the synchronization is that some of the methods in the DataNode are synchronized, which I like if it means that you can get more concurrency on the Object. But now it feels like there is a mix of both approaches which is bit confusing to me. I think that more and more people are going to start pushing ZooKeeper towards it's limits and then it might be reasonable to change the current synchronization schema from the node to the individual variables and maybe even get fancy and use Volatile in some places. Erik
[jira] Updated: (ZOOKEEPER-516) add support for 10 minute test ie pre-commit test
[ https://issues.apache.org/jira/browse/ZOOKEEPER-516?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Patrick Hunt updated ZOOKEEPER-516: --- Assignee: Patrick Hunt Status: Patch Available (was: Open) add support for 10 minute test ie pre-commit test --- Key: ZOOKEEPER-516 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-516 Project: Zookeeper Issue Type: Task Components: tests Reporter: Patrick Hunt Assignee: Patrick Hunt Priority: Minor Fix For: 3.3.0 Attachments: ZOOKEEPER-516.patch Our unit tests are now taking 10 minutes (20 actually). We need to set things up such that we have a 10 minute pre commit build, and a longer running test of tests (unbounded for all intents/purposes). Mainly this requires us to categorize our tests. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Updated: (ZOOKEEPER-516) add support for 10 minute test ie pre-commit test
[ https://issues.apache.org/jira/browse/ZOOKEEPER-516?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Patrick Hunt updated ZOOKEEPER-516: --- Attachment: ZOOKEEPER-516.patch this patch gets the time under 10min for ant -Dtest.quick=yes -Dtest.junit.fork.mode=once clean test on my old/slow/singlecore laptop. notice that this is from clean so no only are the tests being run but the code (java and c) is included in the time as well. Notes - Java: My laptop I was able to reduce the runtime by over 30% just by cleaning up waits fixed a problem in fork once mode for superuser auth test.quick (off by default) when turned on will skip the hammer tests cleanuped the client cleanup test - instead of forcing us to run out of fds I use the jvm instrumentation to monitor the number of active fds. (although this really just works on unix) Notes - C/C++: I added elapsed time for each of the tests I didn't change much here wrt timing. there's some time that could be saved (esp around client est) I did fix the logging of zk client output - it was turned off for some reason (now output similar to java clients) I also fixed some of the logging formats used (esp around hex formatting of session/xid) add support for 10 minute test ie pre-commit test --- Key: ZOOKEEPER-516 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-516 Project: Zookeeper Issue Type: Task Components: tests Reporter: Patrick Hunt Priority: Minor Fix For: 3.3.0 Attachments: ZOOKEEPER-516.patch Our unit tests are now taking 10 minutes (20 actually). We need to set things up such that we have a 10 minute pre commit build, and a longer running test of tests (unbounded for all intents/purposes). Mainly this requires us to categorize our tests. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Updated: (ZOOKEEPER-516) add support for 10 minute test ie pre-commit test
[ https://issues.apache.org/jira/browse/ZOOKEEPER-516?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Patrick Hunt updated ZOOKEEPER-516: --- Status: Open (was: Patch Available) add support for 10 minute test ie pre-commit test --- Key: ZOOKEEPER-516 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-516 Project: Zookeeper Issue Type: Task Components: tests Reporter: Patrick Hunt Assignee: Patrick Hunt Priority: Minor Fix For: 3.3.0 Attachments: ZOOKEEPER-516.patch Our unit tests are now taking 10 minutes (20 actually). We need to set things up such that we have a 10 minute pre commit build, and a longer running test of tests (unbounded for all intents/purposes). Mainly this requires us to categorize our tests. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Updated: (ZOOKEEPER-516) add support for 10 minute test ie pre-commit test
[ https://issues.apache.org/jira/browse/ZOOKEEPER-516?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Patrick Hunt updated ZOOKEEPER-516: --- Status: Patch Available (was: Open) add support for 10 minute test ie pre-commit test --- Key: ZOOKEEPER-516 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-516 Project: Zookeeper Issue Type: Task Components: tests Reporter: Patrick Hunt Assignee: Patrick Hunt Priority: Minor Fix For: 3.3.0 Attachments: ZOOKEEPER-516.patch Our unit tests are now taking 10 minutes (20 actually). We need to set things up such that we have a 10 minute pre commit build, and a longer running test of tests (unbounded for all intents/purposes). Mainly this requires us to categorize our tests. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Created: (ZOOKEEPER-526) Change the access pattern in the DataNode from direct access to the use of getters and setters
Change the access pattern in the DataNode from direct access to the use of getters and setters -- Key: ZOOKEEPER-526 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-526 Project: Zookeeper Issue Type: Improvement Components: server Reporter: Erik Holstad Priority: Minor Fix For: 3.3.0 -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-516) add support for 10 minute test ie pre-commit test
[ https://issues.apache.org/jira/browse/ZOOKEEPER-516?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=12754434#action_12754434 ] Hadoop QA commented on ZOOKEEPER-516: - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12419349/ZOOKEEPER-516.patch against trunk revision 808557. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 98 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/5/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/5/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/5/console This message is automatically generated. add support for 10 minute test ie pre-commit test --- Key: ZOOKEEPER-516 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-516 Project: Zookeeper Issue Type: Task Components: tests Reporter: Patrick Hunt Assignee: Patrick Hunt Priority: Minor Fix For: 3.3.0 Attachments: ZOOKEEPER-516.patch Our unit tests are now taking 10 minutes (20 actually). We need to set things up such that we have a 10 minute pre commit build, and a longer running test of tests (unbounded for all intents/purposes). Mainly this requires us to categorize our tests. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.