NUMBER_BLOCK_COMPRESSED, etc, shouldn't be treated as timer counter #3263
Conversation
…eated as timer counter Summary: NUMBER_BLOCK_DECOMPRESSED and NUMBER_BLOCK_COMPRESSED are not reported unless the stats level contain detailed timers, which is wrong. They are normal counters. Fix it. Test Plan: Verify the counters are reported using db_bench.
|
CC @mdcallag |
There was a problem hiding this comment.
@siying has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
table/format.cc
Outdated
| @@ -370,8 +370,8 @@ Status UncompressBlockContentsForCompressionType( | |||
| MeasureTime(ioptions.statistics, DECOMPRESSION_TIMES_NANOS, | |||
| timer.ElapsedNanos()); | |||
| MeasureTime(ioptions.statistics, BYTES_DECOMPRESSED, contents->data.size()); | |||
There was a problem hiding this comment.
MeasureTime is not actually measures time here. Should it also be put outside the if-then block?
There was a problem hiding this comment.
It's a histogram though, so expect regression:)
table/block_based_table_builder.cc
Outdated
| timer.ElapsedNanos()); | ||
| MeasureTime(r->ioptions.statistics, BYTES_COMPRESSED, | ||
| raw_block_contents.size()); | ||
| } | ||
| RecordTick(r->ioptions.statistics, NUMBER_BLOCK_COMPRESSED); | ||
| } | ||
|
|
There was a problem hiding this comment.
Can we add a stat to also measure the block_contents size? That would give the compression ratio stats that Mark was asking for.
There was a problem hiding this comment.
Add it by yourself if you want:)
I'm just doing a bug fixing.
There was a problem hiding this comment.
By the way, please note this is a histogram. I'm not sure how you can get compression ratio from a histogram and another counter.
|
@siying has updated the pull request. View: changes, changes since last import |
|
The Travis failure is unrelated. |
There was a problem hiding this comment.
@siying is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Summary: NUMBER_BLOCK_DECOMPRESSED and NUMBER_BLOCK_COMPRESSED are not reported unless the stats level contain detailed timers, which is wrong. They are normal counters. Fix it. Closes #3263 Differential Revision: D6552519 Pulled By: siying fbshipit-source-id: 40899ccea7b2856bb39752616657c0bfd432f6f9
Summary: NUMBER_BLOCK_DECOMPRESSED and NUMBER_BLOCK_COMPRESSED are not reported unless the stats level contain detailed timers, which is wrong. They are normal counters. Fix it.
Test Plan: Verify the counters are reported using db_bench.