)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4e168f88a68204f8e7f73897f2fce9a399620d9b","unresolved":false,"context_lines":[{"line_number":13,"context_line":"of metric gathering (read: incoherent results for CPU percentages)."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"By simply caching the timestamp *after* the stats are retrieved, we"},{"line_number":16,"context_line":"avoid this condition and the results are always coherent."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Change-Id: Ia23783f2e6b92b3af2e9acf09f95a9eeacd8eb3b"},{"line_number":19,"context_line":"Closes-Bug: 1490837"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"da20952f_68daabeb","line":16,"updated":"2015-09-01 15:34:30.000000000","message":"Well, it sounds like an hack, right?\n\nI just wonder if the logic could match with the current behaviour of oslo_service.periodic_task where it either runs immediatly if it\u0027s late or waits for the next tick :\n\nhttps://github.com/openstack/oslo.service/blob/master/oslo_service/periodic_task.py\n\nPS: Since periodic_tasks needs to be part of a Nova service, we can\u0027t use the discovery feature of the metaclass","commit_id":"e5eebf373fa0ee9722db453ef5b6498659cba9e5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"f0df375c4c7f61e37e013ad4ca3fb2fb02f74edc","unresolved":false,"context_lines":[{"line_number":13,"context_line":"of metric gathering (read: incoherent results for CPU percentages)."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"By simply caching the timestamp *after* the stats are retrieved, we"},{"line_number":16,"context_line":"avoid this condition and the results are always coherent."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Change-Id: Ia23783f2e6b92b3af2e9acf09f95a9eeacd8eb3b"},{"line_number":19,"context_line":"Closes-Bug: 1490837"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"da20952f_81190c7c","line":16,"in_reply_to":"da20952f_309410e7","updated":"2015-09-01 20:36:41.000000000","message":"So, I won\u0027t paraphrase what we agreed over IRC, just restating the summary :\n- 1/ some metrics need to be correlated in a group of metrics, per se iowait/user/kernel/idle time, in order to make it consistent\n- 2/ some metrics are derivated from 1/ (per se, percentages) so those need to be having the same timestamp\n- 3/ for others, we can safely assume it\u0027s fair to call again libvirt if we\u0027re above 1 sec because it\u0027s fine to have 2 metrics having 2 different timestamps if they are unrelated","commit_id":"e5eebf373fa0ee9722db453ef5b6498659cba9e5"},{"author":{"_account_id":7664,"name":"Joe Cropper","email":"jwcroppe@us.ibm.com","username":"jwcroppe"},"change_message_id":"2d265d86c29e20384bbd13586137a9ce2af893d0","unresolved":false,"context_lines":[{"line_number":13,"context_line":"of metric gathering (read: incoherent results for CPU percentages)."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"By simply caching the timestamp *after* the stats are retrieved, we"},{"line_number":16,"context_line":"avoid this condition and the results are always coherent."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Change-Id: Ia23783f2e6b92b3af2e9acf09f95a9eeacd8eb3b"},{"line_number":19,"context_line":"Closes-Bug: 1490837"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"da20952f_309410e7","line":16,"in_reply_to":"da20952f_5062e4ae","updated":"2015-09-01 19:43:52.000000000","message":"That makes sense, Sylvain.\n\nHowever, I don\u0027t think that is the intent of the logic as it was originally written (i.e., I really think it\u0027s a bug)... the idea is that you get a set of \"metrics\" which are then written to the database as a single entity and sent via RPC notifier.  Also note that it\u0027s only the *first* metric called from the set of metric (10 total... see https://github.com/openstack/nova/blob/master/nova/compute/monitors/base.py#L71 for complete list) that suffers from this issue since the next n-1 calls *will* use the cache all the time since the calls are so successively fast.\n\nThe core problem here is that the get_metric(metric_name), and thus _update_data() is called for each *individual* metric, you can get things out of sync between call 1 and n-1 for each metric.\n\nFor example, one would not expect to get an RPC notification for values of (cpu.kernel.percent, cpu.iowait.percent, cpu.user.percent, cpu.idle.percent) such that the summation of those is not 100.\n\nMy change simply ensures the metrics [1, n] are all using the same cached value.\n\nHope this helps.","commit_id":"e5eebf373fa0ee9722db453ef5b6498659cba9e5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"44400e2626cb3a2f438edd8e7cf7ee71b32f8c0b","unresolved":false,"context_lines":[{"line_number":13,"context_line":"of metric gathering (read: incoherent results for CPU percentages)."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"By simply caching the timestamp *after* the stats are retrieved, we"},{"line_number":16,"context_line":"avoid this condition and the results are always coherent."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Change-Id: Ia23783f2e6b92b3af2e9acf09f95a9eeacd8eb3b"},{"line_number":19,"context_line":"Closes-Bug: 1490837"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"da20952f_5062e4ae","line":16,"in_reply_to":"da20952f_653091e9","updated":"2015-09-01 19:36:05.000000000","message":"So, my bad, I\u0027m unclear, lemme rephrase.\nSo, the time check is just for making sure that there is no reason to call again the driver if we have the same timestamp, right? If that case, we can directly return the cache (because the timestamp tick is per second)\n\nWhat if the driver is taking more than 1 second to answer the call ? In the original version, the next _update_data() call will again query the driver, which I think is good (because it could be due to a transient IO lock, or also because the values could have changed etc.)\nNow, with your change, the TS will be identical for the next _update_data() call, right? In that case, it will serve the cached information, which I think is highly doubtable.\n\nIs it clearer ?","commit_id":"e5eebf373fa0ee9722db453ef5b6498659cba9e5"},{"author":{"_account_id":7664,"name":"Joe Cropper","email":"jwcroppe@us.ibm.com","username":"jwcroppe"},"change_message_id":"f72cba72f37ba90b88a4c13861c23eb09ab327ff","unresolved":false,"context_lines":[{"line_number":13,"context_line":"of metric gathering (read: incoherent results for CPU percentages)."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"By simply caching the timestamp *after* the stats are retrieved, we"},{"line_number":16,"context_line":"avoid this condition and the results are always coherent."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Change-Id: Ia23783f2e6b92b3af2e9acf09f95a9eeacd8eb3b"},{"line_number":19,"context_line":"Closes-Bug: 1490837"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"da20952f_653091e9","line":16,"in_reply_to":"da20952f_68daabeb","updated":"2015-09-01 16:08:15.000000000","message":"The periodic task isn\u0027t \"late\" per se, it\u0027s just that the current code runs the host CPU stat retrieval *after* it caches the timestamp, so you lose part of your 1 second to the stat retrieval.\n\nWhile perhaps we could perform some major overhaul to how this works in general, I think this patch minimizes the overall impact to the code and solves a problem we have at present.","commit_id":"e5eebf373fa0ee9722db453ef5b6498659cba9e5"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"ec465d950319ed65322373be434b64efdb4dfbe1","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch addresses a problem in which the current Nova metrics can"},{"line_number":10,"context_line":"yield a set of incoherent results. For example, prior to this patch,"},{"line_number":11,"context_line":"it is possible that a *single* metric record can contain values such"},{"line_number":12,"context_line":"as the following:"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"{\u0027cpu.user.percent\u0027: A, \u0027cpu.kernel.percent\u0027: B,"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":15,"id":"da20952f_74b2f397","line":11,"updated":"2015-09-11 13:53:07.000000000","message":"Technically, there\u0027s no such thing as a single metric record :) I think you meant to say \"it is possible that the collection of metrics sent in a single notification message can contain...\"","commit_id":"7b3c3b0d2e32bdae007064594351a1c9ec1acc01"},{"author":{"_account_id":7664,"name":"Joe Cropper","email":"jwcroppe@us.ibm.com","username":"jwcroppe"},"change_message_id":"a2b32f0cb5e48d24470159f3d731b7cffabe74e7","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch addresses a problem in which the current Nova metrics can"},{"line_number":10,"context_line":"yield a set of incoherent results. For example, prior to this patch,"},{"line_number":11,"context_line":"it is possible that a *single* metric record can contain values such"},{"line_number":12,"context_line":"as the following:"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"{\u0027cpu.user.percent\u0027: A, \u0027cpu.kernel.percent\u0027: B,"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":15,"id":"da20952f_6eaf77e3","line":11,"in_reply_to":"da20952f_74b2f397","updated":"2015-09-11 16:01:37.000000000","message":"Done","commit_id":"7b3c3b0d2e32bdae007064594351a1c9ec1acc01"},{"author":{"_account_id":7664,"name":"Joe Cropper","email":"jwcroppe@us.ibm.com","username":"jwcroppe"},"change_message_id":"ddeb77c24444950875c8f36febb9c978d66e5255","unresolved":false,"context_lines":[{"line_number":23,"context_line":""},{"line_number":24,"context_line":"To solve this, we have created a more concrete structure to indicate"},{"line_number":25,"context_line":"that a group of metrics are truly a set of metrics; this is realized"},{"line_number":26,"context_line":"through the `MetricSetMonitorBase` abstract class. The CPU monitor"},{"line_number":27,"context_line":"proceeds to extend this new class and returns all of the CPU metrics"},{"line_number":28,"context_line":"as a single set of metrics, which solves the coherency issue."},{"line_number":29,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"9a1a9d01_1f3aec98","line":26,"updated":"2015-10-04 19:02:37.000000000","message":"I suppose we should get rid of the \"set\" piece here too.","commit_id":"98afaa3e4532e9079378a61dfe8065449e5e22bf"}],"nova/compute/monitors/base.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b5e22174b34470c0fb4d198bc690c378875c0702","unresolved":false,"context_lines":[{"line_number":63,"context_line":"            # NOTE(jwcroppe): Refresh the underlying data points the first time"},{"line_number":64,"context_line":"            # through and then simply reuse the same data samples for the"},{"line_number":65,"context_line":"            # remaining metrics so we get a consistent set of metrics."},{"line_number":66,"context_line":"            refresh_data \u003d False"},{"line_number":67,"context_line":"            metric \u003d objects.MonitorMetric(name\u003dname,"},{"line_number":68,"context_line":"                                           value\u003dvalue,"},{"line_number":69,"context_line":"                                           timestamp\u003dtimestamp,"}],"source_content_type":"text/x-python","patch_set":12,"id":"da20952f_d45c431b","line":66,"updated":"2015-09-10 18:08:58.000000000","message":"Why not just call update_data() before the loop (making it a non-private method, of course) instead of hacking the update-first behavior of get_metric()?","commit_id":"12d37ce598a1966124ea91a447fc4170b7da8c05"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"ec465d950319ed65322373be434b64efdb4dfbe1","unresolved":false,"context_lines":[{"line_number":10,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":11,"context_line":"#    under the License."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"import abc"},{"line_number":14,"context_line":"import six"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"from nova import objects"}],"source_content_type":"text/x-python","patch_set":15,"id":"da20952f_ef470a4a","line":13,"updated":"2015-09-11 13:53:07.000000000","message":"Any reason you removed the newline here?","commit_id":"7b3c3b0d2e32bdae007064594351a1c9ec1acc01"},{"author":{"_account_id":7664,"name":"Joe Cropper","email":"jwcroppe@us.ibm.com","username":"jwcroppe"},"change_message_id":"a2b32f0cb5e48d24470159f3d731b7cffabe74e7","unresolved":false,"context_lines":[{"line_number":10,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":11,"context_line":"#    under the License."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"import abc"},{"line_number":14,"context_line":"import six"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"from nova import objects"}],"source_content_type":"text/x-python","patch_set":15,"id":"da20952f_2eb5ff30","line":13,"in_reply_to":"da20952f_ef470a4a","updated":"2015-09-11 16:01:37.000000000","message":"OCD.  I\u0027ll put it back.  ;-)","commit_id":"7b3c3b0d2e32bdae007064594351a1c9ec1acc01"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"ec465d950319ed65322373be434b64efdb4dfbe1","unresolved":false,"context_lines":[{"line_number":49,"context_line":""},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"class MetricSetMonitorBase(MonitorBase):"},{"line_number":52,"context_line":"    \"\"\"Base class for all monitors that return a set of metrics.\"\"\""},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"    @abc.abstractmethod"},{"line_number":55,"context_line":"    def get_metrics(self):"}],"source_content_type":"text/x-python","patch_set":15,"id":"da20952f_ef72aa49","line":52,"updated":"2015-09-11 13:53:07.000000000","message":"Please update this docstring to make it clearer that this base class is for monitors that treat their collection of metrics as a singular, related set.","commit_id":"7b3c3b0d2e32bdae007064594351a1c9ec1acc01"},{"author":{"_account_id":7664,"name":"Joe Cropper","email":"jwcroppe@us.ibm.com","username":"jwcroppe"},"change_message_id":"a2b32f0cb5e48d24470159f3d731b7cffabe74e7","unresolved":false,"context_lines":[{"line_number":49,"context_line":""},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"class MetricSetMonitorBase(MonitorBase):"},{"line_number":52,"context_line":"    \"\"\"Base class for all monitors that return a set of metrics.\"\"\""},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"    @abc.abstractmethod"},{"line_number":55,"context_line":"    def get_metrics(self):"}],"source_content_type":"text/x-python","patch_set":15,"id":"da20952f_4e65d3ac","line":52,"in_reply_to":"da20952f_ef72aa49","updated":"2015-09-11 16:01:37.000000000","message":"Done","commit_id":"7b3c3b0d2e32bdae007064594351a1c9ec1acc01"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"da2af3b616fa387131798d5a71230cd20b7ab958","unresolved":false,"context_lines":[{"line_number":39,"context_line":"        raise NotImplementedError(\u0027get_metric_names\u0027)"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"    @abc.abstractmethod"},{"line_number":42,"context_line":"    def add_metrics_to_list(self, metrics_list):"},{"line_number":43,"context_line":"        \"\"\"Adds metric objects to a supplied list object."},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"        :param metric_list: nova.objects.MonitorMetricList that the monitor"}],"source_content_type":"text/x-python","patch_set":16,"id":"9a1a9d01_d6f7d5f3","line":42,"range":{"start_line":42,"start_character":8,"end_line":42,"end_character":27},"updated":"2015-09-25 08:27:58.000000000","message":"this is a weird interface. MonitorBase doesn\u0027t seem to be used very much yet, is this a good opportunity to remove this abstract method and replace it with get_metrics?","commit_id":"be41d60a3db07601fbc54cdaaa2b2add21a37076"},{"author":{"_account_id":7664,"name":"Joe Cropper","email":"jwcroppe@us.ibm.com","username":"jwcroppe"},"change_message_id":"c54944316a53e8b5626a7cd88024319ca60e8fb1","unresolved":false,"context_lines":[{"line_number":39,"context_line":"        raise NotImplementedError(\u0027get_metric_names\u0027)"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"    @abc.abstractmethod"},{"line_number":42,"context_line":"    def add_metrics_to_list(self, metrics_list):"},{"line_number":43,"context_line":"        \"\"\"Adds metric objects to a supplied list object."},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"        :param metric_list: nova.objects.MonitorMetricList that the monitor"}],"source_content_type":"text/x-python","patch_set":16,"id":"9a1a9d01_470f7ae1","line":42,"in_reply_to":"9a1a9d01_d6f7d5f3","updated":"2015-09-25 09:06:27.000000000","message":"This is what Jay Pipes and I discussed... I think he wanted this for future use cases.  I will need to let Jay comment though.","commit_id":"be41d60a3db07601fbc54cdaaa2b2add21a37076"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"da2af3b616fa387131798d5a71230cd20b7ab958","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        raise NotImplementedError(\u0027add_metrics_to_list\u0027)"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"class MetricSetMonitorBase(MonitorBase):"},{"line_number":53,"context_line":"    \"\"\"Base class for all monitors that return a set of metrics. These metrics"},{"line_number":54,"context_line":"    are treated as a singular, related set of metrics that are derived from the"},{"line_number":55,"context_line":"    same set of underlying statistics (e.g., single call to get libvirt stats)."}],"source_content_type":"text/x-python","patch_set":16,"id":"9a1a9d01_d6459517","line":52,"range":{"start_line":52,"start_character":6,"end_line":52,"end_character":26},"updated":"2015-09-25 08:27:58.000000000","message":"MonitorBase already involves a set of metrics so it\u0027s not obvious from this name what the difference is. Something like SingleCaptureMonitorBase maybe?","commit_id":"be41d60a3db07601fbc54cdaaa2b2add21a37076"},{"author":{"_account_id":7664,"name":"Joe Cropper","email":"jwcroppe@us.ibm.com","username":"jwcroppe"},"change_message_id":"c54944316a53e8b5626a7cd88024319ca60e8fb1","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        raise NotImplementedError(\u0027add_metrics_to_list\u0027)"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"class MetricSetMonitorBase(MonitorBase):"},{"line_number":53,"context_line":"    \"\"\"Base class for all monitors that return a set of metrics. These metrics"},{"line_number":54,"context_line":"    are treated as a singular, related set of metrics that are derived from the"},{"line_number":55,"context_line":"    same set of underlying statistics (e.g., single call to get libvirt stats)."}],"source_content_type":"text/x-python","patch_set":16,"id":"9a1a9d01_27ccc688","line":52,"in_reply_to":"9a1a9d01_d6459517","updated":"2015-09-25 09:06:27.000000000","message":"Jay and I also discussed this for a while too.\n\nYou make a good point, but I think it is likely that you will need to look at the class doc string regardless of the name.  To be honest, I am not sure that I would know what SingleCaptureMonitorBase implies either without reading the docs.  :-)","commit_id":"be41d60a3db07601fbc54cdaaa2b2add21a37076"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"da2af3b616fa387131798d5a71230cd20b7ab958","unresolved":false,"context_lines":[{"line_number":59,"context_line":"    def get_metrics(self):"},{"line_number":60,"context_line":"        \"\"\"Get information for a set of metrics."},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"        :returns: set of (name, value, timestamp) tuples for each metric in"},{"line_number":63,"context_line":"                  the set."},{"line_number":64,"context_line":"        \"\"\""},{"line_number":65,"context_line":"        raise NotImplementedError(\u0027get_metrics\u0027)"}],"source_content_type":"text/x-python","patch_set":16,"id":"9a1a9d01_5c142df4","line":62,"updated":"2015-09-25 08:27:58.000000000","message":"wouldn\u0027t a dict of {name: (value, timestamp)} be more convenient?","commit_id":"be41d60a3db07601fbc54cdaaa2b2add21a37076"},{"author":{"_account_id":7664,"name":"Joe Cropper","email":"jwcroppe@us.ibm.com","username":"jwcroppe"},"change_message_id":"c54944316a53e8b5626a7cd88024319ca60e8fb1","unresolved":false,"context_lines":[{"line_number":59,"context_line":"    def get_metrics(self):"},{"line_number":60,"context_line":"        \"\"\"Get information for a set of metrics."},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"        :returns: set of (name, value, timestamp) tuples for each metric in"},{"line_number":63,"context_line":"                  the set."},{"line_number":64,"context_line":"        \"\"\""},{"line_number":65,"context_line":"        raise NotImplementedError(\u0027get_metrics\u0027)"}],"source_content_type":"text/x-python","patch_set":16,"id":"9a1a9d01_47821a02","line":62,"in_reply_to":"9a1a9d01_5c142df4","updated":"2015-09-25 09:06:27.000000000","message":"This would be a simple change, but I think the data is pretty convenient to work with in its current form as well.  For example, the previous interface was a tuple of (name, value) dict(name\u003d\u003evalue) and that too was simple to work with.","commit_id":"be41d60a3db07601fbc54cdaaa2b2add21a37076"}],"nova/compute/monitors/cpu/virt_driver.py":[{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"8944cfb1ec20697dd1c45924eed8be51d9914770","unresolved":false,"context_lines":[{"line_number":69,"context_line":""},{"line_number":70,"context_line":"        # NOTE(jwcroppe): We set the cache timestamp here so that the call to"},{"line_number":71,"context_line":"        # `get_host_cpu_stats` does not take away from our time-to-live."},{"line_number":72,"context_line":"        self._data[\"timestamp\"] \u003d timeutils.utcnow()"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"        # The compute driver API returns the absolute values for CPU times."},{"line_number":75,"context_line":"        # We compute the utilization percentages for each specific CPU time"}],"source_content_type":"text/x-python","patch_set":3,"id":"da20952f_207703a1","line":72,"updated":"2015-09-01 07:55:24.000000000","message":"I\u0027m thinking whether there is way we can prevent the same mistake when other people add new monitor from the framework.\n\nMaybe something like move the _update_data into the base.CPUMonitorBase, and add a decorator to update the timestamp.\n\nThen other implementor can just overwrite the _update_data, and won\u0027t make same mistake.","commit_id":"e5eebf373fa0ee9722db453ef5b6498659cba9e5"},{"author":{"_account_id":7664,"name":"Joe Cropper","email":"jwcroppe@us.ibm.com","username":"jwcroppe"},"change_message_id":"af6e0bfcfd551c62467bf4ab003271214c7d77a5","unresolved":false,"context_lines":[{"line_number":69,"context_line":""},{"line_number":70,"context_line":"        # NOTE(jwcroppe): We set the cache timestamp here so that the call to"},{"line_number":71,"context_line":"        # `get_host_cpu_stats` does not take away from our time-to-live."},{"line_number":72,"context_line":"        self._data[\"timestamp\"] \u003d timeutils.utcnow()"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"        # The compute driver API returns the absolute values for CPU times."},{"line_number":75,"context_line":"        # We compute the utilization percentages for each specific CPU time"}],"source_content_type":"text/x-python","patch_set":3,"id":"da20952f_2dbb5777","line":72,"in_reply_to":"da20952f_0d80d345","updated":"2015-09-01 19:25:50.000000000","message":"Dan, you\u0027re right that this is imperfect... the key is that the libvirt call is the \u0027expensive\u0027 call, the rest of the loop is retrieving data from the dict-based caches.\n\nI agree it\u0027s not perfect, but it is an improvement over what we have today.  If I was going to re-write the framework, I would do it differently... but this was just intended to help remedy one racey condition we have right now.","commit_id":"e5eebf373fa0ee9722db453ef5b6498659cba9e5"},{"author":{"_account_id":7664,"name":"Joe Cropper","email":"jwcroppe@us.ibm.com","username":"jwcroppe"},"change_message_id":"7c07ffae30c9ec8e765f1c5b46ad67a4cedcb79b","unresolved":false,"context_lines":[{"line_number":69,"context_line":""},{"line_number":70,"context_line":"        # NOTE(jwcroppe): We set the cache timestamp here so that the call to"},{"line_number":71,"context_line":"        # `get_host_cpu_stats` does not take away from our time-to-live."},{"line_number":72,"context_line":"        self._data[\"timestamp\"] \u003d timeutils.utcnow()"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"        # The compute driver API returns the absolute values for CPU times."},{"line_number":75,"context_line":"        # We compute the utilization percentages for each specific CPU time"}],"source_content_type":"text/x-python","patch_set":3,"id":"da20952f_80704f41","line":72,"in_reply_to":"da20952f_207703a1","updated":"2015-09-01 08:04:52.000000000","message":"Interesting idea, Alex!\n\nI think the _update_data() is a function of the specific gatherer, so not sure it belongs in the parent class.  Also, I\u0027d like to minimize the deltas we introduce at this time in Liberty related to this code.","commit_id":"e5eebf373fa0ee9722db453ef5b6498659cba9e5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f45f5e1e874cab943ac1098906e1f3bd448c58c8","unresolved":false,"context_lines":[{"line_number":69,"context_line":""},{"line_number":70,"context_line":"        # NOTE(jwcroppe): We set the cache timestamp here so that the call to"},{"line_number":71,"context_line":"        # `get_host_cpu_stats` does not take away from our time-to-live."},{"line_number":72,"context_line":"        self._data[\"timestamp\"] \u003d timeutils.utcnow()"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"        # The compute driver API returns the absolute values for CPU times."},{"line_number":75,"context_line":"        # We compute the utilization percentages for each specific CPU time"}],"source_content_type":"text/x-python","patch_set":3,"id":"da20952f_0d80d345","line":72,"in_reply_to":"da20952f_80704f41","updated":"2015-09-01 19:12:40.000000000","message":"This doesn\u0027t make any sense to me. Isn\u0027t this just moving the penalty for a delayed response from the stats method to the trailing edge instead of the leading edge?\n\nDepending on when stats are actually collected, if it\u0027s not instantaneous, you\u0027re either going to apply the extra to the cycle in after you took the timestamp or before, right?\n\nTo be honest, this gathering loop looks highly imperfect so expecting the math to work out any time is probably silly. If we really care about things like \"what percentage of the time was spent in iowait\" then we should add up all the numbers and figure out what percentage of *that* total is iowait, right?","commit_id":"e5eebf373fa0ee9722db453ef5b6498659cba9e5"},{"author":{"_account_id":7664,"name":"Joe Cropper","email":"jwcroppe@us.ibm.com","username":"jwcroppe"},"change_message_id":"9823fb0dfbe1da2debd760900ebdccd90a1051a8","unresolved":false,"context_lines":[{"line_number":45,"context_line":"        return self._data[name], self._data[\"timestamp\"]"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    def _update_data(self):"},{"line_number":48,"context_line":"        # Don\u0027t allow to call this function so frequently (\u003c\u003d 1 sec)"},{"line_number":49,"context_line":"        now \u003d timeutils.utcnow()"},{"line_number":50,"context_line":"        if self._data.get(\"timestamp\") is not None:"},{"line_number":51,"context_line":"            delta \u003d now - self._data.get(\"timestamp\")"}],"source_content_type":"text/x-python","patch_set":7,"id":"da20952f_9161de3b","side":"PARENT","line":48,"updated":"2015-09-04 07:48:12.000000000","message":"There is no point to this code with the refresh_data - bauzas and I both assert removing this would be goodness.  :-)","commit_id":"f003b63689654c99f94e41a30951aa1930db7648"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"1b18b174427482f539a591c788c611df9495498a","unresolved":false,"context_lines":[{"line_number":45,"context_line":"        self._update_data()"},{"line_number":46,"context_line":"        for name in self.get_metric_names():"},{"line_number":47,"context_line":"            metrics.append((name, self._data[name], self._data[\"timestamp\"]))"},{"line_number":48,"context_line":"        return metrics"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    def _update_data(self):"},{"line_number":51,"context_line":"        self._data \u003d {}"}],"source_content_type":"text/x-python","patch_set":19,"id":"9a1a9d01_645837fb","line":48,"updated":"2015-10-05 09:33:39.000000000","message":"I\u0027m a big fan of list comprehensions, but this is fine","commit_id":"7fe05a8ca45b36c173458b92aa6ad6d9df861736"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3badc5adb1994f4b49e0c29269778bac882f54b1","unresolved":false,"context_lines":[{"line_number":45,"context_line":"        self._update_data()"},{"line_number":46,"context_line":"        for name in self.get_metric_names():"},{"line_number":47,"context_line":"            metrics.append((name, self._data[name], self._data[\"timestamp\"]))"},{"line_number":48,"context_line":"        return metrics"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    def _update_data(self):"},{"line_number":51,"context_line":"        self._data \u003d {}"}],"source_content_type":"text/x-python","patch_set":19,"id":"7a2fa921_b84e6a6d","line":48,"in_reply_to":"9a1a9d01_645837fb","updated":"2015-10-07 15:34:36.000000000","message":"I\u0027m a big fan of readability. :)","commit_id":"7fe05a8ca45b36c173458b92aa6ad6d9df861736"}],"nova/tests/unit/compute/monitors/cpu/test_virt_driver.py":[{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"b2d6eee8845713d9715e1202ee86dcd665791ec8","unresolved":false,"context_lines":[{"line_number":44,"context_line":""},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"class FakeResourceTracker(object):"},{"line_number":47,"context_line":"    def __init__(self, stats\u003d{\u0027kernel\u0027: 5664160000000,"},{"line_number":48,"context_line":"                              \u0027idle\u0027: 1592705190000000,"},{"line_number":49,"context_line":"                              \u0027frequency\u0027: 800,"},{"line_number":50,"context_line":"                              \u0027user\u0027: 26728850000000,"}],"source_content_type":"text/x-python","patch_set":11,"id":"da20952f_9da1daf9","line":47,"updated":"2015-09-04 17:25:37.000000000","message":"You should never default a named param to a mutable value (see http://docs.python-guide.org/en/latest/writing/gotchas/). Instead, default to None, and then add a \"if stats is None:\" block where you set the value to the default.","commit_id":"f16a4139b6dd3db4d829cfe07d8aca6253a17fb8"},{"author":{"_account_id":7664,"name":"Joe Cropper","email":"jwcroppe@us.ibm.com","username":"jwcroppe"},"change_message_id":"662d754a6aa80ba9625ebdd7ea29ea0abc898930","unresolved":false,"context_lines":[{"line_number":44,"context_line":""},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"class FakeResourceTracker(object):"},{"line_number":47,"context_line":"    def __init__(self, stats\u003d{\u0027kernel\u0027: 5664160000000,"},{"line_number":48,"context_line":"                              \u0027idle\u0027: 1592705190000000,"},{"line_number":49,"context_line":"                              \u0027frequency\u0027: 800,"},{"line_number":50,"context_line":"                              \u0027user\u0027: 26728850000000,"}],"source_content_type":"text/x-python","patch_set":11,"id":"da20952f_dd8f5221","line":47,"in_reply_to":"da20952f_9da1daf9","updated":"2015-09-04 17:32:28.000000000","message":"Thanks!  Done.","commit_id":"f16a4139b6dd3db4d829cfe07d8aca6253a17fb8"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"da2af3b616fa387131798d5a71230cd20b7ab958","unresolved":false,"context_lines":[{"line_number":34,"context_line":"            # NOTE(jwcroppe): Sleep just a little bit more than 2 seconds to"},{"line_number":35,"context_line":"            # force an abnormal delay in `get_host_cpu_stats` to ensure that"},{"line_number":36,"context_line":"            # even a slow libvirt call results in a coherent set of metrics."},{"line_number":37,"context_line":"            time.sleep(2.5)"},{"line_number":38,"context_line":"        if self.auto_increment_stats:"},{"line_number":39,"context_line":"            # Simulate some CPU time"},{"line_number":40,"context_line":"            for (k, v) in six.iteritems(self.stats):"}],"source_content_type":"text/x-python","patch_set":16,"id":"9a1a9d01_fce74110","line":37,"updated":"2015-09-25 08:27:58.000000000","message":"sleeping causes race conditions and slow tests. Is it not possible to avoid this with some mocks?","commit_id":"be41d60a3db07601fbc54cdaaa2b2add21a37076"}],"nova/tests/unit/compute/test_resource_tracker.py":[{"author":{"_account_id":7664,"name":"Joe Cropper","email":"jwcroppe@us.ibm.com","username":"jwcroppe"},"change_message_id":"233701cd33c101e9e1345e49d16212f067b0016f","unresolved":false,"context_lines":[{"line_number":1245,"context_line":"        self.assertEqual(0, len(metrics))"},{"line_number":1246,"context_line":""},{"line_number":1247,"context_line":"    def test_get_host_metrics(self):"},{"line_number":1248,"context_line":"        class FakeCPUMonitor(monitor_base.MetricSetMonitorBase):"},{"line_number":1249,"context_line":""},{"line_number":1250,"context_line":"            NOW_TS \u003d timeutils.utcnow()"},{"line_number":1251,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"9a1a9d01_3fb9f02e","line":1248,"updated":"2015-10-04 18:45:10.000000000","message":"We need to switch this back to MonitorBase.","commit_id":"98afaa3e4532e9079378a61dfe8065449e5e22bf"}]}