[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=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=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=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=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=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=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=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=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=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=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=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=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=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=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
