)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"de04fb7edf6edbafea7f0eca661b43716c495d18","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Hervé Beraud \u003chberaud@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2019-10-14 16:34:00 +0200"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Ensure to remove cinder\u0027s outdated leftover lock files"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Cinder to manage locks use fasteners, the fasteners module have a bug"},{"line_number":10,"context_line":"and it doesn\u0027t remove some lock files automatically after usage [1]"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3fa7e38b_9197de45","line":7,"updated":"2019-10-14 15:14:44.000000000","message":"The pattern you use to find lock files limits the search to \"-delete_volume\" locks. This is a worthy subset, but certainly doesn\u0027t cover *all* cinder locks. I think this could be clarified somewhere, perhaps in the commit message. This patch is a partial fix.","commit_id":"018d9cec8638c6eb4782fb91bdc47a5016b00969"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"de04fb7edf6edbafea7f0eca661b43716c495d18","unresolved":false,"context_lines":[{"line_number":24,"context_line":"The added cron job will remove all the lock files last accessed after 7"},{"line_number":25,"context_line":"days to avoid to remove used lock files."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"The lifetime is not configurable. We want to avoid lets user to"},{"line_number":28,"context_line":"configure a lifetime to short and so possibly introduce some race"},{"line_number":29,"context_line":"conditions with cinder."},{"line_number":30,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3fa7e38b_71eee2ed","line":27,"range":{"start_line":27,"start_character":0,"end_line":27,"end_character":32},"updated":"2019-10-14 15:14:44.000000000","message":"But it is configurable, via the \u0027lifetime\u0027 parameter. Perhaps you mean you don\u0027t plan to expose it at a higher level, e.g. as a tripleo parameter?","commit_id":"018d9cec8638c6eb4782fb91bdc47a5016b00969"}],"manifests/cron/fastener_purge.pp":[{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"de04fb7edf6edbafea7f0eca661b43716c495d18","unresolved":false,"context_lines":[{"line_number":1,"context_line":"#"},{"line_number":2,"context_line":"# Copyright (C) 2015 Red Hat Inc."},{"line_number":3,"context_line":"#"},{"line_number":4,"context_line":"# Author: Martin Magr \u003cmmagr@redhat.com\u003e"},{"line_number":5,"context_line":"#"}],"source_content_type":"text/x-puppet","patch_set":1,"id":"3fa7e38b_518f2629","line":2,"updated":"2019-10-14 15:14:44.000000000","message":"2019","commit_id":"018d9cec8638c6eb4782fb91bdc47a5016b00969"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"de04fb7edf6edbafea7f0eca661b43716c495d18","unresolved":false,"context_lines":[{"line_number":62,"context_line":"  package {\u0027find\u0027: }"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"  cron { \u0027cinder fastener cleanup\u0027:"},{"line_number":65,"context_line":"    command     \u003d\u003e \"find /var/lib/cinder/ -atime +${lifetime} -iname cinder-*-delete_volume -exec rm {} \\;\","},{"line_number":66,"context_line":"    environment \u003d\u003e \u0027PATH\u003d/bin:/usr/bin:/usr/sbin SHELL\u003d/bin/sh\u0027,"},{"line_number":67,"context_line":"    user        \u003d\u003e $user,"},{"line_number":68,"context_line":"    minute      \u003d\u003e $minute,"}],"source_content_type":"text/x-puppet","patch_set":1,"id":"3fa7e38b_31162aaa","line":65,"updated":"2019-10-14 15:14:44.000000000","message":"Several comments here:\n\n1) What if system time has not been acquired (i.e. current time is near the UNIX epoch)?\n\n2) Consider \"-delete\" instead of \"-exec rm {}\" (no need for a subprocess)\n\n3) Because the locks are all in the root of cinder\u0027s service directory, the query could use mindepth/maxdepth to prevent descending into subdirectories (for safety)","commit_id":"018d9cec8638c6eb4782fb91bdc47a5016b00969"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"12612de31e1aa6f6d4b12d470090a0be09d4720d","unresolved":false,"context_lines":[{"line_number":62,"context_line":"  package {\u0027find\u0027: }"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"  cron { \u0027cinder fastener cleanup\u0027:"},{"line_number":65,"context_line":"    command     \u003d\u003e \u0027find /var/lib/cinder/ -atime +${lifetime} -iname \"cinder-*-delete_volume\" -delete -maxdepth 0\u0027,"},{"line_number":66,"context_line":"    environment \u003d\u003e \u0027PATH\u003d/bin:/usr/bin:/usr/sbin SHELL\u003d/bin/sh\u0027,"},{"line_number":67,"context_line":"    user        \u003d\u003e $user,"},{"line_number":68,"context_line":"    minute      \u003d\u003e $minute,"}],"source_content_type":"text/x-puppet","patch_set":3,"id":"3fa7e38b_dc000a37","line":65,"range":{"start_line":65,"start_character":112,"end_line":65,"end_character":113},"updated":"2019-10-14 18:00:22.000000000","message":"The maxdepth (and mindepth) options aren\u0027t intuitive, and I think a maxdepth value of 0 means the command won\u0027t even step into /var/lib/cinder. I did some experiments and think you want \"-maxdepth 1\" (you should confirm).","commit_id":"32423741b068908f8e8fc3030da4b984d4149649"},{"author":{"_account_id":28223,"name":"Cedric Jeanneret","display_name":"cjeanner (Tengu)","email":"cjeanner@redhat.com","username":"cjeanner"},"change_message_id":"afd42c321576229ba25c554624601b125a0558a1","unresolved":false,"context_lines":[{"line_number":62,"context_line":"  package {\u0027find\u0027: }"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"  cron { \u0027cinder fastener cleanup\u0027:"},{"line_number":65,"context_line":"    command     \u003d\u003e \u0027find /var/lib/cinder/ -atime +${lifetime} -iname \"cinder-*-delete_volume\" -delete -maxdepth 0\u0027,"},{"line_number":66,"context_line":"    environment \u003d\u003e \u0027PATH\u003d/bin:/usr/bin:/usr/sbin SHELL\u003d/bin/sh\u0027,"},{"line_number":67,"context_line":"    user        \u003d\u003e $user,"},{"line_number":68,"context_line":"    minute      \u003d\u003e $minute,"}],"source_content_type":"text/x-puppet","patch_set":3,"id":"3fa7e38b_020b948d","line":65,"range":{"start_line":65,"start_character":112,"end_line":65,"end_character":113},"in_reply_to":"3fa7e38b_dc000a37","updated":"2019-10-15 06:53:58.000000000","message":"I think you\u0027re right, Alan - I\u0027m currently deploying the patchset on my lab in order to do some tests.","commit_id":"32423741b068908f8e8fc3030da4b984d4149649"},{"author":{"_account_id":16137,"name":"Tobias Urdin","email":"tobias.urdin@binero.com","username":"tobasco"},"change_message_id":"a181be4cf636c21888f8ffb8c3cc68d121071f22","unresolved":false,"context_lines":[{"line_number":59,"context_line":""},{"line_number":60,"context_line":"  include ::cinder::deps"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"  package {\u0027find\u0027: }"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"  cron { \u0027cinder fastener cleanup\u0027:"},{"line_number":65,"context_line":"    command     \u003d\u003e \u0027find /var/lib/cinder/ -maxdepth 1 -atime +${lifetime} -iname \"cinder-*-delete_volume\" -delete\u0027,"}],"source_content_type":"text/x-puppet","patch_set":4,"id":"3fa7e38b_1dbd3501","line":62,"range":{"start_line":62,"start_character":2,"end_line":62,"end_character":20},"updated":"2019-10-15 07:21:56.000000000","message":"Also, can\u0027t we just assume that find is available by default on all distributions that we are testing, or is this false for some distro?","commit_id":"6d0c35560fc11d25cc9e8232697d98604d312aab"}]}