You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Mike Percy (Code Review)" <ge...@cloudera.org> on 2017/02/20 20:52:50 UTC

[kudu-CR] Use HumanReadableElapsedTime for MonoDelta::ToString()

Mike Percy has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/6082

Change subject: Use HumanReadableElapsedTime for MonoDelta::ToString()
......................................................................

Use HumanReadableElapsedTime for MonoDelta::ToString()

This allows small values in the microsecond range to be printed as
nonzero. This function chooses the highest time unit (microseconds,
milliseconds, seconds, minutes) that is nonzero and then returns the
stringified result rounded to 3 significant digits.

Change-Id: I787071d6ac62b1dc9154ec7b1f9a5d4a3ce64af1
---
M src/kudu/util/monotime.cc
1 file changed, 2 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/82/6082/1
-- 
To view, visit http://gerrit.cloudera.org:8080/6082
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I787071d6ac62b1dc9154ec7b1f9a5d4a3ce64af1
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>

[kudu-CR] Use HumanReadableElapsedTime for MonoDelta::ToString()

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/6082

to look at the new patch set (#2).

Change subject: Use HumanReadableElapsedTime for MonoDelta::ToString()
......................................................................

Use HumanReadableElapsedTime for MonoDelta::ToString()

This allows small values in the microsecond range to be printed as
nonzero. This function chooses the highest time unit (microseconds,
milliseconds, seconds, minutes) that is nonzero and then returns the
stringified result rounded to 3 significant digits.

For the following expressions:

  LOG(INFO) << MonoDelta::FromNanoseconds(1).ToString();
  LOG(INFO) << MonoDelta::FromNanoseconds(600).ToString();
  LOG(INFO) << MonoDelta::FromNanoseconds(1001).ToString();
  LOG(INFO) << MonoDelta::FromNanoseconds(1011).ToString();
  LOG(INFO) << MonoDelta::FromMicroseconds(1).ToString();
  LOG(INFO) << MonoDelta::FromMicroseconds(123).ToString();
  LOG(INFO) << MonoDelta::FromMicroseconds(1234).ToString();
  LOG(INFO) << MonoDelta::FromMicroseconds(12345).ToString();
  LOG(INFO) << MonoDelta::FromMilliseconds(1).ToString();
  LOG(INFO) << MonoDelta::FromSeconds(1).ToString();
  LOG(INFO) << MonoDelta::FromSeconds(1.1).ToString();

The old output was:

  I0220 13:11:14.794692 30062 monotime-test.cc:173] 0.000s
  I0220 13:11:14.794741 30062 monotime-test.cc:174] 0.000s
  I0220 13:11:14.794747 30062 monotime-test.cc:175] 0.000s
  I0220 13:11:14.794750 30062 monotime-test.cc:176] 0.000s
  I0220 13:11:14.794754 30062 monotime-test.cc:177] 0.000s
  I0220 13:11:14.794759 30062 monotime-test.cc:178] 0.000s
  I0220 13:11:14.794762 30062 monotime-test.cc:179] 0.001s
  I0220 13:11:14.794764 30062 monotime-test.cc:180] 0.012s
  I0220 13:11:14.794769 30062 monotime-test.cc:181] 0.001s
  I0220 13:11:14.794772 30062 monotime-test.cc:182] 1.000s
  I0220 13:11:14.794776 30062 monotime-test.cc:183] 1.100s

While the new output is:

  I0220 13:11:23.830267 30340 monotime-test.cc:173] 1 ns
  I0220 13:11:23.830339 30340 monotime-test.cc:174] 600 ns
  I0220 13:11:23.830346 30340 monotime-test.cc:175] 1 us
  I0220 13:11:23.830349 30340 monotime-test.cc:176] 1.01 us
  I0220 13:11:23.830353 30340 monotime-test.cc:177] 1 us
  I0220 13:11:23.830355 30340 monotime-test.cc:178] 123 us
  I0220 13:11:23.830358 30340 monotime-test.cc:179] 1.23 ms
  I0220 13:11:23.830360 30340 monotime-test.cc:180] 12.3 ms
  I0220 13:11:23.830363 30340 monotime-test.cc:181] 1 ms
  I0220 13:11:23.830368 30340 monotime-test.cc:182] 1 s
  I0220 13:11:23.830370 30340 monotime-test.cc:183] 1.1 s

Change-Id: I787071d6ac62b1dc9154ec7b1f9a5d4a3ce64af1
---
M src/kudu/util/monotime.cc
1 file changed, 2 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/82/6082/2
-- 
To view, visit http://gerrit.cloudera.org:8080/6082
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I787071d6ac62b1dc9154ec7b1f9a5d4a3ce64af1
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] Use HumanReadableElapsedTime for MonoDelta::ToString()

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.

Change subject: Use HumanReadableElapsedTime for MonoDelta::ToString()
......................................................................


Patch Set 4:

> I have mixed feelings about this. I agree it's easier for a human
 > to read the output after this change, but it's also harder to do
 > things like grep/sort/etc on log files which include values with
 > different units.
 > 
 > Where are we outputting monodeltas that you found this to be more
 > useful? Maybe we should have a new variant ToHumanReadableString()
 > or something?

I was mostly using this during development while debugging. I can see your argument and since I don't have a burning need for this I'll come back to it later.

-- 
To view, visit http://gerrit.cloudera.org:8080/6082
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I787071d6ac62b1dc9154ec7b1f9a5d4a3ce64af1
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] Use HumanReadableElapsedTime for MonoDelta::ToString()

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has abandoned this change.

Change subject: Use HumanReadableElapsedTime for MonoDelta::ToString()
......................................................................


Abandoned

-- 
To view, visit http://gerrit.cloudera.org:8080/6082
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: abandon
Gerrit-Change-Id: I787071d6ac62b1dc9154ec7b1f9a5d4a3ce64af1
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] Use HumanReadableElapsedTime for MonoDelta::ToString()

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.

Change subject: Use HumanReadableElapsedTime for MonoDelta::ToString()
......................................................................


Patch Set 4:

I have mixed feelings about this. I agree it's easier for a human to read the output after this change, but it's also harder to do things like grep/sort/etc on log files which include values with different units.

Where are we outputting monodeltas that you found this to be more useful? Maybe we should have a new variant ToHumanReadableString() or something?

-- 
To view, visit http://gerrit.cloudera.org:8080/6082
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I787071d6ac62b1dc9154ec7b1f9a5d4a3ce64af1
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No