oslo.messaging/4bf62ed397b1a566864b64727ed...

62 lines
3.0 KiB
Plaintext

{
"comments": [
{
"key": {
"uuid": "0c9a648b_6adff13b",
"filename": "oslo_messaging/metrics/client.py",
"patchSetId": 10
},
"lineNbr": 62,
"author": {
"id": 28522
},
"writtenOn": "2021-05-06T09:35:20Z",
"side": 1,
"message": "Hello,\n\nI think that we need to be cautious here (with threading).\n\nWhen there\u0027s no active request, the worker process gets parked, so, it doesn\u0027t run the thread and we should start observing related issues with the metric collector.\n\nWhen we don\u0027t use a native python thread then process become idle due to green thread/eventlet.\nmod_wsgi and green thread aren\u0027t really compatible, it\u0027s just due to the way mod_wsgi handles processes.\n\nFew years ago we experienced similar issues [1] with the rabbitmq heartbeat, and I think that here we are more or less in a similar context.\n\nThat can be easily fixed by using a python thread by default and all the time (e.g [2][3]).\n\nI think that we need to follow the same path here and applying a native python thread to all the threads used here.\n\n[1] https://opendev.org/openstack/oslo.messaging/commit/22f240b82fffbd62be8568a7d0d3369134596ace\n[2] https://opendev.org/openstack/oslo.messaging/src/branch/master/oslo_messaging/_drivers/impl_rabbit.py#L49-L59\n[3] https://opendev.org/openstack/oslo.messaging/src/branch/master/oslo_messaging/_drivers/impl_rabbit.py#L992",
"range": {
"startLine": 62,
"startChar": 27,
"endLine": 62,
"endChar": 66
},
"revId": "4bf62ed397b1a566864b64727ed9abdd442c0fb1",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": true
},
{
"key": {
"uuid": "da5d0a0e_fd4fb684",
"filename": "oslo_messaging/rpc/client.py",
"patchSetId": 10
},
"lineNbr": 152,
"author": {
"id": 8770
},
"writtenOn": "2021-03-15T15:36:22Z",
"side": 1,
"message": "This start()/end() pattern would be better implemented using a content manager: https://docs.python.org/3/reference/compound_stmts.html#the-with-statement\n\nUsing a context manager should allow you to move all this logic into the the context itself - including the time() tracking - rather than duplicating the code here and in the call() function.\n\n(That\u0027s my assumption, totally untested).",
"revId": "4bf62ed397b1a566864b64727ed9abdd442c0fb1",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": true
},
{
"key": {
"uuid": "e6db507e_5e13adae",
"filename": "oslo_messaging/rpc/client.py",
"patchSetId": 10
},
"lineNbr": 152,
"author": {
"id": 28522
},
"writtenOn": "2021-05-06T09:35:20Z",
"side": 1,
"message": "I agree with Ken that will be more appropriate.",
"parentUuid": "da5d0a0e_fd4fb684",
"revId": "4bf62ed397b1a566864b64727ed9abdd442c0fb1",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": true
}
]
}