You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2020/02/12 21:27:55 UTC

[kudu-CR] [hybrid clock] KUDU-3048 introduce new clock metrics

Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15212


Change subject: [hybrid clock] KUDU-3048 introduce new clock metrics
......................................................................

[hybrid clock] KUDU-3048 introduce new clock metrics

Introduced additional metrics for the hybrid clock:
  * number of intervals when the underlying clock was extrapolated
    instead of using the actual readings
  * the longest interval when when the underlying clock was extrapolated
    instead of using the actual readings
  * the maximum of the max_error ever sampled while reading the
    underlying clock

I ran a small Kudu cluster to manually verify the behavior of the
newly introduced metrics: I'm not sure it's worth adding automated
tests for this given the already existing 'hybrid_clock_error' metric
didn't have any test coverage.

Change-Id: I8575ba7d8baed78b13351e8cebf1a74f44b31b82
---
M src/kudu/clock/hybrid_clock.cc
M src/kudu/clock/hybrid_clock.h
M src/kudu/server/server_base.cc
3 files changed, 69 insertions(+), 17 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8575ba7d8baed78b13351e8cebf1a74f44b31b82
Gerrit-Change-Number: 15212
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>

[kudu-CR] [hybrid clock] KUDU-3048 introduce new clock metrics

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15212 )

Change subject: [hybrid clock] KUDU-3048 introduce new clock metrics
......................................................................


Patch Set 5: Verified+1

TSAN warnings in unrelated tests (known issues).


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8575ba7d8baed78b13351e8cebf1a74f44b31b82
Gerrit-Change-Number: 15212
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 14 Feb 2020 05:21:23 +0000
Gerrit-HasComments: No

[kudu-CR] [hybrid clock] KUDU-3048 introduce new clock metrics

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15212 )

Change subject: [hybrid clock] KUDU-3048 introduce new clock metrics
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/15212/2/src/kudu/clock/hybrid_clock.h
File src/kudu/clock/hybrid_clock.h:

http://gerrit.cloudera.org:8080/#/c/15212/2/src/kudu/clock/hybrid_clock.h@211
PS2, Line 211: happenning
> happening
Done


http://gerrit.cloudera.org:8080/#/c/15212/2/src/kudu/clock/hybrid_clock.h@213
PS2, Line 213: interpolation
> Did you mean extrapolation here?
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8575ba7d8baed78b13351e8cebf1a74f44b31b82
Gerrit-Change-Number: 15212
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 12 Feb 2020 23:54:22 +0000
Gerrit-HasComments: Yes

[kudu-CR] [hybrid clock] KUDU-3048 introduce new clock metrics

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has removed a vote on this change.

Change subject: [hybrid clock] KUDU-3048 introduce new clock metrics
......................................................................


Removed Verified-1 by Kudu Jenkins (120)
-- 
To view, visit http://gerrit.cloudera.org:8080/15212
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I8575ba7d8baed78b13351e8cebf1a74f44b31b82
Gerrit-Change-Number: 15212
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>

[kudu-CR] [hybrid clock] KUDU-3048 introduce new clock metrics

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15212 )

Change subject: [hybrid clock] KUDU-3048 introduce new clock metrics
......................................................................


Patch Set 5: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8575ba7d8baed78b13351e8cebf1a74f44b31b82
Gerrit-Change-Number: 15212
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 14 Feb 2020 01:50:28 +0000
Gerrit-HasComments: No

[kudu-CR] [hybrid clock] KUDU-3048 introduce new clock metrics

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15212 )

Change subject: [hybrid clock] KUDU-3048 introduce new clock metrics
......................................................................

[hybrid clock] KUDU-3048 introduce new clock metrics

Introduced additional metrics for the hybrid clock:
  * whether hybrid clock is using extrapolated readings for the
    underlying clock instead of actual readings
  * histogram for the duration of intervals when the underlying clock
    was extrapolated
  * histogram for the maximum errors reported by the underlying clock

I ran a small Kudu cluster to manually verify the behavior of the
newly introduced metrics: I'm not sure it's worth adding automated
tests for this given the already existing 'hybrid_clock_error' metric
didn't have any test coverage.

Change-Id: I8575ba7d8baed78b13351e8cebf1a74f44b31b82
Reviewed-on: http://gerrit.cloudera.org:8080/15212
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
M src/kudu/clock/hybrid_clock.cc
M src/kudu/clock/hybrid_clock.h
M src/kudu/server/server_base.cc
M src/kudu/util/metrics.cc
4 files changed, 75 insertions(+), 17 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Alexey Serbin: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I8575ba7d8baed78b13351e8cebf1a74f44b31b82
Gerrit-Change-Number: 15212
Gerrit-PatchSet: 8
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>

[kudu-CR] [hybrid clock] KUDU-3048 introduce new clock metrics

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15212 )

Change subject: [hybrid clock] KUDU-3048 introduce new clock metrics
......................................................................


Patch Set 3:

> Build Failed
 > 
 > http://jenkins.kudu.apache.org/job/kudu-gerrit/20432/ : FAILURE

whoops, the DCHECK() I added has triggered -- I need to clarify what's going on


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8575ba7d8baed78b13351e8cebf1a74f44b31b82
Gerrit-Change-Number: 15212
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 12 Feb 2020 23:55:36 +0000
Gerrit-HasComments: No

[kudu-CR] [hybrid clock] KUDU-3048 introduce new clock metrics

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15212 )

Change subject: [hybrid clock] KUDU-3048 introduce new clock metrics
......................................................................


Patch Set 7: Code-Review+2

Carrying over Adar's +2 from PS5.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8575ba7d8baed78b13351e8cebf1a74f44b31b82
Gerrit-Change-Number: 15212
Gerrit-PatchSet: 7
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 14 Feb 2020 21:34:16 +0000
Gerrit-HasComments: No

[kudu-CR] [hybrid clock] KUDU-3048 introduce new clock metrics

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo, 

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

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

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

Change subject: [hybrid clock] KUDU-3048 introduce new clock metrics
......................................................................

[hybrid clock] KUDU-3048 introduce new clock metrics

Introduced additional metrics for the hybrid clock:
  * whether hybrid clock is using extrapolated readings for the
    underlying clock instead of actual readings
  * histogram for the duration of intervals when the underlying clock
    was extrapolated
  * histogram for the maximum errors reported by the underlying clock

I ran a small Kudu cluster to manually verify the behavior of the
newly introduced metrics: I'm not sure it's worth adding automated
tests for this given the already existing 'hybrid_clock_error' metric
didn't have any test coverage.

Change-Id: I8575ba7d8baed78b13351e8cebf1a74f44b31b82
---
M src/kudu/clock/hybrid_clock.cc
M src/kudu/clock/hybrid_clock.h
M src/kudu/server/server_base.cc
3 files changed, 74 insertions(+), 16 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/12/15212/4
-- 
To view, visit http://gerrit.cloudera.org:8080/15212
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8575ba7d8baed78b13351e8cebf1a74f44b31b82
Gerrit-Change-Number: 15212
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] [hybrid clock] KUDU-3048 introduce new clock metrics

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15212 )

Change subject: [hybrid clock] KUDU-3048 introduce new clock metrics
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/15212/1/src/kudu/clock/hybrid_clock.cc
File src/kudu/clock/hybrid_clock.cc:

http://gerrit.cloudera.org:8080/#/c/15212/1/src/kudu/clock/hybrid_clock.cc@131
PS1, Line 131: METRIC_DEFINE_gauge_uint64(server, hybrid_clock_max_error,
> Likewise, if you use a histogram you'll get min, avg, and percentiles too.
Done


http://gerrit.cloudera.org:8080/#/c/15212/1/src/kudu/clock/hybrid_clock.cc@136
PS1, Line 136: METRIC_DEFINE_gauge_int64(server, hybrid_clock_longest_extrapolation_interval,
             :                           "Longest Interval of Hybrid Clock Extrapolation",
             :                           kudu::MetricUnit::kMicroseconds,
             :                           "Longest interval when the underlying clock was "
             :                           "extrapolated instead of directly reading it",
             :                           kudu::MetricLevel::kWarn);
> This and extrapolation_intervals seem like a great fit for a single histogr
Great idea.  Done.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8575ba7d8baed78b13351e8cebf1a74f44b31b82
Gerrit-Change-Number: 15212
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 12 Feb 2020 23:15:51 +0000
Gerrit-HasComments: Yes

[kudu-CR] [hybrid clock] KUDU-3048 introduce new clock metrics

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15212 )

Change subject: [hybrid clock] KUDU-3048 introduce new clock metrics
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/15212/2/src/kudu/clock/hybrid_clock.h
File src/kudu/clock/hybrid_clock.h:

http://gerrit.cloudera.org:8080/#/c/15212/2/src/kudu/clock/hybrid_clock.h@211
PS2, Line 211: happenning
happening


http://gerrit.cloudera.org:8080/#/c/15212/2/src/kudu/clock/hybrid_clock.h@213
PS2, Line 213: interpolation
Did you mean extrapolation here?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8575ba7d8baed78b13351e8cebf1a74f44b31b82
Gerrit-Change-Number: 15212
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 12 Feb 2020 23:47:34 +0000
Gerrit-HasComments: Yes

[kudu-CR] [hybrid clock] KUDU-3048 introduce new clock metrics

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo, 

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

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

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

Change subject: [hybrid clock] KUDU-3048 introduce new clock metrics
......................................................................

[hybrid clock] KUDU-3048 introduce new clock metrics

Introduced additional metrics for the hybrid clock:
  * whether hybrid clock is using extrapolated readings for the
    underlying clock instead of actual readings
  * histogram for the duration of intervals when the underlying clock
    was extrapolated
  * histogram for the maximum errors reported by the underlying clock

I ran a small Kudu cluster to manually verify the behavior of the
newly introduced metrics: I'm not sure it's worth adding automated
tests for this given the already existing 'hybrid_clock_error' metric
didn't have any test coverage.

Change-Id: I8575ba7d8baed78b13351e8cebf1a74f44b31b82
---
M src/kudu/clock/hybrid_clock.cc
M src/kudu/clock/hybrid_clock.h
M src/kudu/server/server_base.cc
3 files changed, 78 insertions(+), 18 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8575ba7d8baed78b13351e8cebf1a74f44b31b82
Gerrit-Change-Number: 15212
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] [hybrid clock] KUDU-3048 introduce new clock metrics

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo, 

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

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

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

Change subject: [hybrid clock] KUDU-3048 introduce new clock metrics
......................................................................

[hybrid clock] KUDU-3048 introduce new clock metrics

Introduced additional metrics for the hybrid clock:
  * whether hybrid clock is using extrapolated readings for the
    underlying clock instead of actual readings
  * histogram for the duration of intervals when the underlying clock
    was extrapolated
  * histogram for the maximum errors reported by the underlying clock

I ran a small Kudu cluster to manually verify the behavior of the
newly introduced metrics: I'm not sure it's worth adding automated
tests for this given the already existing 'hybrid_clock_error' metric
didn't have any test coverage.

Change-Id: I8575ba7d8baed78b13351e8cebf1a74f44b31b82
---
M src/kudu/clock/hybrid_clock.cc
M src/kudu/clock/hybrid_clock.h
M src/kudu/server/server_base.cc
M src/kudu/util/metrics.cc
4 files changed, 75 insertions(+), 17 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/12/15212/5
-- 
To view, visit http://gerrit.cloudera.org:8080/15212
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8575ba7d8baed78b13351e8cebf1a74f44b31b82
Gerrit-Change-Number: 15212
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] [hybrid clock] KUDU-3048 introduce new clock metrics

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15212 )

Change subject: [hybrid clock] KUDU-3048 introduce new clock metrics
......................................................................


Patch Set 4:

> Build Failed
 > 
 > http://jenkins.kudu.apache.org/job/kudu-gerrit/20441/ : FAILURE

There is an issue with RaftConsensusITest.TestEarlyCommitDespiteMemoryPressure, I need to take a look.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8575ba7d8baed78b13351e8cebf1a74f44b31b82
Gerrit-Change-Number: 15212
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 13 Feb 2020 23:00:37 +0000
Gerrit-HasComments: No

[kudu-CR] [hybrid clock] KUDU-3048 introduce new clock metrics

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo, 

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

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

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

Change subject: [hybrid clock] KUDU-3048 introduce new clock metrics
......................................................................

[hybrid clock] KUDU-3048 introduce new clock metrics

Introduced additional metrics for the hybrid clock:
  * whether hybrid clock is using extrapolated readings for the
    underlying clock instead of actual readings
  * histogram for the duration of intervals when the underlying clock
    was extrapolated
  * histogram for the maximum errors reported by the underlying clock

I ran a small Kudu cluster to manually verify the behavior of the
newly introduced metrics: I'm not sure it's worth adding automated
tests for this given the already existing 'hybrid_clock_error' metric
didn't have any test coverage.

Change-Id: I8575ba7d8baed78b13351e8cebf1a74f44b31b82
---
M src/kudu/clock/hybrid_clock.cc
M src/kudu/clock/hybrid_clock.h
M src/kudu/server/server_base.cc
3 files changed, 78 insertions(+), 18 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/12/15212/3
-- 
To view, visit http://gerrit.cloudera.org:8080/15212
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8575ba7d8baed78b13351e8cebf1a74f44b31b82
Gerrit-Change-Number: 15212
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] [hybrid clock] KUDU-3048 introduce new clock metrics

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15212 )

Change subject: [hybrid clock] KUDU-3048 introduce new clock metrics
......................................................................


Patch Set 6: Code-Review+2

Carrying over Adar's +2 from PS5.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8575ba7d8baed78b13351e8cebf1a74f44b31b82
Gerrit-Change-Number: 15212
Gerrit-PatchSet: 6
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 14 Feb 2020 19:54:13 +0000
Gerrit-HasComments: No

[kudu-CR] [hybrid clock] KUDU-3048 introduce new clock metrics

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15212 )

Change subject: [hybrid clock] KUDU-3048 introduce new clock metrics
......................................................................


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/15212/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/15212/1//COMMIT_MSG@12
PS1, Line 12:  when when
double when


http://gerrit.cloudera.org:8080/#/c/15212/1/src/kudu/clock/hybrid_clock.cc
File src/kudu/clock/hybrid_clock.cc:

http://gerrit.cloudera.org:8080/#/c/15212/1/src/kudu/clock/hybrid_clock.cc@131
PS1, Line 131: METRIC_DEFINE_gauge_uint64(server, hybrid_clock_max_error,
Likewise, if you use a histogram you'll get min, avg, and percentiles too.


http://gerrit.cloudera.org:8080/#/c/15212/1/src/kudu/clock/hybrid_clock.cc@136
PS1, Line 136: METRIC_DEFINE_gauge_int64(server, hybrid_clock_longest_extrapolation_interval,
             :                           "Longest Interval of Hybrid Clock Extrapolation",
             :                           kudu::MetricUnit::kMicroseconds,
             :                           "Longest interval when the underlying clock was "
             :                           "extrapolated instead of directly reading it",
             :                           kudu::MetricLevel::kWarn);
This and extrapolation_intervals seem like a great fit for a single histogram metric.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8575ba7d8baed78b13351e8cebf1a74f44b31b82
Gerrit-Change-Number: 15212
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 12 Feb 2020 21:32:33 +0000
Gerrit-HasComments: Yes