You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2016/08/20 00:54:54 UTC
[kudu-CR] KUDU-1571. Generate "deprecated" metrics in MDL
Hello Mike Percy, Adar Dembo,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/4067
to review the following change.
Change subject: KUDU-1571. Generate "deprecated" metrics in MDL
......................................................................
KUDU-1571. Generate "deprecated" metrics in MDL
This checks in the "--dump_metrics_json" output from already-released
versions, and changes the generate_mdl.py script so that any metrics
which were ever present in an earlier version are carried through to the
current version of the MDL, but marked as deprecated.
This works around an issue in which Cloudera Manager doesn't properly
handle the removal of metrics. Namely, if a dashboard (user-generated or
built-in) references a removed metric, it will produce an NPE.
This will now become part of the release process: after a release, we
should check in the output of 'dump_metrics_json' so that the metrics in
that release will be carried forward.
I verified the results using both the newly included
'check_csd_compatibility' script as well as installing a newly built
0.10.x CSD on a cluster. Whereas the tablet server status page was
previously broken, now it shows the remote bootstrap metrics as
DEPRECATED.
Change-Id: I121203d781ab593cb94d7248d164cdad7bf26e78
---
M build-support/release/rat_exclude_files.txt
A java/kudu-csd/check_csd_compatibility.py
M java/kudu-csd/generate_mdl.py
A java/kudu-csd/old-version-metrics/0.10.0-master.json
A java/kudu-csd/old-version-metrics/0.10.0-tserver.json
A java/kudu-csd/old-version-metrics/0.9.1-master.json
A java/kudu-csd/old-version-metrics/0.9.1-tserver.json
7 files changed, 4,831 insertions(+), 17 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/67/4067/1
--
To view, visit http://gerrit.cloudera.org:8080/4067
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I121203d781ab593cb94d7248d164cdad7bf26e78
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
[kudu-CR] KUDU-1571. Generate "deprecated" metrics in MDL
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.
Change subject: KUDU-1571. Generate "deprecated" metrics in MDL
......................................................................
Patch Set 1: Code-Review+2
(2 comments)
I found a couple nits, if you have time to fix them please just +2 this patch after making those changes
http://gerrit.cloudera.org:8080/#/c/4067/1/java/kudu-csd/check_csd_compatibility.py
File java/kudu-csd/check_csd_compatibility.py:
Line 64: if len(removed_metrics):
nit: This isn't very pythonic. Should be `if removed_metrics:`, I think.
http://gerrit.cloudera.org:8080/#/c/4067/1/java/kudu-csd/generate_mdl.py
File java/kudu-csd/generate_mdl.py:
Line 177: all_metrics = load_current_metrics()
nit: s/all_metrics/current_metrics/
--
To view, visit http://gerrit.cloudera.org:8080/4067
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I121203d781ab593cb94d7248d164cdad7bf26e78
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1571. Generate "deprecated" metrics in MDL
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: KUDU-1571. Generate "deprecated" metrics in MDL
......................................................................
Patch Set 1: Code-Review-1
I would prefer we don't go down this route. It's adding complexity to our release process. I'd rather we do a one-off fix for 0.10.0 (such as manually adding the deprecated metrics to the generated MDL), and fix the underlying tight coupling in Cloudera Manager. I can volunteer to do that if need be. Then we document the limitation for previously released versions of CM and call it a day.
It's not a great solution, but it's far cheaper (since it places no onus on future Kudu releases), and it improves the quality of CM too.
--
To view, visit http://gerrit.cloudera.org:8080/4067
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I121203d781ab593cb94d7248d164cdad7bf26e78
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No
[kudu-CR] KUDU-1571. Generate "deprecated" metrics in MDL
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.
Change subject: KUDU-1571. Generate "deprecated" metrics in MDL
......................................................................
Patch Set 1:
Build Started http://104.196.14.100/job/kudu-gerrit/3001/
--
To view, visit http://gerrit.cloudera.org:8080/4067
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I121203d781ab593cb94d7248d164cdad7bf26e78
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No
[kudu-CR] KUDU-1571. Generate "deprecated" metrics in MDL
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: KUDU-1571. Generate "deprecated" metrics in MDL
......................................................................
Patch Set 1: -Code-Review
Removing my -1 based on our discussion.
--
To view, visit http://gerrit.cloudera.org:8080/4067
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I121203d781ab593cb94d7248d164cdad7bf26e78
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No
[kudu-CR] KUDU-1571. Generate "deprecated" metrics in MDL
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: KUDU-1571. Generate "deprecated" metrics in MDL
......................................................................
Patch Set 1:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/4067/1/java/kudu-csd/check_csd_compatibility.py
File java/kudu-csd/check_csd_compatibility.py:
Line 34: Return a set() representing the metrics in the given MDL JSON object.
nit: set() --> set ?
Line 42: for entity_def in mdl['metricEntityTypeDefinitions']:
What if 'metricDefinitions' key is absent in the MDL? Should it be an error or the script should with that like if the list of metric type entities is empty?
Line 62: for m_type, m_entity, m_name in added_metrics:
nit: consider using '_' for unused fields:
for _, m_entity, m_name in added_metrics:
...
Line 64: if len(removed_metrics):
> nit: This isn't very pythonic. Should be `if removed_metrics:`, I think.
nit: consider writing it with less code duplication, like
for metrics, tag in [(added_metrics, "Added"), (removed_metrics), "Removed"]:
print "%s %d metric(s):" % (tag, len(metrics))
for _, m_entity, m_name in metrics:
print " %s metric %s" % (m_entity, m_name)
print "Compatibility check %s" % ("FAILED" if removed_metrics else "PASSED")
PS1, Line 77: May also generate a file containing JSON
Probably I missed something, but I could not see how this script can do this without a slight modification :)
--
To view, visit http://gerrit.cloudera.org:8080/4067
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I121203d781ab593cb94d7248d164cdad7bf26e78
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1571. Generate "deprecated" metrics in MDL
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: KUDU-1571. Generate "deprecated" metrics in MDL
......................................................................
Patch Set 2:
Oops, just realized I missed Mike's code review comments above. Will address these in a follow-on
--
To view, visit http://gerrit.cloudera.org:8080/4067
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I121203d781ab593cb94d7248d164cdad7bf26e78
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] KUDU-1571. Generate "deprecated" metrics in MDL
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has submitted this change and it was merged.
Change subject: KUDU-1571. Generate "deprecated" metrics in MDL
......................................................................
KUDU-1571. Generate "deprecated" metrics in MDL
This checks in the "--dump_metrics_json" output from already-released
versions, and changes the generate_mdl.py script so that any metrics
which were ever present in an earlier version are carried through to the
current version of the MDL, but marked as deprecated.
This works around an issue in which Cloudera Manager doesn't properly
handle the removal of metrics. Namely, if a dashboard (user-generated or
built-in) references a removed metric, it will produce an NPE.
This will now become part of the release process: after a release, we
should check in the output of 'dump_metrics_json' so that the metrics in
that release will be carried forward.
I verified the results using both the newly included
'check_csd_compatibility' script as well as installing a newly built
0.10.x CSD on a cluster. Whereas the tablet server status page was
previously broken, now it shows the remote bootstrap metrics as
DEPRECATED.
Change-Id: I121203d781ab593cb94d7248d164cdad7bf26e78
Reviewed-on: http://gerrit.cloudera.org:8080/4067
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy <mp...@apache.org>
---
M build-support/release/rat_exclude_files.txt
A java/kudu-csd/check_csd_compatibility.py
M java/kudu-csd/generate_mdl.py
A java/kudu-csd/old-version-metrics/0.10.0-master.json
A java/kudu-csd/old-version-metrics/0.10.0-tserver.json
A java/kudu-csd/old-version-metrics/0.9.1-master.json
A java/kudu-csd/old-version-metrics/0.9.1-tserver.json
7 files changed, 4,831 insertions(+), 17 deletions(-)
Approvals:
Mike Percy: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/4067
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I121203d781ab593cb94d7248d164cdad7bf26e78
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-1571. Generate "deprecated" metrics in MDL
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: KUDU-1571. Generate "deprecated" metrics in MDL
......................................................................
Patch Set 1:
To fill in anyone following along, adar and I had an offline discussion about this over the weekend (he was mostly AFK). The gist is that he doesn't like this change but willing to take it as a temporary workaround until CM has some releases which avoid this problem.
--
To view, visit http://gerrit.cloudera.org:8080/4067
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I121203d781ab593cb94d7248d164cdad7bf26e78
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No