New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 735698 link

Starred by 1 user
Status: Archived
Owner:
█ pho...@chromium.org
Last visit > 30 days ago
Closed: Jun 2017
Cc:
dgarr...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

Refactor ts_mon_config._ConsumeMessages

Project Member Reported by pho...@chromium.org, Jun 21 2017 
The logic in _ConsumeMessages is a bit hairy and leads to weird bugs like  crbug.com/735211 . It would be nicer to keep its state in an object, and use methods to mutate or refer to the state. This would let us pull logic out of the loop into helper methods easily.
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 29 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/73a00deaea807968deee0b903c291a8312000a87

commit 73a00deaea807968deee0b903c291a8312000a87
Author: Paul Hobbs <phobbs@google.com>
Date: Thu Jun 29 21:16:36 2017

ts_mon_config: Refactor _ConsumeMessages

Refactor _ConsumeMessages loop into a class. It's more convenient to keep the
state in an object. (The state is the reset_after_flush list, whether we have
metrics pending, and when our last flush was)

This also improves the flush loop's behavior to not wait *indefinitely* for a
new metric - instead, it will wait up to FLUSH_INTERVAL.  This forces a flush of
existing metrics every FLUSH_INTERVAL, which fixes the behavior of metrics which
depend on repeated flushes, such as the chromite.lib.metrics.Presence context
manager.

BUG= chromium:735698 , chromium:734657 
TEST=Unit tests, which pass.

Change-Id: If96df3e4d7d0d9673efb5377f045c8e45b848695
Reviewed-on: https://chromium-review.googlesource.com/545101
Commit-Ready: Paul Hobbs <phobbs@google.com>
Tested-by: Paul Hobbs <phobbs@google.com>
Reviewed-by: Paul Hobbs <phobbs@google.com>

[modify] https://crrev.com/73a00deaea807968deee0b903c291a8312000a87/lib/ts_mon_config.py
[modify] https://crrev.com/73a00deaea807968deee0b903c291a8312000a87/lib/ts_mon_config_unittest.py

Comment 2 by pho...@chromium.org, Jun 29 2017

Status: Fixed (was: Started)

Comment 3 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)
►

Sign in to add a comment