[ http://jira.jboss.com/jira/browse/JBTM-352?page=all ] Ivan Szanto updated JBTM-352: ----------------------------- Attachment: TransactionReaper.java This fixes the memory leak. Tested it by running ~ 100,000 transactions with good results. This is why I think it is a good starting point, but this is probably not the best way to fix this problem, because it is both ugly and slow to use instanceof. I wrote it this way, because there are 3 methods (remove, getTimeout and getRemainingTimeout) that pass the hashtable key as an Object, therefore I was not sure if the only things I can recieve here are Reapable and Uid objects. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://jira.jboss.com/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira

[ http://jira.jboss.com/jira/browse/JBTM-352?page=comments#action_12405209 ] Andrew Dinn commented on JBTM-352: ---------------------------------- Well spotted. Unfortunately, this is probably not a valid fix. Essentially you are using the Uid as a key for entries in map _timeouts and this is inadequate for the general case. The reason for using Reapable objects as the key is that it allows the comparison to be defined by the Reapable -- this is possible because when Hashmap does a get or put with key 'k' it compares against existing entries using k.equals(entry.key). So, any Reapable can implement 'equals' to ensure that it compares correctly against other Reapables. If we switch to keying using Uids then this test will by bypassed. Your 'fix' may work with your test but it may break other cases. This feature is used when inserting and removing ControlWrapper objects. If the argument is a ControlWrapper the equals method checks whether both ControlWrappers are local. If so it checks whether their ControlImple fields are 'equal'. If either ControlWrapper is non-local then it has to invoke is_same_transaction on the associated Coordinators. The equals method in ControlWrapper also contains a check for the case that the argument is a Uid. If so it compares the supplied Uid argument to the wrapper's Uid. This is intended to catch the case where a Uid is supplied as argument to TransactionReaper.remove(). The reason this is failing is because the comparison is being done the wrong way round i.e. by calling Uid.equals(ControllWrapper) rather than ControlWrapper.equals(Uid). This looks to me like a failed attempt to optimize the remove operation. A correct but, possibly, slow implementation might be to change all calls to remove which supply a Uid and instead supply a ControlWrapper constructed using a Control available where the call is made. I will look for a better option before attempting to implement this fix. By the way, I only found two places where a non-Reapable is supplied, both in OTSManager.destroyControl. In one case the argument is a Uid, in the other a Control! I am not yet sure if the second case is correct or an error. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://jira.jboss.com/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira

[ http://jira.jboss.com/jira/browse/JBTM-352?page=comments#action_12405216 ] Andrew Dinn commented on JBTM-352: ---------------------------------- Well spotted. Unfortunately, this is probably not a valid fix. Essentially you are using the Uid as a key for entries in map _timeouts and this is inadequate for the general case. The reason for using Reapable objects as the key is that it allows the comparison to be defined by the Reapable -- this is possible because when Hashmap does a get or put with key 'k' it compares against existing entries using k.equals(entry.key). So, any Reapable can implement 'equals' to ensure that it compares correctly against other Reapables. If we switch to keying using Uids then this test will by bypassed. Your 'fix' may work with your test but it may break other cases. This feature is used when inserting and removing ControlWrapper objects. If the argument is a ControlWrapper the equals method checks whether both ControlWrappers are local. If so it checks whether their ControlImple fields are 'equal'. If either ControlWrapper is non-local then it has to invoke is_same_transaction on the associated Coordinators. The equals method in ControlWrapper also contains a check for the case that the argument is a Uid. If so it compares the supplied Uid argument to the wrapper's Uid. This is intended to catch the case where a Uid is supplied as argument to TransactionReaper.remove(). The reason this is failing is because the comparison is being done the wrong way round i.e. by calling Uid.equals(ControllWrapper) rather than ControlWrapper.equals(Uid). This looks to me like a failed attempt to optimize the remove operation. A correct but, possibly, slow implementation might be to change all calls to remove which supply a Uid and instead supply a ControlWrapper constructed using a Control available where the call is made. I will look for a better option before attempting to implement this fix. By the way, I only found two places where a non-Reapable is supplied, both in OTSManager.destroyControl. In one case the argument is a Uid, in the other a Control! I am not yet sure if the second case is correct or an error. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://jira.jboss.com/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira

[ http://jira.jboss.com/jira/browse/JBTM-352?page=all ] Andrew Dinn updated JBTM-352: ----------------------------- Comment: was deleted -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://jira.jboss.com/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira

[ http://jira.jboss.com/jira/browse/JBTM-352?page=all ] Andrew Dinn updated JBTM-352: ----------------------------- Comment: was deleted -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://jira.jboss.com/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira

[ http://jira.jboss.com/jira/browse/JBTM-352?page=comments#action_12405454 ] Andrew Dinn commented on JBTM-352: ---------------------------------- Yes, this will work but i) it misses another issue -- when the OTSManager only has a non-local Control it passes the Control to remove() which is bound not to work ii) it will still suck -- not only will the iteration take a long time it will also block concurrent transaction inserts/removes from other threads while it is running so kiss throughput goodbye I have a proper fix which I still need to test. I'll attach the 2 files you need if you want to try it. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://jira.jboss.com/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira

[ http://jira.jboss.com/jira/browse/JBTM-352?page=all ] Andrew Dinn updated JBTM-352: ----------------------------- Attachment: PseudoControlWrapper.java Wrapper class for use by OTSManager -- needs to go in the same source directory/package as ControlWrapper -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://jira.jboss.com/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira

[ http://jira.jboss.com/jira/browse/JBTM-352?page=all ] Andrew Dinn closed JBTM-352. ---------------------------- Resolution: Done Fixed this by adding a wrapper for OTS controls which compares correctly against the ControlWrapper held in the Reaper hash table. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://jira.jboss.com/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira