Dan Kenigsberg has posted comments on this change. Change subject: Related to BZ#738120 - Logging in OOP. ......................................................................
Patch Set 6: I would prefer that you didn't submit this (2 inline comments) .................................................... File vdsm/storage/processPool.py Line 247: Line 248: # Add logger handler (via Queue) Line 249: hdlr = QueueHandler(logQueue) Line 250: hdlr.setLevel(_log.getEffectiveLevel()) Line 251: logging.root.addHandler(hdlr) looking at its code, it seems that addHandler could hang in the child process, if logging._lock is already taken. Maybe we should override logging._lock = logging.RLock() before calling this? Line 252: for log in logging.Logger.manager.loggerDict.values(): Line 253: if hasattr(log, 'handlers'): log.handlers.append(hdlr) Line 254: Line 255: poller = select.poll() Line 249: hdlr = QueueHandler(logQueue) Line 250: hdlr.setLevel(_log.getEffectiveLevel()) Line 251: logging.root.addHandler(hdlr) Line 252: for log in logging.Logger.manager.loggerDict.values(): Line 253: if hasattr(log, 'handlers'): log.handlers.append(hdlr) maybe, just maybe, the same log appears under 3 keys in loggerDict and log.handlers.addHandler(hdlr) would fix the triple log issue? HOWEVER see former comment about locking. Line 254: Line 255: poller = select.poll() Line 256: poller.register(lifeLine, 0) # Only SIGERR\SIGHUP Line 257: poller.register(pipe.fileno(), select.EPOLLIN | select.EPOLLPRI) -- To view, visit http://gerrit.usersys.redhat.com/985 To unsubscribe, visit http://gerrit.usersys.redhat.com/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iabe8abf31f46469e3c6fdac670f0ff03fe9cefe8 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Igor Lvovsky <[email protected]> Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: David Naori <[email protected]> Gerrit-Reviewer: Igor Lvovsky <[email protected]> Gerrit-Reviewer: Saggi Mizrahi <[email protected]> _______________________________________________ vdsm-patches mailing list [email protected] https://fedorahosted.org/mailman/listinfo/vdsm-patches
