[jira] [Commented] (CASSANDRA-15773) Add test to cover metrics related to the BufferPool
[ https://issues.apache.org/jira/browse/CASSANDRA-15773?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17129029#comment-17129029 ] Benjamin Lerer commented on CASSANDRA-15773: The patch looks good to me. :-) > Add test to cover metrics related to the BufferPool > --- > > Key: CASSANDRA-15773 > URL: https://issues.apache.org/jira/browse/CASSANDRA-15773 > Project: Cassandra > Issue Type: Improvement > Components: Test/unit >Reporter: Stephen Mallette >Assignee: Stephen Mallette >Priority: Normal > > At this time there do not appear to be unit tests to validate > {{BufferPoolMetrics}}. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-15773) Add test to cover metrics related to the BufferPool
[ https://issues.apache.org/jira/browse/CASSANDRA-15773?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17128342#comment-17128342 ] Benjamin Lerer commented on CASSANDRA-15773: [~djoshi] I can take the review if you do not have time for it. > Add test to cover metrics related to the BufferPool > --- > > Key: CASSANDRA-15773 > URL: https://issues.apache.org/jira/browse/CASSANDRA-15773 > Project: Cassandra > Issue Type: Improvement > Components: Test/unit >Reporter: Stephen Mallette >Assignee: Stephen Mallette >Priority: Normal > > At this time there do not appear to be unit tests to validate > {{BufferPoolMetrics}}. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-15773) Add test to cover metrics related to the BufferPool
[ https://issues.apache.org/jira/browse/CASSANDRA-15773?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17111412#comment-17111412 ] David Capwell commented on CASSANDRA-15773: --- [~djoshi] can we get your eyes? > Add test to cover metrics related to the BufferPool > --- > > Key: CASSANDRA-15773 > URL: https://issues.apache.org/jira/browse/CASSANDRA-15773 > Project: Cassandra > Issue Type: Improvement > Components: Test/unit >Reporter: Stephen Mallette >Assignee: Stephen Mallette >Priority: Normal > > At this time there do not appear to be unit tests to validate > {{BufferPoolMetrics}}. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-15773) Add test to cover metrics related to the BufferPool
[ https://issues.apache.org/jira/browse/CASSANDRA-15773?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=1747#comment-1747 ] Stephen Mallette commented on CASSANDRA-15773: -- I was just wondering if anything else needed here on this one for it to be merged. Thanks. > Add test to cover metrics related to the BufferPool > --- > > Key: CASSANDRA-15773 > URL: https://issues.apache.org/jira/browse/CASSANDRA-15773 > Project: Cassandra > Issue Type: Improvement > Components: Test/unit >Reporter: Stephen Mallette >Assignee: Stephen Mallette >Priority: Normal > > At this time there do not appear to be unit tests to validate > {{BufferPoolMetrics}}. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-15773) Add test to cover metrics related to the BufferPool
[ https://issues.apache.org/jira/browse/CASSANDRA-15773?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17101951#comment-17101951 ] David Capwell commented on CASSANDRA-15773: --- +1 from me. I ran the build and the only failures were the View/Checkum tests (all known flaky tests) > Add test to cover metrics related to the BufferPool > --- > > Key: CASSANDRA-15773 > URL: https://issues.apache.org/jira/browse/CASSANDRA-15773 > Project: Cassandra > Issue Type: Improvement > Components: Test/unit >Reporter: Stephen Mallette >Assignee: Stephen Mallette >Priority: Normal > > At this time there do not appear to be unit tests to validate > {{BufferPoolMetrics}}. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-15773) Add test to cover metrics related to the BufferPool
[ https://issues.apache.org/jira/browse/CASSANDRA-15773?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17101895#comment-17101895 ] Stephen Mallette commented on CASSANDRA-15773: -- {{assertThatThrownBy()}} is better - should have thought of that. done > Add test to cover metrics related to the BufferPool > --- > > Key: CASSANDRA-15773 > URL: https://issues.apache.org/jira/browse/CASSANDRA-15773 > Project: Cassandra > Issue Type: Improvement > Components: Test/unit >Reporter: Stephen Mallette >Assignee: Stephen Mallette >Priority: Normal > > At this time there do not appear to be unit tests to validate > {{BufferPoolMetrics}}. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-15773) Add test to cover metrics related to the BufferPool
[ https://issues.apache.org/jira/browse/CASSANDRA-15773?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17101888#comment-17101888 ] David Capwell commented on CASSANDRA-15773: --- bq. I've initialized the test with a fixed size to the pool Sorry, must have missed that. bq. I might not understand something but I thought that this would prevent such problems: I most likely didn't look close enough, feel free to ignore me with regard to leaking. Thanks for the new failure tests! * https://github.com/apache/cassandra/compare/trunk...spmallette:CASSANDRA-15773-trunk#diff-790a43fcd6ed69f4122cd0fe205c847bR187-R198. You could use org.assertj.core.api.Assertions#assertThatThrownBy(org.assertj.core.api.ThrowableAssert.ThrowingCallable). Other than that, this patch LGTM so I am +1. Ill run it through CI to make sure its stable > Add test to cover metrics related to the BufferPool > --- > > Key: CASSANDRA-15773 > URL: https://issues.apache.org/jira/browse/CASSANDRA-15773 > Project: Cassandra > Issue Type: Improvement > Components: Test/unit >Reporter: Stephen Mallette >Assignee: Stephen Mallette >Priority: Normal > > At this time there do not appear to be unit tests to validate > {{BufferPoolMetrics}}. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-15773) Add test to cover metrics related to the BufferPool
[ https://issues.apache.org/jira/browse/CASSANDRA-15773?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17101742#comment-17101742 ] Stephen Mallette commented on CASSANDRA-15773: -- thanks [~djoshi] > You're making gets of the same size. I feel randomizing the size of requested > buffers would be better > We should add negative test cases i.e. request buffers with bad sizes I've made some adjustment there. Please let me know if that's in line with what you were thinking. > Add test to cover metrics related to the BufferPool > --- > > Key: CASSANDRA-15773 > URL: https://issues.apache.org/jira/browse/CASSANDRA-15773 > Project: Cassandra > Issue Type: Improvement > Components: Test/unit >Reporter: Stephen Mallette >Assignee: Stephen Mallette >Priority: Normal > > At this time there do not appear to be unit tests to validate > {{BufferPoolMetrics}}. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-15773) Add test to cover metrics related to the BufferPool
[ https://issues.apache.org/jira/browse/CASSANDRA-15773?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17101123#comment-17101123 ] Dinesh Joshi commented on CASSANDRA-15773: -- [~spmallette] I think the test is ok. I don't think you're not leaking memory ([~dcapwell] please correct me if I am missing something here). A few things to think about - # You're making gets of the same size. I feel randomizing the size of requested buffers would be better # We should add negative test cases i.e. request buffers with bad sizes > Add test to cover metrics related to the BufferPool > --- > > Key: CASSANDRA-15773 > URL: https://issues.apache.org/jira/browse/CASSANDRA-15773 > Project: Cassandra > Issue Type: Improvement > Components: Test/unit >Reporter: Stephen Mallette >Assignee: Stephen Mallette >Priority: Normal > > At this time there do not appear to be unit tests to validate > {{BufferPoolMetrics}}. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-15773) Add test to cover metrics related to the BufferPool
[ https://issues.apache.org/jira/browse/CASSANDRA-15773?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17100695#comment-17100695 ] Stephen Mallette commented on CASSANDRA-15773: -- [~dcapwell] thanks for the feedback: > Since we don't free, and the metric is how many chunks are leant out, >= > totalBytesRequestedFromPool should hold true and would be more accurate than > > 0 yes - that sounds smart to me - I've made that change. > the test makes an assumption at buffer pool size, which may not be true in CI, I've initialized the test with a fixed size to the pool: https://github.com/apache/cassandra/compare/trunk...spmallette:CASSANDRA-15773-trunk#diff-790a43fcd6ed69f4122cd0fe205c847bR48-R52 > leaking, should cleanup after the test is over; same for the other test I might not understand something but I thought that this would prevent such problems: https://github.com/apache/cassandra/compare/trunk...spmallette:CASSANDRA-15773-trunk#diff-790a43fcd6ed69f4122cd0fe205c847bR54-R58 which effectively calls: {code} static void unsafeReset() { localPool.get().unsafeRecycle(); globalPool.unsafeFree(); } {code} on {{BufferPool}} itself. Do I need to do something more? > Add test to cover metrics related to the BufferPool > --- > > Key: CASSANDRA-15773 > URL: https://issues.apache.org/jira/browse/CASSANDRA-15773 > Project: Cassandra > Issue Type: Improvement > Components: Test/unit >Reporter: Stephen Mallette >Assignee: Stephen Mallette >Priority: Normal > > At this time there do not appear to be unit tests to validate > {{BufferPoolMetrics}}. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-15773) Add test to cover metrics related to the BufferPool
[ https://issues.apache.org/jira/browse/CASSANDRA-15773?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17100342#comment-17100342 ] David Capwell commented on CASSANDRA-15773: --- * https://github.com/apache/cassandra/compare/trunk...spmallette:CASSANDRA-15773-trunk#diff-790a43fcd6ed69f4122cd0fe205c847bR72. leaking, should cleanup after the test is over; same for the other test * https://github.com/apache/cassandra/compare/trunk...spmallette:CASSANDRA-15773-trunk#diff-790a43fcd6ed69f4122cd0fe205c847bR75 can this be >= totalBytesRequestedFromPool? Since we don't free, and the metric is how many chunks are leant out, >= totalBytesRequestedFromPool should hold true and would be more accurate than > 0 * the test makes an assumption at buffer pool size, which may not be true in CI, see [1] pool size calculation. For this reason the tests can be flaky under environments with less resources. One way to deal with this is to define file_cache_size_in_mb in test/conf/cassandra.yaml, that way memory is constant for all environments, and the loops can be adjusted to it. Overall the patch LGTM, mostly small stuff. [1] - {code} if (conf.file_cache_size_in_mb == null) conf.file_cache_size_in_mb = Math.min(512, (int) (Runtime.getRuntime().maxMemory() / (4 * 1048576))); {code} > Add test to cover metrics related to the BufferPool > --- > > Key: CASSANDRA-15773 > URL: https://issues.apache.org/jira/browse/CASSANDRA-15773 > Project: Cassandra > Issue Type: Improvement > Components: Test/unit >Reporter: Stephen Mallette >Assignee: Stephen Mallette >Priority: Normal > > At this time there do not appear to be unit tests to validate > {{BufferPoolMetrics}}. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-15773) Add test to cover metrics related to the BufferPool
[ https://issues.apache.org/jira/browse/CASSANDRA-15773?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17095704#comment-17095704 ] Dinesh Joshi commented on CASSANDRA-15773: -- thanks for the patch [~spmallette]. I've kicked off CI [here|https://circleci.com/workflow-run/fa06016a-fb74-455c-8bc0-4ee96034c3dd]. > Add test to cover metrics related to the BufferPool > --- > > Key: CASSANDRA-15773 > URL: https://issues.apache.org/jira/browse/CASSANDRA-15773 > Project: Cassandra > Issue Type: Improvement > Components: Test/unit >Reporter: Stephen Mallette >Assignee: Stephen Mallette >Priority: Normal > > At this time there do not appear to be unit tests to validate > {{BufferPoolMetrics}}. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-15773) Add test to cover metrics related to the BufferPool
[ https://issues.apache.org/jira/browse/CASSANDRA-15773?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17095691#comment-17095691 ] David Capwell commented on CASSANDRA-15773: --- have a few things on my plate this week, if anyone else wants to review first go ahead, else ill try to pick up Friday/Monday > Add test to cover metrics related to the BufferPool > --- > > Key: CASSANDRA-15773 > URL: https://issues.apache.org/jira/browse/CASSANDRA-15773 > Project: Cassandra > Issue Type: Improvement > Components: Test/unit >Reporter: Stephen Mallette >Assignee: Stephen Mallette >Priority: Normal > > At this time there do not appear to be unit tests to validate > {{BufferPoolMetrics}}. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-15773) Add test to cover metrics related to the BufferPool
[ https://issues.apache.org/jira/browse/CASSANDRA-15773?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17095572#comment-17095572 ] Stephen Mallette commented on CASSANDRA-15773: -- Please find this new test here for review: https://github.com/apache/cassandra/compare/trunk...spmallette:CASSANDRA-15773-trunk > Add test to cover metrics related to the BufferPool > --- > > Key: CASSANDRA-15773 > URL: https://issues.apache.org/jira/browse/CASSANDRA-15773 > Project: Cassandra > Issue Type: Improvement > Components: Test/unit >Reporter: Stephen Mallette >Assignee: Stephen Mallette >Priority: Normal > > At this time there do not appear to be unit tests to validate > {{BufferPoolMetrics}}. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org