cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Joel Knighton (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CASSANDRA-9692) Print sensible units for all log messages
Date Fri, 11 Mar 2016 22:47:29 GMT

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

Mime
View raw message