Yaniv Bronhaim has uploaded a new change for review.
Change subject: Changing threadName when using multiprocessing package ......................................................................
Changing threadName when using multiprocessing package
multiprocessing.manager adds current process name in front of current threadName. This patch changes the name back to Thread-[No].
Change-Id: I541beb63fd5e1a2f7d6eed7f66f4970bf37a6705 Bug-Id: https://bugzilla.redhat.com/show_bug.cgi?id=868663 Signed-off-by: Yaniv Bronhaim ybronhei@redhat.com --- M vdsm/supervdsmServer.py 1 file changed, 3 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/37/9037/1
diff --git a/vdsm/supervdsmServer.py b/vdsm/supervdsmServer.py index a1bf8e6..a5bfc68 100755 --- a/vdsm/supervdsmServer.py +++ b/vdsm/supervdsmServer.py @@ -74,6 +74,9 @@
def wrapper(*args, **kwargs): try: + threadName = threading.current_thread().name + if threadName.find('|') != -1: + _, threading.current_thread().name = threadName.split('|') return func(*args, **kwargs) except: callbackLogger.error("Error in %s", func.__name__, exc_info=True)
-- To view, visit http://gerrit.ovirt.org/9037 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange Gerrit-Change-Id: I541beb63fd5e1a2f7d6eed7f66f4970bf37a6705 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim ybronhei@redhat.com
Yaniv Bronhaim has posted comments on this change.
Change subject: Changing threadName when using multiprocessing package ......................................................................
Patch Set 1: Verified
-- To view, visit http://gerrit.ovirt.org/9037 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I541beb63fd5e1a2f7d6eed7f66f4970bf37a6705 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Yeela Kaplan ykaplan@redhat.com
Shu Ming has posted comments on this change.
Change subject: Changing threadName when using multiprocessing package ......................................................................
Patch Set 1:
Not sure if it is a right fix. I am thinking to decorate the multiprocessing.baseManager class or multiprocessing.BaseProxy class to replace the broken function in them.
-- To view, visit http://gerrit.ovirt.org/9037 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I541beb63fd5e1a2f7d6eed7f66f4970bf37a6705 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Yeela Kaplan ykaplan@redhat.com
Yaniv Bronhaim has posted comments on this change.
Change subject: Changing threadName when using multiprocessing package ......................................................................
Patch Set 1:
for that i need to override the implementation of _connect in ProxyManager. I Think it would be more complex for this log error to create MyBaseProxy with _connect that does the same + what i added to LogDecorator... I'm not sure if it is a better solution.
-- To view, visit http://gerrit.ovirt.org/9037 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I541beb63fd5e1a2f7d6eed7f66f4970bf37a6705 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Yeela Kaplan ykaplan@redhat.com
Dan Kenigsberg has posted comments on this change.
Change subject: Changing threadName when using multiprocessing package ......................................................................
Patch Set 1:
I'm not sure that changing the process name is so pretty, but a problem with your solution is that the pipe-searching is performed on each and every log line.
-- To view, visit http://gerrit.ovirt.org/9037 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I541beb63fd5e1a2f7d6eed7f66f4970bf37a6705 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsland@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Yeela Kaplan ykaplan@redhat.com
Yaniv Bronhaim has posted comments on this change.
Change subject: Changing threadName when using multiprocessing package ......................................................................
Patch Set 1:
Only for functions that use the logDecorator (almost all svdsm api), and it won't run every log print, but every svdsm call before we run the function.
I don't think it's expensive here, print the log is much more complex then that. To calculate thread name before running the function is reasonable
-- To view, visit http://gerrit.ovirt.org/9037 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I541beb63fd5e1a2f7d6eed7f66f4970bf37a6705 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsland@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Yeela Kaplan ykaplan@redhat.com
Zhou Zheng Sheng has posted comments on this change.
Change subject: Changing threadName when using multiprocessing package ......................................................................
Patch Set 1:
Why we want to delete the process name? I think the process name is useful.
-- To view, visit http://gerrit.ovirt.org/9037 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I541beb63fd5e1a2f7d6eed7f66f4970bf37a6705 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsland@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Yeela Kaplan ykaplan@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com
Zhou Zheng Sheng has posted comments on this change.
Change subject: Changing threadName when using multiprocessing package ......................................................................
Patch Set 1:
In the bug report I see part of the reason why need to change the format. Is this disturbing the log analysis tools? Could we consider the "ProcessName|ThreadName" as thread name? I think name like "Thread-198772" is not useful.
-- To view, visit http://gerrit.ovirt.org/9037 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I541beb63fd5e1a2f7d6eed7f66f4970bf37a6705 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsland@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Yeela Kaplan ykaplan@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com
Yaniv Bronhaim has posted comments on this change.
Change subject: Changing threadName when using multiprocessing package ......................................................................
Patch Set 1:
currently for each operation we can grep from vdsm log only the Thread-[no] that related to the operation, and see the thread's steps.
When using supervdsm, that runs as python's multiprocessing.manager, currentThread.Name is changed to MainProcess|Thread-[No] if the Thread is not the "MainThread" of the process, currentProcess.Name stays MainProcess forever so its redundant to add it..
-- To view, visit http://gerrit.ovirt.org/9037 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I541beb63fd5e1a2f7d6eed7f66f4970bf37a6705 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsland@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Yeela Kaplan ykaplan@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com
Saggi Mizrahi has posted comments on this change.
Change subject: Changing threadName when using multiprocessing package ......................................................................
Patch Set 1: Looks good to me, but someone else must approve
-- To view, visit http://gerrit.ovirt.org/9037 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I541beb63fd5e1a2f7d6eed7f66f4970bf37a6705 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsland@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Yeela Kaplan ykaplan@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com
Dan Kenigsberg has posted comments on this change.
Change subject: Changing threadName when using multiprocessing package ......................................................................
Patch Set 1: I would prefer that you didn't submit this
I'm actually surprised with Saggi's ack. Changing a thread-local variable from a logging helper function is not very pretty. Could you think of another solution, such as changing the log format, putting the tweaked threadName in its head?
-- To view, visit http://gerrit.ovirt.org/9037 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I541beb63fd5e1a2f7d6eed7f66f4970bf37a6705 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsland@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Yeela Kaplan ykaplan@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com
Yaniv Bronhaim has posted comments on this change.
Change subject: Changing threadName when using multiprocessing package ......................................................................
Patch Set 1:
The log format puts the threadName in its head already, the multiprocessing implementation modifies the threadName to MainProcess|[threadName] as I explained. So it this way, or override the 'connect' function of manager so that modification won't process.. in my opinion this is the easiest solution and not so bad one.. overriding can be a mess here.. but I can if you prefer
-- To view, visit http://gerrit.ovirt.org/9037 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I541beb63fd5e1a2f7d6eed7f66f4970bf37a6705 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsland@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Yeela Kaplan ykaplan@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com
Dan Kenigsberg has posted comments on this change.
Change subject: Changing threadName when using multiprocessing package ......................................................................
Patch Set 1:
I've suggested to put the thread name verbatim in the format string, not by virtue of %(threadName). This way, you can choose whatever you want to print there, without touching a global variable.
-- To view, visit http://gerrit.ovirt.org/9037 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I541beb63fd5e1a2f7d6eed7f66f4970bf37a6705 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsland@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Yeela Kaplan ykaplan@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com
Yaniv Bronhaim has abandoned this change.
Change subject: Changing threadName when using multiprocessing package ......................................................................
Patch Set 1: Abandoned
wont fix.
-- To view, visit http://gerrit.ovirt.org/9037 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: abandon Gerrit-Change-Id: I541beb63fd5e1a2f7d6eed7f66f4970bf37a6705 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsland@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Yeela Kaplan ykaplan@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com
vdsm-patches@lists.fedorahosted.org