[ https://issues.apache.org/jira/browse/CASSANDRA-13929?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16504704#comment-16504704 ]
Jason Brown commented on CASSANDRA-13929: ----------------------------------------- On the whole, this looks pretty good. I'm running tests locally now, and will run in circleci shortly. [~jay.zhuang] There's one thing I'm wondering: in {{#reuse{}}}, we do not explicitly null out the {{values}} array, like we used to do in {{#cleanup()}}: {code} Arrays.fill(values, null); {code} While every access of {{values}} seems safe, and uses the {{count}} to ensure proper sizing of any built {{Object[]}}, I'd like to be over-cautious and null out the array to prevent the possibility of data leak wherein a reuse of Builder erroneously gets extra data from the last use. This error doesn't seem like it's possible in the current code, but I'd like to future-proof this - I'd be a complete nightmare to debug. wdyt? > BTree$Builder / io.netty.util.Recycler$Stack leaking memory > ----------------------------------------------------------- > > Key: CASSANDRA-13929 > URL: https://issues.apache.org/jira/browse/CASSANDRA-13929 > Project: Cassandra > Issue Type: Bug > Components: Core > Reporter: Thomas Steinmaurer > Assignee: Jay Zhuang > Priority: Major > Fix For: 3.11.3 > > Attachments: cassandra_3.11.0_min_memory_utilization.jpg, > cassandra_3.11.1_NORECYCLE_memory_utilization.jpg, > cassandra_3.11.1_mat_dominator_classes.png, > cassandra_3.11.1_mat_dominator_classes_FIXED.png, > cassandra_3.11.1_snapshot_heaputilization.png, > cassandra_3.11.1_vs_3.11.2recyclernullingpatch.png, > cassandra_heapcpu_memleak_patching_test_30d.png, > dtest_example_80_request.png, dtest_example_80_request_fix.png, > dtest_example_heap.png, memleak_heapdump_recyclerstack.png > > > Different to CASSANDRA-13754, there seems to be another memory leak in > 3.11.0+ in BTree$Builder / io.netty.util.Recycler$Stack. > * heap utilization increase after upgrading to 3.11.0 => > cassandra_3.11.0_min_memory_utilization.jpg > * No difference after upgrading to 3.11.1 (snapshot build) => > cassandra_3.11.1_snapshot_heaputilization.png; thus most likely after fixing > CASSANDRA-13754, more visible now > * MAT shows io.netty.util.Recycler$Stack as top contributing class => > cassandra_3.11.1_mat_dominator_classes.png > * With -Xmx8G (CMS) and our load pattern, we have to do a rolling restart > after ~ 72 hours > Verified the following fix, namely explicitly unreferencing the > _recycleHandle_ member (making it non-final). In > _org.apache.cassandra.utils.btree.BTree.Builder.recycle()_ > {code} > public void recycle() > { > if (recycleHandle != null) > { > this.cleanup(); > builderRecycler.recycle(this, recycleHandle); > recycleHandle = null; // ADDED > } > } > {code} > Patched a single node in our loadtest cluster with this change and after ~ 10 > hours uptime, no sign of the previously offending class in MAT anymore => > cassandra_3.11.1_mat_dominator_classes_FIXED.png > Can' say if this has any other side effects etc., but I doubt. -- This message was sent by Atlassian JIRA (v7.6.3#76005) --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org