Yaniv Bronhaim has posted comments on this change. Change subject: AdvancedStatsThread: Throttle duplicated exception log ......................................................................
Patch Set 1: (3 inline comments) .................................................... Commit Message Line 14: This patch proposes a way to throttle duplicated exception log in stats Line 15: functions. For the same exception in the same stats function, if it is Line 16: raised too many times, just suppress the log. Line 17: Line 18: Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=853725 The exception we see in this bug is libvirtError that is raised (Good reason to print its traceback every time it happens.. we don't want to continue without handling this bug). Usually sampling methods don't throw exceptions, if exception is thrown its a bug and we want to log it. The exceptions can be raised in different times and not related to each other, even if the exception was raised from the same sample function. Here you swallow this kind of exceptions. But I like the idea of this function and I think it should be part of utils.py, and not for exceptions, maybe for duplicate logs, like "Disk hdc stats not available" that continues until connectivity to storage is back... exceptions should be print because they point on a serious failure. Line 19: Line 20: Change-Id: I2ee04d8d82e2a14b0a003627981c28e5e64a46ab .................................................... File vdsm/utils.py Line 436: statsFunction() Line 437: except Exception, e: Line 438: if not self.handleStatsException(e): Line 439: if not self._logThrottle.needSuppress( Line 440: statsFunction, e): you don't have to implement such a logger, you just grep the right data from the current log and I think that its enough Line 441: self._log.error("Stats function failed: %s", Line 442: statsFunction, exc_info=True) Line 443: Line 444: self._stopEvent.wait(waitInterval) Line 439: if not self._logThrottle.needSuppress( Line 440: statsFunction, e): Line 441: self._log.error("Stats function failed: %s", Line 442: statsFunction, exc_info=True) Line 443: This handling means stats function failed, something wrong happened, for example, it won't be raise when we are disconnected from storage.. disconnection from storage won't throw exception but report right warning to log inside the sample function implementation Line 444: self._stopEvent.wait(waitInterval) Line 445: intervalAccum = (intervalAccum + waitInterval) % maxInterval Line 446: Line 447: class StatsThread(threading.Thread): -- To view, visit http://gerrit.ovirt.org/8884 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2ee04d8d82e2a14b0a003627981c28e5e64a46ab Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Gal Hammer <[email protected]> Gerrit-Reviewer: Mark Wu <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
