Nir Soffer has posted comments on this change. Change subject: rpc: Log important info from VM stats ......................................................................
Patch Set 19: (2 comments) https://gerrit.ovirt.org/#/c/58465/19/lib/vdsm/throttledlog.py File lib/vdsm/throttledlog.py: Line 39: self._timeout = timeout Line 40: self._counter = 0 Line 41: self._last_time = 0 Line 42: Line 43: def should_log(self): One issue with this, is having a predicate with side effect, which is confusing. We can solve this by implementing log() instead. Line 44: now = monotonic_time() Line 45: result = self._should_log(now) Line 46: self._counter = (self._counter + 1) % self._interval Line 47: self._last_time = now Line 100: else: Line 101: if not periodic.should_log(): Line 102: return Line 103: Line 104: _logger.log(level, message, *args) I think my advice on the previous patch was bad, now we have a predicate with side effect. But we can do: try: periodic = _periodic[name] except KeyError: # Unthrottled _logger.log(level, message, *args) else: periodic.log(level, message, *args) -- To view, visit https://gerrit.ovirt.org/58465 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifcbac615323b62fb9a27e5c0f5a4e98990076146 Gerrit-PatchSet: 19 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal <mzama...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek <michal.skriva...@redhat.com> Gerrit-Reviewer: Milan Zamazal <mzama...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczew...@gmail.com> Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
