)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"27f0f0cb93fb2a3355c7b6d8c50c9c88c4d90a25","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Always run the rabbitmq heartbeat inside a standard pthread."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The proposed changes will fix related issues who run"},{"line_number":10,"context_line":"heartbeat under mod_wsgi in a monkey patched environment."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Propose changes to always run the rabbitmq health check heartbeat in a"},{"line_number":13,"context_line":"standard python thread."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":13,"id":"7faddb67_3acd5b5d","line":10,"range":{"start_line":10,"start_character":16,"end_line":10,"end_character":24},"updated":"2019-07-04 13:02:22.000000000","message":"You specifically call out mod_wsgi but your comment in the next file says this can affect uWSGI too? Is it just WSGI in general that\u0027s broken? If so, you should update this","commit_id":"82f5a53b40214e120814a5daf2a2af81f2968abc"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"27f0f0cb93fb2a3355c7b6d8c50c9c88c4d90a25","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Always run the rabbitmq heartbeat inside a standard pthread."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The proposed changes will fix related issues who run"},{"line_number":10,"context_line":"heartbeat under mod_wsgi in a monkey patched environment."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Propose changes to always run the rabbitmq health check heartbeat in a"},{"line_number":13,"context_line":"standard python thread."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"We want to isolate the heartbeat execution model from the parent process"},{"line_number":16,"context_line":"inherited execution model."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"We want to force to use the python stdlib threading module to run the"},{"line_number":19,"context_line":"rabbitmq heartbeat."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"To fix this issue we have propose different approaches:"},{"line_number":22,"context_line":"- using a python thread (the changes of this patch set)"},{"line_number":23,"context_line":"- using eventlet tpool execute (patch set 9)"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"The only version which work like expected is this one (import the native python"},{"line_number":26,"context_line":"thread from eventlet)"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"To understand how services consume the oslo.messaging drivers,"},{"line_number":29,"context_line":"and especially how to nova consome the oslo.messaging rabbitmq driver,"},{"line_number":30,"context_line":"please take a look to:"},{"line_number":31,"context_line":"https://github.com/4383/pyamqp-heartbeat/blob/test_debug_env/fakeservice/README.md"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"With this walkthrough you can observe the different the differents steps"},{"line_number":34,"context_line":"used by nova to use oslo.messaging."},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"Also, I would propose to you to compare the different solutions that I\u0027ve"},{"line_number":37,"context_line":"tested and propose to you to observe the behaviour."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":13,"id":"7faddb67_9ab7afd9","line":34,"range":{"start_line":9,"start_character":0,"end_line":34,"end_character":35},"updated":"2019-07-04 13:02:22.000000000","message":"I\u0027m not fully up-to-speed on this issue so I can\u0027t review this properly, however, I find this very confusing and it\u0027s not really helping me understand things. Could you reword this to a format like the below to help me understand what\u0027s happening?\n\n- What the issue is (you\u0027ve talked about monkey patched environment but not what\u0027s doing the monkey patching. I assume it\u0027s eventlet? If so, what\u0027s mod_wsgi got to do with things)\n- What this patch does to fix that issue (it seems to be running the heartbeat in a explicit thread, ensuring it won\u0027t be run in a greenthread if eventlet monkey patches things. Correct?)\n- What the alternatives explored were and why they weren\u0027t chosen/weren\u0027t suitable","commit_id":"82f5a53b40214e120814a5daf2a2af81f2968abc"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"27f0f0cb93fb2a3355c7b6d8c50c9c88c4d90a25","unresolved":false,"context_lines":[{"line_number":36,"context_line":"Also, I would propose to you to compare the different solutions that I\u0027ve"},{"line_number":37,"context_line":"tested and propose to you to observe the behaviour."},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"To do this we will use some of the patches set related to this review."},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"To test the different solution we will play with git review and patches"},{"line_number":42,"context_line":"sets like:"},{"line_number":43,"context_line":"`git review -d 663074,\u003cpatch_set\u003e`"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"But in a first time we will setup a testing environment to reproduce the"},{"line_number":46,"context_line":"issue and observe the patches results."},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"/## First setup the testing env"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"Setup the testing environment:"},{"line_number":51,"context_line":"```"},{"line_number":52,"context_line":"cd /tmp"},{"line_number":53,"context_line":"TEST_DEBUG_ENV\u003d/tmp/663074"},{"line_number":54,"context_line":"mkdir ${TEST_DEBUG_ENV}"},{"line_number":55,"context_line":"cd ${TEST_DEBUG_ENV}"},{"line_number":56,"context_line":"git clone git@github.com:openstack/oslo.messaging.git"},{"line_number":57,"context_line":"cd oslo.messaging"},{"line_number":58,"context_line":"git review -s"},{"line_number":59,"context_line":"cd ${TEST_DEBUG_ENV}"},{"line_number":60,"context_line":"git clone https://github.com/4383/pyamqp-heartbeat/ -b test_debug_env"},{"line_number":61,"context_line":"```"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"Now the testing environment is setup."},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"You need to ensure to have pipenv installed on your laptop:"},{"line_number":66,"context_line":"```"},{"line_number":67,"context_line":"python3 -m pip install --user pipenv"},{"line_number":68,"context_line":"```"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"/## Solution 1 - Using a native python thread"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"We will first check this patch set (the solution that works)(the latest"},{"line_number":73,"context_line":"patch set)."},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"https://review.opendev.org/#/c/663074/"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"How can test these changes by using the following commands:"},{"line_number":78,"context_line":"```"},{"line_number":79,"context_line":"cd ${TEST_DEBUG_ENV}/oslo.messaging"},{"line_number":80,"context_line":"git review -d 663074"},{"line_number":81,"context_line":"cd ${TEST_DEBUG_ENV}/pyamqp-heartbeat"},{"line_number":82,"context_line":"pipenv run ./setup-containers.sh"},{"line_number":83,"context_line":"pipenv run ./start-oslo-mod_wsgi.sh ${TEST_DEBUG_ENV}/oslo.messaging"},{"line_number":84,"context_line":"```"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"/!\\-------------------------------------------------------------------------/!\\"},{"line_number":87,"context_line":"/!\\ For some obscure reasons when we run this container                     /!\\"},{"line_number":88,"context_line":"/!\\ (who emulate the service) in                                            /!\\"},{"line_number":89,"context_line":"/!\\ a tmux, resize a pane or create a new pane kill the container... so for /!\\"},{"line_number":90,"context_line":"/!\\ the next steps you need to create a second terminal to avoid tmux pane  /!\\"},{"line_number":91,"context_line":"/!\\ creation and similar things.                                            /!\\"},{"line_number":92,"context_line":"/!\\-------------------------------------------------------------------------/!\\"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"Your emulated service is now running."},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"The emulate service monkey patch the stdlib at start like nova do the"},{"line_number":97,"context_line":"thing."},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"For further reading about the emulated service and monkey patch:"},{"line_number":100,"context_line":"- https://github.com/4383/pyamqp-heartbeat/blob/test_debug_env/Dockerfile#L13"},{"line_number":101,"context_line":"- https://github.com/4383/pyamqp-heartbeat/blob/test_debug_env/server-eventlet.wsgi#L10"},{"line_number":102,"context_line":"- https://github.com/4383/pyamqp-heartbeat/blob/test_debug_env/server.py#L45"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"Now In your open the rabbit management dashboard and observe the"},{"line_number":105,"context_line":"connections:"},{"line_number":106,"context_line":"http://127.0.0.1:15672/#/connections"},{"line_number":107,"context_line":"user: guest"},{"line_number":108,"context_line":"pwd: guest"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"In a second time you need to open a second terminal where you need to send"},{"line_number":111,"context_line":"a request to this service by using:"},{"line_number":112,"context_line":"```"},{"line_number":113,"context_line":"curl 0.0.0.0:8000"},{"line_number":114,"context_line":"```"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"The service will return \"it\u0027s work\" and you now can observe the"},{"line_number":117,"context_line":"established connections (related to the heartbeat) in the rabbit"},{"line_number":118,"context_line":"dashboard previously opened."},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"You can observe that the heartbeat work without issues and the"},{"line_number":121,"context_line":"connection stay opened. Please observe that during more than 10 minutes."},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"Now you can kill the running process."},{"line_number":124,"context_line":""},{"line_number":125,"context_line":"/## Solution 2 - Using eventlet tpool execute"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"We will now check how the things works if we use the eventlet tpool"},{"line_number":128,"context_line":"execute functionality:"},{"line_number":129,"context_line":"https://eventlet.net/doc/threading.html#eventlet.tpool.execute"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"https://review.opendev.org/#/c/663074/9/"},{"line_number":132,"context_line":""},{"line_number":133,"context_line":"How can test these changes by using the following commands:"},{"line_number":134,"context_line":"```"},{"line_number":135,"context_line":"cd ${TEST_DEBUG_ENV}/oslo.messaging"},{"line_number":136,"context_line":"git review -d 663074,9 # to setup the patch set number 9"},{"line_number":137,"context_line":"cd ${TEST_DEBUG_ENV}/pyamqp-heartbeat"},{"line_number":138,"context_line":"pipenv run ./setup-containers.sh # if rabbit server is not running"},{"line_number":139,"context_line":"pipenv run ./start-oslo-mod_wsgi.sh ${TEST_DEBUG_ENV}/oslo.messaging"},{"line_number":140,"context_line":"```"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"/!\\-------------------------------------------------------------------------/!\\"},{"line_number":143,"context_line":"/!\\ For some obscure reasons when we run this container                     /!\\"},{"line_number":144,"context_line":"/!\\ (who emulate the service) in                                            /!\\"},{"line_number":145,"context_line":"/!\\ a tmux, resize a pane or create a new pane kill the container... so for /!\\"},{"line_number":146,"context_line":"/!\\ the next steps you need to create a second terminal to avoid tmux pane  /!\\"},{"line_number":147,"context_line":"/!\\ creation and similar things.                                            /!\\"},{"line_number":148,"context_line":"/!\\-------------------------------------------------------------------------/!\\"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"Your emulated service is now running."},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"The emulate service monkey patch the stdlib at start like nova do the"},{"line_number":153,"context_line":"thing."},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"For further reading about the emulated service and monkey patch:"},{"line_number":156,"context_line":"- https://github.com/4383/pyamqp-heartbeat/blob/test_debug_env/Dockerfile#L13"},{"line_number":157,"context_line":"- https://github.com/4383/pyamqp-heartbeat/blob/test_debug_env/server-eventlet.wsgi#L10"},{"line_number":158,"context_line":"- https://github.com/4383/pyamqp-heartbeat/blob/test_debug_env/server.py#L45"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"Now In your open the rabbit management dashboard and observe the"},{"line_number":161,"context_line":"connections:"},{"line_number":162,"context_line":"http://127.0.0.1:15672/#/connections"},{"line_number":163,"context_line":"user: guest"},{"line_number":164,"context_line":"pwd: guest"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"In a second time you need to open a second terminal where you need to send"},{"line_number":167,"context_line":"a request to this service by using:"},{"line_number":168,"context_line":"```"},{"line_number":169,"context_line":"curl 0.0.0.0:8000"},{"line_number":170,"context_line":"```"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"The service will return \"it\u0027s work\" and you now can observe the"},{"line_number":173,"context_line":"established connections (related to the heartbeat) in the rabbit"},{"line_number":174,"context_line":"dashboard previously opened."},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"You can observe that the heartbeat work doesn\u0027t stream output to the"},{"line_number":177,"context_line":"terminal."},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"You also can observe on the rabbit dashboard that after some minutes"},{"line_number":180,"context_line":"(3 minutes on my side), all the connections are closed."},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"/## More informations"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"- During all these tests we force to run the heartbeat by using the send"},{"line_number":185,"context_line":"  purpose:"},{"line_number":186,"context_line":"  https://github.com/4383/pyamqp-heartbeat/blob/test_debug_env/server.py#L30"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"See the specifications for more informations:"},{"line_number":189,"context_line":"https://review.opendev.org/661314"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":13,"id":"7faddb67_fac10330","line":186,"range":{"start_line":39,"start_character":0,"end_line":186,"end_character":76},"updated":"2019-07-04 13:02:22.000000000","message":"All of this belongs in a bug report, not the commit message. Because #1826281 is marked as already closed, perhaps we need a new bug report?","commit_id":"82f5a53b40214e120814a5daf2a2af81f2968abc"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b9a3c664c7dfa0fb6801f324a19a2ebe553c7029","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Always run the rabbitmq heartbeat inside a standard pthread."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The proposed changes will fix related issues when we run"},{"line_number":10,"context_line":"heartbeat under apache/httpd enviornment with the apache MPM `prefork`"},{"line_number":11,"context_line":"[1] engine and mod_wsgi or uwsgi in a monkey patched environment."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Propose changes to always run the rabbitmq health check heartbeat in a"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":14,"id":"7faddb67_9ec8f155","line":10,"range":{"start_line":10,"start_character":29,"end_line":10,"end_character":40},"updated":"2019-07-05 16:29:57.000000000","message":"environment","commit_id":"ed7625be665319fc5b608dbb79163fd96d06c39e"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b9a3c664c7dfa0fb6801f324a19a2ebe553c7029","unresolved":false,"context_lines":[{"line_number":53,"context_line":"Possible proposed solutions"},{"line_number":54,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"To fix this issue we have proposed different approaches during discusses [8]:"},{"line_number":57,"context_line":"- using a python thread (the changes of this patch set)"},{"line_number":58,"context_line":"- using eventlet tpool execute (patch set 9)"},{"line_number":59,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":14,"id":"7faddb67_9ecd515f","line":56,"range":{"start_line":56,"start_character":63,"end_line":56,"end_character":72},"updated":"2019-07-05 16:29:57.000000000","message":"discussions","commit_id":"ed7625be665319fc5b608dbb79163fd96d06c39e"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b9a3c664c7dfa0fb6801f324a19a2ebe553c7029","unresolved":false,"context_lines":[{"line_number":54,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"To fix this issue we have proposed different approaches during discusses [8]:"},{"line_number":57,"context_line":"- using a python thread (the changes of this patch set)"},{"line_number":58,"context_line":"- using eventlet tpool execute (patch set 9)"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"The only version which work like expected is this patch set who"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":14,"id":"7faddb67_fe05657b","line":57,"range":{"start_line":57,"start_character":23,"end_line":57,"end_character":55},"updated":"2019-07-05 16:29:57.000000000","message":"(this change)","commit_id":"ed7625be665319fc5b608dbb79163fd96d06c39e"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b9a3c664c7dfa0fb6801f324a19a2ebe553c7029","unresolved":false,"context_lines":[{"line_number":55,"context_line":""},{"line_number":56,"context_line":"To fix this issue we have proposed different approaches during discusses [8]:"},{"line_number":57,"context_line":"- using a python thread (the changes of this patch set)"},{"line_number":58,"context_line":"- using eventlet tpool execute (patch set 9)"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"The only version which work like expected is this patch set who"},{"line_number":61,"context_line":"correspond to use a native python thread and not eventlet."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":14,"id":"7faddb67_5ed7d9ee","line":58,"range":{"start_line":58,"start_character":30,"end_line":58,"end_character":44},"updated":"2019-07-05 16:29:57.000000000","message":"I think this could be dropped or replaced with a web link to the review","commit_id":"ed7625be665319fc5b608dbb79163fd96d06c39e"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b9a3c664c7dfa0fb6801f324a19a2ebe553c7029","unresolved":false,"context_lines":[{"line_number":92,"context_line":"module which support non blocking sockets, use modern kernel features"},{"line_number":93,"context_line":"like epoll through APR [3]."},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"POC"},{"line_number":96,"context_line":"\u003d\u003d\u003d"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"To do this comparison we will use:"},{"line_number":99,"context_line":"- some of the patches set of this review"},{"line_number":100,"context_line":"- the POC section bellow"},{"line_number":101,"context_line":"- and the POC project \u003d\u003e https://github.com/4383/pyamqp-heartbeat/"},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"To test the different solutions we will play with git review and patches"},{"line_number":104,"context_line":"sets by using commands like `git review -d 663074,\u003cpatch_set\u003e`"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"But in a first time we will setup a testing environment to reproduce the"},{"line_number":107,"context_line":"issue and observe the patches results."},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"Setup the testing environment"},{"line_number":110,"context_line":"-----------------------------"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"The testing env is based on https://github.com/4383/pyamqp-heartbeat/"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"It was designed to help to reproduce the issue easily and to reproduce"},{"line_number":115,"context_line":"the original environment as much as possible."},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"So, now setup your env by using:"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"```"},{"line_number":120,"context_line":"cd /tmp"},{"line_number":121,"context_line":"TEST_DEBUG_ENV\u003d/tmp/663074"},{"line_number":122,"context_line":"mkdir ${TEST_DEBUG_ENV}"},{"line_number":123,"context_line":"cd ${TEST_DEBUG_ENV}"},{"line_number":124,"context_line":"git clone git@github.com:openstack/oslo.messaging.git"},{"line_number":125,"context_line":"cd oslo.messaging"},{"line_number":126,"context_line":"git review -s"},{"line_number":127,"context_line":"cd ${TEST_DEBUG_ENV}"},{"line_number":128,"context_line":"git clone https://github.com/4383/pyamqp-heartbeat/"},{"line_number":129,"context_line":"```"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"Now the testing environment is setup."},{"line_number":132,"context_line":""},{"line_number":133,"context_line":"You need to ensure to have pipenv installed on your laptop:"},{"line_number":134,"context_line":"```"},{"line_number":135,"context_line":"python3 -m pip install --user pipenv"},{"line_number":136,"context_line":"```"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"Reproduce the original issue with non patched code"},{"line_number":139,"context_line":"--------------------------------------------------"},{"line_number":140,"context_line":""},{"line_number":141,"context_line":"You can test the original issue with a non patched code by simply switch"},{"line_number":142,"context_line":"to the `master` branch of your oslo.messaging clone that you have"},{"line_number":143,"context_line":"realize during the previous setup step:"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"```"},{"line_number":146,"context_line":"cd ${TEST_DEBUG_ENV}/oslo.messaging"},{"line_number":147,"context_line":"git checkout master"},{"line_number":148,"context_line":"cd ${TEST_DEBUG_ENV}/pyamqp-heartbeat"},{"line_number":149,"context_line":"pipenv run ./setup-containers.sh"},{"line_number":150,"context_line":"pipenv run ./start-oslo-mod_wsgi.sh ${TEST_DEBUG_ENV}/oslo.messaging"},{"line_number":151,"context_line":"```"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"Solution 1 - Using a native python thread"},{"line_number":154,"context_line":"-----------------------------------------"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"We will first check this patch set (the solution that works)(the latest"},{"line_number":157,"context_line":"patch set https://review.opendev.org/#/c/663074/)."},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"How can test these changes by using the following commands:"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"```"},{"line_number":162,"context_line":"cd ${TEST_DEBUG_ENV}/oslo.messaging"},{"line_number":163,"context_line":"git review -d 663074"},{"line_number":164,"context_line":"cd ${TEST_DEBUG_ENV}/pyamqp-heartbeat"},{"line_number":165,"context_line":"pipenv run ./setup-containers.sh"},{"line_number":166,"context_line":"pipenv run ./start-oslo-mod_wsgi.sh ${TEST_DEBUG_ENV}/oslo.messaging"},{"line_number":167,"context_line":"```"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"/!\\-------------------------------------------------------------------------/!\\"},{"line_number":170,"context_line":"/!\\ For some obscure reasons when we run this container                     /!\\"},{"line_number":171,"context_line":"/!\\ (who emulate the service) in  a tmux with have an issue with the        /!\\"},{"line_number":172,"context_line":"/!\\ resize of a tmux pane or when we create a new pane, the both kill       /!\\"},{"line_number":173,"context_line":"/!\\ the container... so for the next steps you need to create a second      /!\\"},{"line_number":174,"context_line":"/!\\ terminal or tmux window to avoid tmux pane creation and similar things. /!\\"},{"line_number":175,"context_line":"/!\\-------------------------------------------------------------------------/!\\"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"Your emulated service is now running."},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"The emulated service have monkey patched the stdlib at start like nova do the"},{"line_number":180,"context_line":"thing."},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"For further reading about the emulated service and monkey patch:"},{"line_number":183,"context_line":"- https://github.com/4383/pyamqp-heartbeat/blob/test_debug_env/Dockerfile#L13"},{"line_number":184,"context_line":"- https://github.com/4383/pyamqp-heartbeat/blob/test_debug_env/server-eventlet.wsgi#L10"},{"line_number":185,"context_line":"- https://github.com/4383/pyamqp-heartbeat/blob/test_debug_env/server.py#L45"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"Now open the rabbit management dashboard and observe the connections:"},{"line_number":188,"context_line":"http://127.0.0.1:15672/#/connections"},{"line_number":189,"context_line":"user: guest"},{"line_number":190,"context_line":"pwd: guest"},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"In a second time you need to open a second terminal like described in"},{"line_number":193,"context_line":"the previous warning, where you need to send a request to"},{"line_number":194,"context_line":"the service by using:"},{"line_number":195,"context_line":"```"},{"line_number":196,"context_line":"curl 0.0.0.0:8000"},{"line_number":197,"context_line":"```"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"The service will return \"it\u0027s work\" and you now can observe the"},{"line_number":200,"context_line":"established connections (related to the heartbeat) in the rabbit"},{"line_number":201,"context_line":"dashboard previously opened."},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"You can observe that the heartbeat work without issues and the"},{"line_number":204,"context_line":"connection stay opened indefinitely."},{"line_number":205,"context_line":"Please observe that during more than 10 minutes."},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"Now you can kill the running process."},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"Solution 2 - Using eventlet tpool execute"},{"line_number":210,"context_line":"-----------------------------------------"},{"line_number":211,"context_line":""},{"line_number":212,"context_line":"We will now check how the things works if we use the eventlet tpool"},{"line_number":213,"context_line":"execute functionality:"},{"line_number":214,"context_line":"https://eventlet.net/doc/threading.html#eventlet.tpool.execute"},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"This test correspond to the patch set https://review.opendev.org/#/c/663074/9/"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"How can test these changes by using the following commands:"},{"line_number":219,"context_line":"```"},{"line_number":220,"context_line":"cd ${TEST_DEBUG_ENV}/oslo.messaging"},{"line_number":221,"context_line":"git review -d 663074,9 # to setup the patch set number 9"},{"line_number":222,"context_line":"cd ${TEST_DEBUG_ENV}/pyamqp-heartbeat"},{"line_number":223,"context_line":"pipenv run ./setup-containers.sh # if rabbit server is not running"},{"line_number":224,"context_line":"pipenv run ./start-oslo-mod_wsgi.sh ${TEST_DEBUG_ENV}/oslo.messaging"},{"line_number":225,"context_line":"```"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"/!\\-------------------------------------------------------------------------/!\\"},{"line_number":228,"context_line":"/!\\ For some obscure reasons when we run this container                     /!\\"},{"line_number":229,"context_line":"/!\\ (who emulate the service) in  a tmux with have an issue with the        /!\\"},{"line_number":230,"context_line":"/!\\ resize of a tmux pane or when we create a new pane, the both kill       /!\\"},{"line_number":231,"context_line":"/!\\ the container... so for the next steps you need to create a second      /!\\"},{"line_number":232,"context_line":"/!\\ terminal or tmux window to avoid tmux pane creation and similar things. /!\\"},{"line_number":233,"context_line":"/!\\-------------------------------------------------------------------------/!\\"},{"line_number":234,"context_line":""},{"line_number":235,"context_line":"Your emulated service is now running."},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"The emulated service have monkey patched the stdlib at start like nova do the"},{"line_number":238,"context_line":"thing."},{"line_number":239,"context_line":""},{"line_number":240,"context_line":"For further reading about the emulated service and monkey patch:"},{"line_number":241,"context_line":"- https://github.com/4383/pyamqp-heartbeat/blob/test_debug_env/Dockerfile#L13"},{"line_number":242,"context_line":"- https://github.com/4383/pyamqp-heartbeat/blob/test_debug_env/server-eventlet.wsgi#L10"},{"line_number":243,"context_line":"- https://github.com/4383/pyamqp-heartbeat/blob/test_debug_env/server.py#L45"},{"line_number":244,"context_line":""},{"line_number":245,"context_line":"Now open the rabbit management dashboard and observe the connections:"},{"line_number":246,"context_line":"http://127.0.0.1:15672/#/connections"},{"line_number":247,"context_line":"user: guest"},{"line_number":248,"context_line":"pwd: guest"},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"In a second time you need to open a second terminal where you need to send"},{"line_number":251,"context_line":"a request to this service by using:"},{"line_number":252,"context_line":"```"},{"line_number":253,"context_line":"curl 0.0.0.0:8000"},{"line_number":254,"context_line":"```"},{"line_number":255,"context_line":""},{"line_number":256,"context_line":"The service will return \"it\u0027s work\" and you now can observe the"},{"line_number":257,"context_line":"established connections (related to the heartbeat) in the rabbit"},{"line_number":258,"context_line":"dashboard previously opened."},{"line_number":259,"context_line":""},{"line_number":260,"context_line":"You can observe on the rabbit dashboard that after some minutes"},{"line_number":261,"context_line":"(3 minutes on my side), all the connections are closed."},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"Conclusion"},{"line_number":264,"context_line":"----------"},{"line_number":265,"context_line":""},{"line_number":266,"context_line":"With standard python thread connections stay opened indefinitely"},{"line_number":267,"context_line":"With tpool connections still to disappears after few minutes like the"},{"line_number":268,"context_line":"original issue."},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"Additional informations"},{"line_number":271,"context_line":"-----------------------"},{"line_number":272,"context_line":""},{"line_number":273,"context_line":"During all these tests:"},{"line_number":274,"context_line":"- we force to run the heartbeat by using the send purpose"},{"line_number":275,"context_line":"  https://github.com/4383/pyamqp-heartbeat/blob/test_debug_env/server.py#L30"},{"line_number":276,"context_line":"- we run oslo.messaging under a monkey patched environment"},{"line_number":277,"context_line":""},{"line_number":278,"context_line":"Further reading"},{"line_number":279,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":14,"id":"7faddb67_fe2a45e5","line":276,"range":{"start_line":95,"start_character":0,"end_line":276,"end_character":58},"updated":"2019-07-05 16:29:57.000000000","message":"I still think all of this should go in the bug report. This is great information but it\u0027s too much for a commit message IMO","commit_id":"ed7625be665319fc5b608dbb79163fd96d06c39e"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b9a3c664c7dfa0fb6801f324a19a2ebe553c7029","unresolved":false,"context_lines":[{"line_number":314,"context_line":""},{"line_number":315,"context_line":"- https://review.opendev.org/661314"},{"line_number":316,"context_line":""},{"line_number":317,"context_line":"Partial-Bug: #1826281"},{"line_number":318,"context_line":""},{"line_number":319,"context_line":"Change-Id: If8846599efc48fe18ecfb99c04e2c38f9a45b9ed"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":14,"id":"7faddb67_3e48ddca","line":317,"range":{"start_line":317,"start_character":0,"end_line":317,"end_character":21},"updated":"2019-07-05 16:29:57.000000000","message":"As before, should we have an entirely new bug for this? If not, why isn\u0027t this \u0027Closes-Bug\u0027?","commit_id":"ed7625be665319fc5b608dbb79163fd96d06c39e"},{"author":{"_account_id":6928,"name":"Ben Nemec","email":"openstack@nemebean.com","username":"bnemec"},"change_message_id":"84608a157db3f5170fba7c9a98a86237b1f6e6c0","unresolved":false,"context_lines":[{"line_number":153,"context_line":""},{"line_number":154,"context_line":"- https://review.opendev.org/661314"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"Closes-Bug: #1826281"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"Change-Id: If8846599efc48fe18ecfb99c04e2c38f9a45b9ed"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":20,"id":"7faddb67_8dc9c226","line":156,"updated":"2019-08-07 14:17:53.000000000","message":"Nit: This bug is already closed. I tend to agree with Stephen that some of the detail in this commit message probably belongs in a bug report.","commit_id":"e589f93ab360ad6dd0ba313c3805b407c1eebd3a"}],"oslo_messaging/_drivers/impl_rabbit.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"27f0f0cb93fb2a3355c7b6d8c50c9c88c4d90a25","unresolved":false,"context_lines":[{"line_number":57,"context_line":"eventlet \u003d importutils.try_import(\u0027eventlet\u0027)"},{"line_number":58,"context_line":"eventlet_patched \u003d eventlet and eventletutils.is_monkey_patched(\"thread\")"},{"line_number":59,"context_line":"if eventlet_patched:"},{"line_number":60,"context_line":"        # We need to make # sure the *native* threading module for initialize"},{"line_number":61,"context_line":"        # the heartbeat thread.."},{"line_number":62,"context_line":"        threading \u003d eventlet.patcher.original(\u0027threading\u0027)"},{"line_number":63,"context_line":"else:"}],"source_content_type":"text/x-python","patch_set":13,"id":"7faddb67_ba0aab87","line":60,"range":{"start_line":60,"start_character":25,"end_line":60,"end_character":27},"updated":"2019-07-04 13:02:22.000000000","message":"x","commit_id":"82f5a53b40214e120814a5daf2a2af81f2968abc"},{"author":{"_account_id":6928,"name":"Ben Nemec","email":"openstack@nemebean.com","username":"bnemec"},"change_message_id":"b4912babfa2ff3c12c3c23726530b4fea2d906bf","unresolved":false,"context_lines":[{"line_number":46,"context_line":"from oslo_messaging import _utils"},{"line_number":47,"context_line":"from oslo_messaging import exceptions"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"import threading"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"# NOTE(sileht): don\u0027t exists in py2 socket module"},{"line_number":52,"context_line":"TCP_USER_TIMEOUT \u003d 18"}],"source_content_type":"text/x-python","patch_set":19,"id":"7faddb67_f5ea4daa","line":49,"updated":"2019-07-24 19:19:06.000000000","message":"Nit: This violates our import grouping recommendations. My comments below might make it kind of a moot point as they would add a bunch of logic here that makes the recommendation somewhat meaningless.","commit_id":"197e4f47af5d0cea7e408ac4136bb18b2f8f1ac8"},{"author":{"_account_id":6928,"name":"Ben Nemec","email":"openstack@nemebean.com","username":"bnemec"},"change_message_id":"b4912babfa2ff3c12c3c23726530b4fea2d906bf","unresolved":false,"context_lines":[{"line_number":474,"context_line":"            # that doesn\u0027t support eventlet without forcing the parent process"},{"line_number":475,"context_line":"            # to stop to use eventlet if they need monkey patching for some"},{"line_number":476,"context_line":"            # specific reasons."},{"line_number":477,"context_line":"            eventlet \u003d importutils.try_import(\u0027eventlet\u0027)"},{"line_number":478,"context_line":"            ispatched \u003d eventlet and eventletutils.is_monkey_patched(\"thread\")"},{"line_number":479,"context_line":"            if ispatched:"},{"line_number":480,"context_line":"                    # If users want to use pthread we need to make sure that we"}],"source_content_type":"text/x-python","patch_set":19,"id":"7faddb67_554ec1c1","line":477,"updated":"2019-07-24 19:19:06.000000000","message":"How often do we create new connections? I\u0027m wondering if we should factor out this code and run it once at import. None of these conditions can change at runtime anyway, and then we don\u0027t have potential issues with running a bunch of import code every time we create a connection.\n\nSince we don\u0027t currently use the global conf object here, maybe we just factor out the import code and only leave the conf check and the module overwrite. Basically, above when we import threading, we also try the eventlet import, check for thread being patched, and then save off the original module to something like stdlib_threading.\n\nThen at connection create time the only thing we have to do is \n\nif self.heartbeat_in_pthread:\n    global threading\n    threading \u003d stdlib_threading\n\nThat should be significantly lower overhead than trying to import eventlet every time.\n\nI realize this may qualify as premature optimization, but given that we\u0027ve had performance issues related to imports in the past and I doubt anyone is going to do a detailed performance analysis before and after this patch, I\u0027d rather hedge our bets. It doesn\u0027t make the code appreciably more complex.","commit_id":"197e4f47af5d0cea7e408ac4136bb18b2f8f1ac8"},{"author":{"_account_id":6928,"name":"Ben Nemec","email":"openstack@nemebean.com","username":"bnemec"},"change_message_id":"84608a157db3f5170fba7c9a98a86237b1f6e6c0","unresolved":false,"context_lines":[{"line_number":49,"context_line":""},{"line_number":50,"context_line":"eventlet \u003d importutils.try_import(\u0027eventlet\u0027)"},{"line_number":51,"context_line":"ispatched \u003d eventlet and eventletutils.is_monkey_patched(\"thread\")"},{"line_number":52,"context_line":"if ispatched:"},{"line_number":53,"context_line":"        # Here we initialize module with the native python threading module"},{"line_number":54,"context_line":"        # if it was already monkey patched by eventlet/greenlet."},{"line_number":55,"context_line":"        stdlib_threading \u003d eventlet.patcher.original(\u0027threading\u0027)"}],"source_content_type":"text/x-python","patch_set":20,"id":"7faddb67_6db566d0","line":52,"range":{"start_line":52,"start_character":3,"end_line":52,"end_character":12},"updated":"2019-08-07 14:17:53.000000000","message":"Nit: The variable doesn\u0027t seem necessary.","commit_id":"e589f93ab360ad6dd0ba313c3805b407c1eebd3a"},{"author":{"_account_id":6928,"name":"Ben Nemec","email":"openstack@nemebean.com","username":"bnemec"},"change_message_id":"84608a157db3f5170fba7c9a98a86237b1f6e6c0","unresolved":false,"context_lines":[{"line_number":50,"context_line":"eventlet \u003d importutils.try_import(\u0027eventlet\u0027)"},{"line_number":51,"context_line":"ispatched \u003d eventlet and eventletutils.is_monkey_patched(\"thread\")"},{"line_number":52,"context_line":"if ispatched:"},{"line_number":53,"context_line":"        # Here we initialize module with the native python threading module"},{"line_number":54,"context_line":"        # if it was already monkey patched by eventlet/greenlet."},{"line_number":55,"context_line":"        stdlib_threading \u003d eventlet.patcher.original(\u0027threading\u0027)"},{"line_number":56,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"7faddb67_2daf6e7e","line":53,"updated":"2019-08-07 14:17:53.000000000","message":"Nit: Extra indentation.","commit_id":"e589f93ab360ad6dd0ba313c3805b407c1eebd3a"},{"author":{"_account_id":6928,"name":"Ben Nemec","email":"openstack@nemebean.com","username":"bnemec"},"change_message_id":"84608a157db3f5170fba7c9a98a86237b1f6e6c0","unresolved":false,"context_lines":[{"line_number":487,"context_line":"            # threading module with the native python threading module"},{"line_number":488,"context_line":"            # if it was already monkey patched by eventlet/greenlet."},{"line_number":489,"context_line":"            global threading"},{"line_number":490,"context_line":"            threading \u003d stdlib_threading"},{"line_number":491,"context_line":""},{"line_number":492,"context_line":"        if self.ssl:"},{"line_number":493,"context_line":"            self.ssl_version \u003d driver_conf.ssl_version"}],"source_content_type":"text/x-python","patch_set":20,"id":"7faddb67_70e69d4d","line":490,"range":{"start_line":490,"start_character":24,"end_line":490,"end_character":40},"updated":"2019-08-07 14:17:53.000000000","message":"If someone were to set this option in a service where eventlet isn\u0027t used, this would fail because we wouldn\u0027t have set stdlib_threading above.\n\nSo I guess I was wrong about refactoring this not making things more complicated. ;-)","commit_id":"e589f93ab360ad6dd0ba313c3805b407c1eebd3a"}],"oslo_messaging/tests/drivers/test_impl_rabbit.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"27f0f0cb93fb2a3355c7b6d8c50c9c88c4d90a25","unresolved":false,"context_lines":[{"line_number":50,"context_line":"    def _do_test_heartbeat_sent(self, fake_ensure_connection,"},{"line_number":51,"context_line":"                                fake_heartbeat_support, fake_heartbeat,"},{"line_number":52,"context_line":"                                fake_logger, heartbeat_side_effect\u003dNone,"},{"line_number":53,"context_line":"                                info\u003dNone):"},{"line_number":54,"context_line":"        event \u003d threading.Event()"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"        def heartbeat_check(rate\u003d2):"}],"source_content_type":"text/x-python","patch_set":13,"id":"7faddb67_daf2079b","line":53,"updated":"2019-07-04 13:02:22.000000000","message":"Unrelated change (the line removal)","commit_id":"82f5a53b40214e120814a5daf2a2af81f2968abc"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"27f0f0cb93fb2a3355c7b6d8c50c9c88c4d90a25","unresolved":false,"context_lines":[{"line_number":1000,"context_line":"class ConnectionLockTestCase(test_utils.BaseTestCase):"},{"line_number":1001,"context_line":"    def _thread(self, lock, sleep, heartbeat\u003dFalse):"},{"line_number":1002,"context_line":"        # NOTE(hberaud): threading module is in use to run the rabbitmq"},{"line_number":1003,"context_line":"        # health check # heartbeat, we want to force to use pthread here"},{"line_number":1004,"context_line":"        # instead of using a monkey patched thread for the heartbeat to avoid"},{"line_number":1005,"context_line":"        # issue related to the parent execution environment"},{"line_number":1006,"context_line":"        # (example: mod_wsgi, # uwsgi). eventlet green threads doesn\u0027t work"}],"source_content_type":"text/x-python","patch_set":13,"id":"7faddb67_faefc3ad","line":1003,"range":{"start_line":1003,"start_character":23,"end_line":1003,"end_character":25},"updated":"2019-07-04 13:02:22.000000000","message":"x","commit_id":"82f5a53b40214e120814a5daf2a2af81f2968abc"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"27f0f0cb93fb2a3355c7b6d8c50c9c88c4d90a25","unresolved":false,"context_lines":[{"line_number":1003,"context_line":"        # health check # heartbeat, we want to force to use pthread here"},{"line_number":1004,"context_line":"        # instead of using a monkey patched thread for the heartbeat to avoid"},{"line_number":1005,"context_line":"        # issue related to the parent execution environment"},{"line_number":1006,"context_line":"        # (example: mod_wsgi, # uwsgi). eventlet green threads doesn\u0027t work"},{"line_number":1007,"context_line":"        # properly in a mod_wsgi environment so if oslo.messaging users"},{"line_number":1008,"context_line":"        # have monkey patch the stdlib"},{"line_number":1009,"context_line":"        # for some reasons and if they run theirs services under mod_wsgi the"}],"source_content_type":"text/x-python","patch_set":13,"id":"7faddb67_9a2a4fe5","line":1006,"range":{"start_line":1006,"start_character":30,"end_line":1006,"end_character":32},"updated":"2019-07-04 13:02:22.000000000","message":"x (looks like your editor is wrapping things properly)","commit_id":"82f5a53b40214e120814a5daf2a2af81f2968abc"}]}