)]}'
{"specs/juno/add-nodelocked-retry.rst":[{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"6b3e8817668559744f31e5124389f277ee157f8b","unresolved":false,"context_lines":[{"line_number":62,"context_line":"It is unlikely that spec will be approved in Juno because of the work that"},{"line_number":63,"context_line":"it entails to change the APIs. This proposal is simple and can be implemented"},{"line_number":64,"context_line":"quickly."},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"Data model impact"},{"line_number":67,"context_line":"-----------------"},{"line_number":68,"context_line":""}],"source_content_type":"text/x-rst","patch_set":4,"id":"baada198_804b5386","line":65,"updated":"2014-07-02 08:00:07.000000000","message":"Another alternative would be to implement this retry within the API service itself,\neg. as a decorator on RPCAPI methods. Off-hand, I can think of two tradeoffs:\n\n- pro: no additional lock holding within greenthreads on the conductor\n- con: more RPC bus traffic\n\nAnother alternative would be for locks to indicate some expectation of the duration.\nWhile that might be a much larger change, it\u0027s worth mentioning it here, I think.","commit_id":"c435a90bccd748ba00ce84f1e3a0316ba5c6dec4"},{"author":{"_account_id":3099,"name":"David Shrewsbury","email":"dshrewsb@redhat.com","username":"dshrews"},"change_message_id":"6e6c638416b69f68368b2c127ed88821e6bb7dd5","unresolved":false,"context_lines":[{"line_number":62,"context_line":"It is unlikely that spec will be approved in Juno because of the work that"},{"line_number":63,"context_line":"it entails to change the APIs. This proposal is simple and can be implemented"},{"line_number":64,"context_line":"quickly."},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"Data model impact"},{"line_number":67,"context_line":"-----------------"},{"line_number":68,"context_line":""}],"source_content_type":"text/x-rst","patch_set":4,"id":"baada198_09122bb4","line":65,"in_reply_to":"baada198_804b5386","updated":"2014-07-09 14:09:20.000000000","message":"@Devananda: \"no additional lock holding\" ... Confused by that statement. Are there other locks that are grabbed before attempting to get the node lock?","commit_id":"c435a90bccd748ba00ce84f1e3a0316ba5c6dec4"},{"author":{"_account_id":11076,"name":"Shivanand Tendulker","email":"stendulker@gmail.com","username":"stendulker"},"change_message_id":"650924baf69be9a113bf2b7bdd1aab9d27ee854d","unresolved":false,"context_lines":[{"line_number":92,"context_line":"---------------------"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"The only impact this will have on the user is the reduced amount of node lock"},{"line_number":95,"context_line":"errors from API requests."},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"Scalability impact"},{"line_number":98,"context_line":"------------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"baada198_da93a14d","line":95,"updated":"2014-07-09 05:46:00.000000000","message":"Given that  the multiple management operation could be potentially invoked mostly in the test environment.  \nNormally users would wait for the management operation to get completed before invoking next one in real deploy scenario. \nCan this change be restricted to be enabled only during testing?\n\nOther usecase could be that user wants to invoke all the operations wherein lock is taken at once and would want them to finish one after another (order is not significant). In this case user may want to set the retry as \u0027forever\u0027 till operation is complete. It could be a way of scheduling.","commit_id":"c435a90bccd748ba00ce84f1e3a0316ba5c6dec4"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"6b3e8817668559744f31e5124389f277ee157f8b","unresolved":false,"context_lines":[{"line_number":92,"context_line":"---------------------"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"The only impact this will have on the user is the reduced amount of node lock"},{"line_number":95,"context_line":"errors from API requests."},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"Scalability impact"},{"line_number":98,"context_line":"------------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"baada198_a04e9794","line":95,"updated":"2014-07-02 08:00:07.000000000","message":"Won\u0027t it also potentially delay API responses, eg. instead of getting 404 NodeLocked quickly,\nthe API response will be delayed by the retries without any guarantee of success?","commit_id":"c435a90bccd748ba00ce84f1e3a0316ba5c6dec4"},{"author":{"_account_id":3099,"name":"David Shrewsbury","email":"dshrewsb@redhat.com","username":"dshrews"},"change_message_id":"6e6c638416b69f68368b2c127ed88821e6bb7dd5","unresolved":false,"context_lines":[{"line_number":92,"context_line":"---------------------"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"The only impact this will have on the user is the reduced amount of node lock"},{"line_number":95,"context_line":"errors from API requests."},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"Scalability impact"},{"line_number":98,"context_line":"------------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"baada198_e9725748","line":95,"in_reply_to":"baada198_a04e9794","updated":"2014-07-09 14:09:20.000000000","message":"@Devananda: True. I mention this in Performance Impact section, but probably worth noting here as well.","commit_id":"c435a90bccd748ba00ce84f1e3a0316ba5c6dec4"},{"author":{"_account_id":3099,"name":"David Shrewsbury","email":"dshrewsb@redhat.com","username":"dshrews"},"change_message_id":"6e6c638416b69f68368b2c127ed88821e6bb7dd5","unresolved":false,"context_lines":[{"line_number":92,"context_line":"---------------------"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"The only impact this will have on the user is the reduced amount of node lock"},{"line_number":95,"context_line":"errors from API requests."},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"Scalability impact"},{"line_number":98,"context_line":"------------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"baada198_2909af95","line":95,"in_reply_to":"baada198_da93a14d","updated":"2014-07-09 14:09:20.000000000","message":"@Shivanand: We could control whether or not the retry is enabled with the proposed config variables. We could set these appropriately within devstack to affect testing.","commit_id":"c435a90bccd748ba00ce84f1e3a0316ba5c6dec4"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"fd32ef757ccdbe5fa036410ca625afdd7f578e10","unresolved":false,"context_lines":[{"line_number":92,"context_line":"---------------------"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"The only impact this will have on the user is the reduced amount of node lock"},{"line_number":95,"context_line":"errors from API requests."},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"Scalability impact"},{"line_number":98,"context_line":"------------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"baada198_b076f704","line":95,"in_reply_to":"baada198_da93a14d","updated":"2014-07-09 17:44:15.000000000","message":"Using different configuration in test vs. deployment environments is bad -- not testing your production configuration will introduce bugs in production.\n\nThe source of the problem isn\u0027t invoking multiple management operations in parallel -- it\u0027s the internal periodic-task conflicting with a single management operation. This is just as likely in the upstream test environment as it is in a production environment.\n\nAlso, infinite retry does not make sense at this layer -- there needs to be a response from the conductor to the API service, so that it can give a response to the HTTP request. Thus, the total time for the conductor to process the request and pass the reply back across the RPC bus must be less than the HTTP timeout.","commit_id":"c435a90bccd748ba00ce84f1e3a0316ba5c6dec4"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"0ef41d87e61887c099c694d6b55d52035d43eefe","unresolved":false,"context_lines":[{"line_number":121,"context_line":"doing exponential backoff)."},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"Hard coded values for retry attempts and time between attempts would"},{"line_number":124,"context_line":"create no deployer impact."},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"Developer impact"},{"line_number":127,"context_line":"----------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"baada198_6097d848","line":124,"updated":"2014-07-03 16:54:50.000000000","message":"Please don\u0027t hard-code. You never know when you might want to change the values. Having config options would also allow for turning off the retry (or ugh, retrying forever).","commit_id":"c435a90bccd748ba00ce84f1e3a0316ba5c6dec4"},{"author":{"_account_id":3099,"name":"David Shrewsbury","email":"dshrewsb@redhat.com","username":"dshrews"},"change_message_id":"6e6c638416b69f68368b2c127ed88821e6bb7dd5","unresolved":false,"context_lines":[{"line_number":121,"context_line":"doing exponential backoff)."},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"Hard coded values for retry attempts and time between attempts would"},{"line_number":124,"context_line":"create no deployer impact."},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"Developer impact"},{"line_number":127,"context_line":"----------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"baada198_a9f4df87","line":124,"in_reply_to":"baada198_6097d848","updated":"2014-07-09 14:09:20.000000000","message":"@Ruby: Agreed. Just throwing the alternative out there.","commit_id":"c435a90bccd748ba00ce84f1e3a0316ba5c6dec4"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"164abe3887604c5a36c918362b06ec8067d92e3a","unresolved":false,"context_lines":[{"line_number":28,"context_line":""},{"line_number":29,"context_line":"* Almost all of the REST API calls lock the node in question except"},{"line_number":30,"context_line":"  for: change_node_maintenance_mode, driver_vendor_passthru,"},{"line_number":31,"context_line":"  get_console_information and validate_driver_interfaces."},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"* The conductor periodic task to synchronize power states."},{"line_number":34,"context_line":""}],"source_content_type":"text/x-rst","patch_set":5,"id":"baada198_34299e37","line":31,"updated":"2014-07-11 17:30:30.000000000","message":"hmm... that\u0027s not very accurate, I think that none of the GET calls and POST (for resource creation) faces that problem.","commit_id":"ef579a154d9f80ae1d5bcdc9cd5159214f32410d"},{"author":{"_account_id":3099,"name":"David Shrewsbury","email":"dshrewsb@redhat.com","username":"dshrews"},"change_message_id":"5f1f9354cb3809f9da19c643e920995d36ead206","unresolved":false,"context_lines":[{"line_number":28,"context_line":""},{"line_number":29,"context_line":"* Almost all of the REST API calls lock the node in question except"},{"line_number":30,"context_line":"  for: change_node_maintenance_mode, driver_vendor_passthru,"},{"line_number":31,"context_line":"  get_console_information and validate_driver_interfaces."},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"* The conductor periodic task to synchronize power states."},{"line_number":34,"context_line":""}],"source_content_type":"text/x-rst","patch_set":5,"id":"baada198_ebde32d9","line":31,"in_reply_to":"baada198_34299e37","updated":"2014-07-14 15:19:09.000000000","message":"Yes, you are correct. I really meant RPC (not REST) API calls here. Thanks for pointing this out. Clarified in the next patch set.","commit_id":"ef579a154d9f80ae1d5bcdc9cd5159214f32410d"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"164abe3887604c5a36c918362b06ec8067d92e3a","unresolved":false,"context_lines":[{"line_number":34,"context_line":""},{"line_number":35,"context_line":"* The conductor periodic task to check deploy timeouts."},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"The amount of time the node locks are held are typically not long."},{"line_number":38,"context_line":"We could eliminate many of these errors by simply retrying the lock"},{"line_number":39,"context_line":"attempt. Admittedly, this would not *totally* eliminate the problem,"},{"line_number":40,"context_line":"but it would make it much less likely to occur for clients, and thus"}],"source_content_type":"text/x-rst","patch_set":5,"id":"baada198_b4eccebc","line":37,"updated":"2014-07-11 17:30:30.000000000","message":"[OFF-TOPIC]: One of them may take a good time to run per node which is the getting sensor data stuff[1], this will run as a periodic task and will take a couple of seconds to run on each node.\n\n[1] https://github.com/openstack/ironic-specs/blob/master/specs/juno/send-data-to-ceilometer.rst","commit_id":"ef579a154d9f80ae1d5bcdc9cd5159214f32410d"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"164abe3887604c5a36c918362b06ec8067d92e3a","unresolved":false,"context_lines":[{"line_number":132,"context_line":"maximum retry attempts and time in between attempts (or values for"},{"line_number":133,"context_line":"doing exponential backoff). Using sane defaults (setting the values"},{"line_number":134,"context_line":"such that no retry attempts are performed) should help alleviate"},{"line_number":135,"context_line":"much of this impact."},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"Developer impact"},{"line_number":138,"context_line":"----------------"}],"source_content_type":"text/x-rst","patch_set":5,"id":"baada198_cf4e0f6f","line":135,"updated":"2014-07-11 17:30:30.000000000","message":"That sounds correct but in this section we have to say what are the config options, are they part of a group? What are sane defaults for it? See [1]\n\nBecause I think we just raised a question here, which type of retrying we are doing? Fixed number of attempts with a fixed interval between them? exponential backoff?\n\n[1] https://github.com/devananda/ironic-specs/blob/master/specs/template.rst#other-deployer-impact","commit_id":"ef579a154d9f80ae1d5bcdc9cd5159214f32410d"},{"author":{"_account_id":3099,"name":"David Shrewsbury","email":"dshrewsb@redhat.com","username":"dshrews"},"change_message_id":"5f1f9354cb3809f9da19c643e920995d36ead206","unresolved":false,"context_lines":[{"line_number":132,"context_line":"maximum retry attempts and time in between attempts (or values for"},{"line_number":133,"context_line":"doing exponential backoff). Using sane defaults (setting the values"},{"line_number":134,"context_line":"such that no retry attempts are performed) should help alleviate"},{"line_number":135,"context_line":"much of this impact."},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"Developer impact"},{"line_number":138,"context_line":"----------------"}],"source_content_type":"text/x-rst","patch_set":5,"id":"baada198_c1bced08","line":135,"in_reply_to":"baada198_cf4e0f6f","updated":"2014-07-14 15:19:09.000000000","message":"Good to be explicit, I guess. Added these options to the spec and removed the bit about exponential backoff as I think fixed attempts is better. Thx. :)","commit_id":"ef579a154d9f80ae1d5bcdc9cd5159214f32410d"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"164abe3887604c5a36c918362b06ec8067d92e3a","unresolved":false,"context_lines":[{"line_number":161,"context_line":"encasulates all of the logic we would want. Version 1.2.2 (the latest"},{"line_number":162,"context_line":"release as of this writing) would be the minimum version we would want"},{"line_number":163,"context_line":"since it contains an important bug fix related to retrying on certain"},{"line_number":164,"context_line":"exceptions."},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"Testing"},{"line_number":167,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":5,"id":"baada198_8f5257dc","line":164,"updated":"2014-07-11 17:30:30.000000000","message":"Right so we need a patch to bump the version of the global-requirements that right now is 1.2.1[1]\n\n[1] https://github.com/openstack/requirements/blob/master/global-requirements.txt#L181","commit_id":"ef579a154d9f80ae1d5bcdc9cd5159214f32410d"},{"author":{"_account_id":3099,"name":"David Shrewsbury","email":"dshrewsb@redhat.com","username":"dshrews"},"change_message_id":"5f1f9354cb3809f9da19c643e920995d36ead206","unresolved":false,"context_lines":[{"line_number":161,"context_line":"encasulates all of the logic we would want. Version 1.2.2 (the latest"},{"line_number":162,"context_line":"release as of this writing) would be the minimum version we would want"},{"line_number":163,"context_line":"since it contains an important bug fix related to retrying on certain"},{"line_number":164,"context_line":"exceptions."},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"Testing"},{"line_number":167,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":5,"id":"baada198_c1014db0","line":164,"in_reply_to":"baada198_8f5257dc","updated":"2014-07-14 15:19:09.000000000","message":"Right. I\u0027ve now noted this in the spec. Thx.  :)","commit_id":"ef579a154d9f80ae1d5bcdc9cd5159214f32410d"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"01343e9e97d3d81e674cc9831aa2fdedbe743a3d","unresolved":false,"context_lines":[{"line_number":137,"context_line":"The following new configuration variables are proposed (and their default"},{"line_number":138,"context_line":"values) to be added to the conductor variable group:"},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"* node_locked_retry_attempts \u003d 0        (default to no retry attempts)"},{"line_number":141,"context_line":"* node_locked_retry_interval_ms \u003d 1000  (default to 1s between attempts)"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"The default for node_locked_retry_attempts will be zero, which will duplicate"}],"source_content_type":"text/x-rst","patch_set":6,"id":"baada198_250e923b","line":140,"updated":"2014-07-14 16:55:25.000000000","message":"I agree with @Dmitry on having this enabled by default\n\nAnd as a sane default I think that 3 attempts should be a reasonable starting point","commit_id":"58e24fcf1880d7ad054bfa5c62b6f251bba5bbe6"},{"author":{"_account_id":3099,"name":"David Shrewsbury","email":"dshrewsb@redhat.com","username":"dshrews"},"change_message_id":"39f9d3fbba17387d4d30500b1f2c9d22306036df","unresolved":false,"context_lines":[{"line_number":137,"context_line":"The following new configuration variables are proposed (and their default"},{"line_number":138,"context_line":"values) to be added to the conductor variable group:"},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"* node_locked_retry_attempts \u003d 0        (default to no retry attempts)"},{"line_number":141,"context_line":"* node_locked_retry_interval_ms \u003d 1000  (default to 1s between attempts)"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"The default for node_locked_retry_attempts will be zero, which will duplicate"}],"source_content_type":"text/x-rst","patch_set":6,"id":"baada198_d7dbc44c","line":140,"in_reply_to":"baada198_250e923b","updated":"2014-07-14 19:22:23.000000000","message":"Done","commit_id":"58e24fcf1880d7ad054bfa5c62b6f251bba5bbe6"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"01343e9e97d3d81e674cc9831aa2fdedbe743a3d","unresolved":false,"context_lines":[{"line_number":144,"context_line":"current behavior and produce no impact on response time."},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"The value for node_locked_retry_interval_ms (specified in milliseconds to match"},{"line_number":147,"context_line":"the units used in the retrying [3] module) will be 1000ms. This matches the"},{"line_number":148,"context_line":"default value used for the retrying module. (NOTE: The documentation for"},{"line_number":149,"context_line":"retrying says that by default it retries \"as fast as possible\", but an"},{"line_number":150,"context_line":"inspection of the code reveals 1000ms is the default)."}],"source_content_type":"text/x-rst","patch_set":6,"id":"baada198_2874a966","line":147,"updated":"2014-07-14 16:55:25.000000000","message":"Hmm I don\u0027t know whether milliseconds would be a good unit for it, we have other configs for intervals and they are all using seconds. Also, usually nodes gets blocked for operations where it needs to poke the BMC (syncing power states) so it\u0027s very likely that those operations will take a few seconds to run.\n\nI\u0027d like to see others feedback on that, but IMO I think it\u0027s less confusing to have it in seconds.\n\nWe could convert it internally to work fine with retrying, e.g:\n\n @retry(interval\u003dCONF.node_locked_retry_interval * 1000)","commit_id":"58e24fcf1880d7ad054bfa5c62b6f251bba5bbe6"},{"author":{"_account_id":3099,"name":"David Shrewsbury","email":"dshrewsb@redhat.com","username":"dshrews"},"change_message_id":"39f9d3fbba17387d4d30500b1f2c9d22306036df","unresolved":false,"context_lines":[{"line_number":144,"context_line":"current behavior and produce no impact on response time."},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"The value for node_locked_retry_interval_ms (specified in milliseconds to match"},{"line_number":147,"context_line":"the units used in the retrying [3] module) will be 1000ms. This matches the"},{"line_number":148,"context_line":"default value used for the retrying module. (NOTE: The documentation for"},{"line_number":149,"context_line":"retrying says that by default it retries \"as fast as possible\", but an"},{"line_number":150,"context_line":"inspection of the code reveals 1000ms is the default)."}],"source_content_type":"text/x-rst","patch_set":6,"id":"baada198_f7de083d","line":147,"in_reply_to":"baada198_2874a966","updated":"2014-07-14 19:22:23.000000000","message":"Done","commit_id":"58e24fcf1880d7ad054bfa5c62b6f251bba5bbe6"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"d937f3625e76d1f3791a21b7f808e8b9d79bb784","unresolved":false,"context_lines":[{"line_number":142,"context_line":""},{"line_number":143,"context_line":"The default for node_locked_retry_attempts will be 3, which could potentially"},{"line_number":144,"context_line":"affect existing installations (see Performance Impact section) when upgraded,"},{"line_number":145,"context_line":"but should reduce NodeLocked errors encountered."},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"Developer impact"},{"line_number":148,"context_line":"----------------"}],"source_content_type":"text/x-rst","patch_set":7,"id":"baada198_beb1b390","line":145,"updated":"2014-07-15 10:43:53.000000000","message":"small nit: \"reduce number of NodeLocked errors encountered\"","commit_id":"7fbd26ac061a506dfe81f1be6da781f3f318877b"}]}