[
https://issues.apache.org/jira/browse/CASSANDRA-9692?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15191630#comment-15191630
]
Joel Knighton commented on CASSANDRA-9692:
------------------------------------------
Thanks [~giampaolo]! This looks good overall and very thorough.
A few comments to fix up before we get this merged:
* In CompactionManager, we shouldn't pretty print the expected bloom filter
size. This size is in entries, not bytes.
* Is there a strong reason to change the IOException to a RuntimeException in
RangeAwareSSTableWriter? I'd undo that change.
* There's a small typo in the format string for "Insufficient disk space" in
StreamReader.
* A parenthesis is missing in IndexSummaryRedistribution.
* A comma is missing in the argument list in BufferPool.
* In BulkLoader, we need to append a parenthesis at the end of (avg: ). We
should also add a space in front. The change to the padding of the percentage
fields is unnecessary, and we also don't need to pad the rate in seconds.
The details of these suggested tweaks are here: [review
comments|https://github.com/jkni/cassandra/commit/290adc8d1f1f1644e5a886dace857430a0f2f958].
There are a few other changes I'd suggest but haven't done:
* The prettyPrintRateInSeconds method seems useful - I think we should move it
to FBUtilities like prettyPrintMemory. Renaming the argument from size to rate
would make more sense to me.
* It seems like we should have consistency in printing between
prettyPrintRateInSeconds (X.XX YiB/s) and prettyPrintMemory (X.XXXYiB). I'd
prefer the format of prettyPrintMemory, where we add another digit and remove
the space.
* In CompactionTask, we may want to convert the rate in {{runMayThrow}} and the
total bytes compacted in {{runMayThrow}}.
Rebasing on trunk before pushing a patch addressing these concerns would be
great. Once we have a patch we agree on, I'll run it in CI so we can confirm
that this doesn't break any tests.
> Print sensible units for all log messages
> -----------------------------------------
>
> Key: CASSANDRA-9692
> URL: https://issues.apache.org/jira/browse/CASSANDRA-9692
> Project: Cassandra
> Issue Type: Improvement
> Reporter: Benedict
> Assignee: Giampaolo
> Priority: Minor
> Labels: lhf
> Fix For: 3.x
>
> Attachments:
> Cassandra9692-trunk-giampaolo-trapasso-at-radicalbit-io.diff
>
>
> Like CASSANDRA-9691, this has bugged me too long. it also adversely impacts
> log analysis. I've introduced some improvements to the bits I touched for
> CASSANDRA-9681, but we should do this across the codebase. It's a small
> investment for a lot of long term clarity in the logs.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
