You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Yifan Zhang (Code Review)" <ge...@cloudera.org> on 2019/10/17 13:30:30 UTC
[kudu-CR] WIP: fix wrong value of live row count
Yifan Zhang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14484
Change subject: WIP: fix wrong value of live_row_count
......................................................................
WIP: fix wrong value of live_row_count
When some tablet of a table doesn't support live row counting,
but the other tablets support, e.g. add range partition for a
table after upgrading the cluster. In general, the table still
dosen't support live row counting, so the 'live_row_count'
metric of the table should be set '-1'.
*need to add tests.
*need to fix merge metric bug.
Change-Id: I757978cb496d47306f91ea251cba00bde7929e36
---
M src/kudu/master/catalog_manager.cc
M src/kudu/master/table_metrics.cc
2 files changed, 3 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/14484/1
--
To view, visit http://gerrit.cloudera.org:8080/14484
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I757978cb496d47306f91ea251cba00bde7929e36
Gerrit-Change-Number: 14484
Gerrit-PatchSet: 1
Gerrit-Owner: Yifan Zhang <ch...@163.com>
[kudu-CR] WIP: fix wrong value of live row count
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/14484 )
Change subject: WIP: fix wrong value of live_row_count
......................................................................
Patch Set 1:
> Patch Set 1:
>
> (1 comment)
I'll try to fix the 2nd problem for general metrics merge.
--
To view, visit http://gerrit.cloudera.org:8080/14484
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I757978cb496d47306f91ea251cba00bde7929e36
Gerrit-Change-Number: 14484
Gerrit-PatchSet: 1
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Fri, 18 Oct 2019 03:24:58 +0000
Gerrit-HasComments: No
[kudu-CR] WIP: fix wrong value of live row count
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has abandoned this change. ( http://gerrit.cloudera.org:8080/14484 )
Change subject: WIP: fix wrong value of live_row_count
......................................................................
Abandoned
--
To view, visit http://gerrit.cloudera.org:8080/14484
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I757978cb496d47306f91ea251cba00bde7929e36
Gerrit-Change-Number: 14484
Gerrit-PatchSet: 1
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
[kudu-CR] WIP: fix wrong value of live row count
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14484 )
Change subject: WIP: fix wrong value of live_row_count
......................................................................
Patch Set 1:
> There are sevral palce need fix:
> 1. tablet metric: METRIC_live_row_count instantiate with a 'invisible' status instead of -1
> 2. tserver heartbeat to master: use a separate boolean to indicate that this tablet not support live_row_count
> 3. metrics merge: if any one of metric is 'invisible' when merge two metrics, the result should be 'invisible' too.
Unfortunately InstantiateHidden doesn't work for FunctionGauges, which is what METRIC_live_row_count is. So we can't hide them using that mechanism.
FunctionGauge(const GaugePrototype<T>* proto, Callback<T()> function)
: Gauge(proto), function_(std::move(function)) {
// Override the modification epoch to the maximum, since we don't have any idea
// when the bound function changes value.
m_epoch_ = std::numeric_limits<decltype(m_epoch_.load())>::max();
}
--
To view, visit http://gerrit.cloudera.org:8080/14484
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I757978cb496d47306f91ea251cba00bde7929e36
Gerrit-Change-Number: 14484
Gerrit-PatchSet: 1
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Fri, 18 Oct 2019 04:43:20 +0000
Gerrit-HasComments: No
[kudu-CR] WIP: fix wrong value of live row count
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/14484 )
Change subject: WIP: fix wrong value of live_row_count
......................................................................
Patch Set 1:
> Patch Set 1:
>
> (1 comment)
>
> > Patch Set 1:
> >
> > (1 comment)
There are sevral palce need fix:
1. tablet metric: METRIC_live_row_count instantiate with a 'invisible' status instead of -1
2. tserver heartbeat to master: use a separate boolean to indicate that this tablet not support live_row_count
3. metrics merge: if any one of metric is 'invisible' when merge two metrics, the result should be 'invisible' too.
--
To view, visit http://gerrit.cloudera.org:8080/14484
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I757978cb496d47306f91ea251cba00bde7929e36
Gerrit-Change-Number: 14484
Gerrit-PatchSet: 1
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Fri, 18 Oct 2019 03:54:12 +0000
Gerrit-HasComments: No
[kudu-CR] WIP: fix wrong value of live row count
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14484 )
Change subject: WIP: fix wrong value of live_row_count
......................................................................
Patch Set 1:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/14484/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14484/1//COMMIT_MSG@15
PS1, Line 15: *need to add tests.
Yes. We should have added one for the last fix too.
http://gerrit.cloudera.org:8080/#/c/14484/1//COMMIT_MSG@16
PS1, Line 16: *need to fix merge metric bug.
What is this alluding to?
http://gerrit.cloudera.org:8080/#/c/14484/1/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/14484/1/src/kudu/master/catalog_manager.cc@5580
PS1, Line 5580: // Some tablet of the table dosen't support live row counting.
"This tablet doesn't support live row counting, so we disable it for the entire table".
http://gerrit.cloudera.org:8080/#/c/14484/1/src/kudu/master/table_metrics.cc
File src/kudu/master/table_metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14484/1/src/kudu/master/table_metrics.cc@31
PS1, Line 31: "When some tablet of the table doesn't support live row counting, -1 will be returned.");
Rephrase as "If even one tablet of the table doesn't support live row counting, -1 will be returned."
--
To view, visit http://gerrit.cloudera.org:8080/14484
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I757978cb496d47306f91ea251cba00bde7929e36
Gerrit-Change-Number: 14484
Gerrit-PatchSet: 1
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 17 Oct 2019 18:23:03 +0000
Gerrit-HasComments: Yes
[kudu-CR] WIP: fix wrong value of live row count
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/14484 )
Change subject: WIP: fix wrong value of live_row_count
......................................................................
Patch Set 1:
> Patch Set 1:
>
> > There are sevral palce need fix:
> > 1. tablet metric: METRIC_live_row_count instantiate with a 'invisible' status instead of -1
> > 2. tserver heartbeat to master: use a separate boolean to indicate that this tablet not support live_row_count
> > 3. metrics merge: if any one of metric is 'invisible' when merge two metrics, the result should be 'invisible' too.
>
> Unfortunately InstantiateHidden doesn't work for FunctionGauges, which is what METRIC_live_row_count is. So we can't hide them using that mechanism.
>
> FunctionGauge(const GaugePrototype<T>* proto, Callback<T()> function)
> : Gauge(proto), function_(std::move(function)) {
> // Override the modification epoch to the maximum, since we don't have any idea
> // when the bound function changes value.
> m_epoch_ = std::numeric_limits<decltype(m_epoch_.load())>::max();
> }
We can check wether this replica support live_row_count before init it, then init it by InstantiateHidden or InstantiateFunctionGauge.
--
To view, visit http://gerrit.cloudera.org:8080/14484
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I757978cb496d47306f91ea251cba00bde7929e36
Gerrit-Change-Number: 14484
Gerrit-PatchSet: 1
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Fri, 18 Oct 2019 05:35:16 +0000
Gerrit-HasComments: No
[kudu-CR] WIP: fix wrong value of live row count
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14484 )
Change subject: WIP: fix wrong value of live_row_count
......................................................................
Patch Set 1:
(1 comment)
> Patch Set 1:
>
> (1 comment)
http://gerrit.cloudera.org:8080/#/c/14484/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14484/1//COMMIT_MSG@16
PS1, Line 16: *need to fix merge metric bug.
> When we fetch aggregated metrics from a tablet server by retrieving data fr
Ugh. What about this as an alternative: in TabletReplica::Start, only instantiate METRIC_live_row_count if the tablet metadata supports live row counting. Then we should never see a -1 value there. We could even make it a uint64 gauge at that point. It's a little weird that that some tablets will report this metric and others won't, but maybe that's OK?
Separately I really want us to stop passing -1 through heartbeats and the catalog manager to indicate "not supported". We could use a separate boolean for that and it'd make the decision making in catalog manager much clearer.
--
To view, visit http://gerrit.cloudera.org:8080/14484
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I757978cb496d47306f91ea251cba00bde7929e36
Gerrit-Change-Number: 14484
Gerrit-PatchSet: 1
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Fri, 18 Oct 2019 03:24:27 +0000
Gerrit-HasComments: Yes
[kudu-CR] WIP: fix wrong value of live row count
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14484 )
Change subject: WIP: fix wrong value of live_row_count
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14484/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14484/1//COMMIT_MSG@16
PS1, Line 16: *need to fix merge metric bug.
> What is this alluding to?
When we fetch aggregated metrics from a tablet server by retrieving data from URLs of form http://<host>:<port>/metrics?merge_rules=tablet|table|table_name, the value of metric 'live_row_count' should also be specially treated.
I'm considering modify the method 'MergeFrom' of class 'AtomicGauge' , fix it for uint64 vs. int64, for the existing metrics of AtomicGauge are all uint64 values. However, it may be not suitable to do that because AtomicGauge is implemented for int64 types.
--
To view, visit http://gerrit.cloudera.org:8080/14484
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I757978cb496d47306f91ea251cba00bde7929e36
Gerrit-Change-Number: 14484
Gerrit-PatchSet: 1
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Fri, 18 Oct 2019 03:07:19 +0000
Gerrit-HasComments: Yes