)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"11c4bc75c0488e316ff62047a63882f545a824b9","unresolved":false,"context_lines":[{"line_number":36,"context_line":""},{"line_number":37,"context_line":"Despite the config option name handoffs_first this change really"},{"line_number":38,"context_line":"implements more of a handoffs_*only* mode.  This behavior is somewhat"},{"line_number":39,"context_line":"consistent with  the replicator option (any error any handoff will make"},{"line_number":40,"context_line":"it essentially handoff only forever) but very true in the reconstructor."},{"line_number":41,"context_line":"This implementation will start to log a warning every cycle if you leave"},{"line_number":42,"context_line":"your reconstructor running in handoffs_first."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3a461143_1e55fcf8","line":39,"range":{"start_line":39,"start_character":40,"end_line":39,"end_character":61},"updated":"2017-01-30 23:56:54.000000000","message":"nit: \"any error on any handoff\", maybe?","commit_id":"3eb5857ed36dd7ae296a46359a60ab42b3b300a1"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"75b3323eb69554cded0ea1c30c9ac3c8ef8b9f29","unresolved":false,"context_lines":[{"line_number":36,"context_line":""},{"line_number":37,"context_line":"Despite the config option name handoffs_first this change really"},{"line_number":38,"context_line":"implements more of a handoffs_*only* mode.  This behavior is somewhat"},{"line_number":39,"context_line":"consistent with  the replicator option (any error any handoff will make"},{"line_number":40,"context_line":"it essentially handoff only forever) but very true in the reconstructor."},{"line_number":41,"context_line":"This implementation will start to log a warning every cycle if you leave"},{"line_number":42,"context_line":"your reconstructor running in handoffs_first."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3a461143_87fc1cb9","line":39,"in_reply_to":"3a461143_1e55fcf8","updated":"2017-01-31 20:09:16.000000000","message":"#willfix","commit_id":"3eb5857ed36dd7ae296a46359a60ab42b3b300a1"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5e66eac29fb63c1b8801c7a82b205db2319dbe21","unresolved":false,"context_lines":[{"line_number":36,"context_line":""},{"line_number":37,"context_line":"Despite the config option name handoffs_first this change really"},{"line_number":38,"context_line":"implements more of a handoffs_*only* mode.  This behavior is somewhat"},{"line_number":39,"context_line":"consistent with  the replicator option (any error any handoff will make"},{"line_number":40,"context_line":"it essentially handoff only forever) but very true in the reconstructor."},{"line_number":41,"context_line":"This implementation will start to log a warning every cycle if you leave"},{"line_number":42,"context_line":"your reconstructor running in handoffs_first."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3a461143_82aec72b","line":39,"in_reply_to":"3a461143_87fc1cb9","updated":"2017-01-31 23:35:11.000000000","message":"Done","commit_id":"3eb5857ed36dd7ae296a46359a60ab42b3b300a1"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"11c4869f740d91a42a77ccc8fbf6999afb13127c","unresolved":false,"context_lines":[{"line_number":39,"context_line":"consistent with  the replicator option (any error any handoff will make"},{"line_number":40,"context_line":"it essentially handoff only forever) but very true in the reconstructor."},{"line_number":41,"context_line":"This implementation will start to log a warning every cycle if you leave"},{"line_number":42,"context_line":"your reconstructor running in handoffs_first."},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"Closes-Bug: #1653018"},{"line_number":45,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3a461143_04287199","line":42,"updated":"2017-01-30 08:14:14.000000000","message":"During the Ops feedback sessions in the last summit, a note was made about _always_ prioritising reverts over sync as reverts are much easier. \n\nIn this case I wonder if we should be handoff first like your doing here, but skip to the next handoff or sync (if there are no more handoffs) on an error, rather then being stuck in hand off first.\n\nEither that, or *handoff mode* means only do handoff nodes until they\u0027re done, while not handoff mode means skip on errors and keep moving?\nOr maybe the idea of at a part level here makes sense.. obviously I haven\u0027t really thought everything through in my head yet.. just thinking out loud. And this change is sparking my memory.","commit_id":"3eb5857ed36dd7ae296a46359a60ab42b3b300a1"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"75b3323eb69554cded0ea1c30c9ac3c8ef8b9f29","unresolved":false,"context_lines":[{"line_number":39,"context_line":"consistent with  the replicator option (any error any handoff will make"},{"line_number":40,"context_line":"it essentially handoff only forever) but very true in the reconstructor."},{"line_number":41,"context_line":"This implementation will start to log a warning every cycle if you leave"},{"line_number":42,"context_line":"your reconstructor running in handoffs_first."},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"Closes-Bug: #1653018"},{"line_number":45,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3a461143_273288a7","line":42,"in_reply_to":"3a461143_04287199","updated":"2017-01-31 20:09:16.000000000","message":"you are thinking about the right things!  Thanks matt!","commit_id":"3eb5857ed36dd7ae296a46359a60ab42b3b300a1"},{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"e0eba2e06c99f218a41a94046612b5c5561eb2ab","unresolved":false,"context_lines":[{"line_number":21,"context_line":"partition from a handoff than it is to rebuild an entire partition from"},{"line_number":22,"context_line":"another primary using the other EC fragments in the cluster."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"This change deprecates handoffs_first in favor of handoffs_only which is"},{"line_number":25,"context_line":"far more useful - and just like handoffs_first mode in the replicator -"},{"line_number":26,"context_line":"it gives the operator the option of forcing the consistency engine to"},{"line_number":27,"context_line":"focus on rebalance.  The handoffs_only behavior is somewhat consistent"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"fa31d9ce_adbb930d","line":24,"updated":"2017-02-10 08:53:02.000000000","message":"This line sounds like we\u0027re deprecating it for both replicator and reconstructor (although reading further below clarifies it). So might be worth it to mention \"in favor of handoffs_only in the reconstructor\"","commit_id":"d603e4be521a1076de323360b040c3b80e61cded"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"435ec14d4bbd5ff6cfdcf888c890b40bf3b174f4","unresolved":false,"context_lines":[{"line_number":21,"context_line":"partition from a handoff than it is to rebuild an entire partition from"},{"line_number":22,"context_line":"another primary using the other EC fragments in the cluster."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"This change deprecates handoffs_first in favor of handoffs_only which is"},{"line_number":25,"context_line":"far more useful - and just like handoffs_first mode in the replicator -"},{"line_number":26,"context_line":"it gives the operator the option of forcing the consistency engine to"},{"line_number":27,"context_line":"focus on rebalance.  The handoffs_only behavior is somewhat consistent"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"fa31d9ce_7b0a16d4","line":24,"in_reply_to":"fa31d9ce_adbb930d","updated":"2017-02-10 22:12:36.000000000","message":"Done","commit_id":"d603e4be521a1076de323360b040c3b80e61cded"},{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"e0eba2e06c99f218a41a94046612b5c5561eb2ab","unresolved":false,"context_lines":[{"line_number":52,"context_line":"Known-Issues:"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"handoffs_only is not documented outside of the example config, see lp"},{"line_number":55,"context_line":"bug #1626290"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"Closes-Bug: #1653018"},{"line_number":58,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"fa31d9ce_ed090bdf","line":55,"updated":"2017-02-10 08:53:02.000000000","message":"While this https://github.com/openstack/swift/blob/master/etc/object-server.conf-sample#L277-L280 would have been enough for the prior patch, now that it\u0027s \"handoffs_only\" for reconstructor, maybe worth mentioning a note on it in the example config.","commit_id":"d603e4be521a1076de323360b040c3b80e61cded"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"435ec14d4bbd5ff6cfdcf888c890b40bf3b174f4","unresolved":false,"context_lines":[{"line_number":52,"context_line":"Known-Issues:"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"handoffs_only is not documented outside of the example config, see lp"},{"line_number":55,"context_line":"bug #1626290"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"Closes-Bug: #1653018"},{"line_number":58,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"fa31d9ce_bb872e0e","line":55,"in_reply_to":"fa31d9ce_ed090bdf","updated":"2017-02-10 22:12:36.000000000","message":"Done","commit_id":"d603e4be521a1076de323360b040c3b80e61cded"}],"etc/object-server.conf-sample":[{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"cace38cce57ada45fa97b4df51a5a175d9a94d36","unresolved":false,"context_lines":[{"line_number":304,"context_line":"# enabled the reconstrcutor will *only* revert rebalance fragments to primaries"},{"line_number":305,"context_line":"# and not attempt to sync any primary parts with neighbor primaries.  This will"},{"line_number":306,"context_line":"# force the reconstructor to sync and deleting handoffs fragments more quickly"},{"line_number":307,"context_line":"# and minimize rebuild."},{"line_number":308,"context_line":"# handoffs_only \u003d False"},{"line_number":309,"context_line":"#"},{"line_number":310,"context_line":"# You can set scheduling priority of processes. Niceness values range from -20"}],"source_content_type":"application/octet-stream","patch_set":5,"id":"fa31d9ce_7bf3f673","line":307,"updated":"2017-02-10 22:16:50.000000000","message":"sounds good.  maybe also emphasize the temporary nature : that if set to True, then do change this back to False as soon as emergency situation is resolved.\n\ns/to sync and deleting/to sync and delete/","commit_id":"a0440ae0fb48cb8af7e2de1726b579b4aab0bd32"},{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"97d622e6297d38a525008a16168ab9c8285c358e","unresolved":false,"context_lines":[{"line_number":304,"context_line":"# enabled the reconstrcutor will *only* revert rebalance fragments to primaries"},{"line_number":305,"context_line":"# and not attempt to sync any primary parts with neighbor primaries.  This will"},{"line_number":306,"context_line":"# force the reconstructor to sync and deleting handoffs fragments more quickly"},{"line_number":307,"context_line":"# and minimize rebuild."},{"line_number":308,"context_line":"# handoffs_only \u003d False"},{"line_number":309,"context_line":"#"},{"line_number":310,"context_line":"# You can set scheduling priority of processes. Niceness values range from -20"}],"source_content_type":"application/octet-stream","patch_set":5,"id":"fa31d9ce_ebab571b","line":307,"in_reply_to":"fa31d9ce_7bf3f673","updated":"2017-02-13 16:09:37.000000000","message":"+1\nSorry if it\u0027s gonna be back and forth for Clay :)","commit_id":"a0440ae0fb48cb8af7e2de1726b579b4aab0bd32"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"6ac96cfa2860570a16d1330a362809653fe4ad56","unresolved":false,"context_lines":[{"line_number":298,"context_line":"# lockup_timeout \u003d 1800"},{"line_number":299,"context_line":"# ring_check_interval \u003d 15"},{"line_number":300,"context_line":"# recon_cache_path \u003d /var/cache/swift"},{"line_number":301,"context_line":"# The handoffs_only mode option is for a special case emergency situations"},{"line_number":302,"context_line":"# during rebalance such as disk full in the cluster.  This option SHOULD NOT BE"},{"line_number":303,"context_line":"# CHANGED, except for such an extreme situations.  When handoffs_only mode is"},{"line_number":304,"context_line":"# enabled the reconstrcutor will *only* revert rebalance fragments to primaries"}],"source_content_type":"application/octet-stream","patch_set":6,"id":"fa31d9ce_8370f4dd","line":301,"range":{"start_line":301,"start_character":39,"end_line":301,"end_character":40},"updated":"2017-02-13 21:43:41.000000000","message":"Should just be \"for special-case emergency situations\"","commit_id":"39b5fbc3643850e7ad9340843891e9fb39a87f33"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a01dd1d1a4e8ec4d5ab1c9146e31545c1dd749f6","unresolved":false,"context_lines":[{"line_number":298,"context_line":"# lockup_timeout \u003d 1800"},{"line_number":299,"context_line":"# ring_check_interval \u003d 15"},{"line_number":300,"context_line":"# recon_cache_path \u003d /var/cache/swift"},{"line_number":301,"context_line":"# The handoffs_only mode option is for a special case emergency situations"},{"line_number":302,"context_line":"# during rebalance such as disk full in the cluster.  This option SHOULD NOT BE"},{"line_number":303,"context_line":"# CHANGED, except for such an extreme situations.  When handoffs_only mode is"},{"line_number":304,"context_line":"# enabled the reconstrcutor will *only* revert rebalance fragments to primaries"}],"source_content_type":"application/octet-stream","patch_set":6,"id":"fa31d9ce_ce0bf328","line":301,"in_reply_to":"fa31d9ce_8370f4dd","updated":"2017-02-14 07:27:36.000000000","message":"Done","commit_id":"39b5fbc3643850e7ad9340843891e9fb39a87f33"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"6ac96cfa2860570a16d1330a362809653fe4ad56","unresolved":false,"context_lines":[{"line_number":300,"context_line":"# recon_cache_path \u003d /var/cache/swift"},{"line_number":301,"context_line":"# The handoffs_only mode option is for a special case emergency situations"},{"line_number":302,"context_line":"# during rebalance such as disk full in the cluster.  This option SHOULD NOT BE"},{"line_number":303,"context_line":"# CHANGED, except for such an extreme situations.  When handoffs_only mode is"},{"line_number":304,"context_line":"# enabled the reconstrcutor will *only* revert rebalance fragments to primaries"},{"line_number":305,"context_line":"# and not attempt to sync any primary parts with neighbor primaries.  This will"},{"line_number":306,"context_line":"# force the reconstructor to sync and deleting handoffs fragments more quickly"}],"source_content_type":"application/octet-stream","patch_set":6,"id":"fa31d9ce_6342206a","line":303,"range":{"start_line":303,"start_character":27,"end_line":303,"end_character":48},"updated":"2017-02-13 21:43:41.000000000","message":"\"extreme situations\" or \"an extreme situation\"","commit_id":"39b5fbc3643850e7ad9340843891e9fb39a87f33"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a01dd1d1a4e8ec4d5ab1c9146e31545c1dd749f6","unresolved":false,"context_lines":[{"line_number":300,"context_line":"# recon_cache_path \u003d /var/cache/swift"},{"line_number":301,"context_line":"# The handoffs_only mode option is for a special case emergency situations"},{"line_number":302,"context_line":"# during rebalance such as disk full in the cluster.  This option SHOULD NOT BE"},{"line_number":303,"context_line":"# CHANGED, except for such an extreme situations.  When handoffs_only mode is"},{"line_number":304,"context_line":"# enabled the reconstrcutor will *only* revert rebalance fragments to primaries"},{"line_number":305,"context_line":"# and not attempt to sync any primary parts with neighbor primaries.  This will"},{"line_number":306,"context_line":"# force the reconstructor to sync and deleting handoffs fragments more quickly"}],"source_content_type":"application/octet-stream","patch_set":6,"id":"fa31d9ce_49061d1e","line":303,"in_reply_to":"fa31d9ce_6342206a","updated":"2017-02-14 07:27:36.000000000","message":"Done","commit_id":"39b5fbc3643850e7ad9340843891e9fb39a87f33"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"6ac96cfa2860570a16d1330a362809653fe4ad56","unresolved":false,"context_lines":[{"line_number":301,"context_line":"# The handoffs_only mode option is for a special case emergency situations"},{"line_number":302,"context_line":"# during rebalance such as disk full in the cluster.  This option SHOULD NOT BE"},{"line_number":303,"context_line":"# CHANGED, except for such an extreme situations.  When handoffs_only mode is"},{"line_number":304,"context_line":"# enabled the reconstrcutor will *only* revert rebalance fragments to primaries"},{"line_number":305,"context_line":"# and not attempt to sync any primary parts with neighbor primaries.  This will"},{"line_number":306,"context_line":"# force the reconstructor to sync and deleting handoffs fragments more quickly"},{"line_number":307,"context_line":"# and minimize rebuild.  The handoffs_only option is only for temporary use, it"}],"source_content_type":"application/octet-stream","patch_set":6,"id":"fa31d9ce_0354041f","line":304,"range":{"start_line":304,"start_character":14,"end_line":304,"end_character":27},"updated":"2017-02-13 21:43:41.000000000","message":"reconstructor","commit_id":"39b5fbc3643850e7ad9340843891e9fb39a87f33"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a01dd1d1a4e8ec4d5ab1c9146e31545c1dd749f6","unresolved":false,"context_lines":[{"line_number":301,"context_line":"# The handoffs_only mode option is for a special case emergency situations"},{"line_number":302,"context_line":"# during rebalance such as disk full in the cluster.  This option SHOULD NOT BE"},{"line_number":303,"context_line":"# CHANGED, except for such an extreme situations.  When handoffs_only mode is"},{"line_number":304,"context_line":"# enabled the reconstrcutor will *only* revert rebalance fragments to primaries"},{"line_number":305,"context_line":"# and not attempt to sync any primary parts with neighbor primaries.  This will"},{"line_number":306,"context_line":"# force the reconstructor to sync and deleting handoffs fragments more quickly"},{"line_number":307,"context_line":"# and minimize rebuild.  The handoffs_only option is only for temporary use, it"}],"source_content_type":"application/octet-stream","patch_set":6,"id":"fa31d9ce_a915d940","line":304,"in_reply_to":"fa31d9ce_0354041f","updated":"2017-02-14 07:27:36.000000000","message":"Done","commit_id":"39b5fbc3643850e7ad9340843891e9fb39a87f33"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"6ac96cfa2860570a16d1330a362809653fe4ad56","unresolved":false,"context_lines":[{"line_number":304,"context_line":"# enabled the reconstrcutor will *only* revert rebalance fragments to primaries"},{"line_number":305,"context_line":"# and not attempt to sync any primary parts with neighbor primaries.  This will"},{"line_number":306,"context_line":"# force the reconstructor to sync and deleting handoffs fragments more quickly"},{"line_number":307,"context_line":"# and minimize rebuild.  The handoffs_only option is only for temporary use, it"},{"line_number":308,"context_line":"# should be disabled as soon as emergency situation is resolved."},{"line_number":309,"context_line":"# handoffs_only \u003d False"},{"line_number":310,"context_line":"#"}],"source_content_type":"application/octet-stream","patch_set":6,"id":"fa31d9ce_431fdc6b","line":307,"range":{"start_line":307,"start_character":15,"end_line":307,"end_character":22},"updated":"2017-02-13 21:43:41.000000000","message":"Rebuild time, or number of rebuilds? I suppose both, ultimately... but just saying \"minimize rebuild\" seems awkward.","commit_id":"39b5fbc3643850e7ad9340843891e9fb39a87f33"},{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"e796dfcec24c89c6b85359a9299407a218fdf28b","unresolved":false,"context_lines":[{"line_number":304,"context_line":"# enabled the reconstrcutor will *only* revert rebalance fragments to primaries"},{"line_number":305,"context_line":"# and not attempt to sync any primary parts with neighbor primaries.  This will"},{"line_number":306,"context_line":"# force the reconstructor to sync and deleting handoffs fragments more quickly"},{"line_number":307,"context_line":"# and minimize rebuild.  The handoffs_only option is only for temporary use, it"},{"line_number":308,"context_line":"# should be disabled as soon as emergency situation is resolved."},{"line_number":309,"context_line":"# handoffs_only \u003d False"},{"line_number":310,"context_line":"#"}],"source_content_type":"application/octet-stream","patch_set":6,"id":"fa31d9ce_f2eee3b2","line":307,"updated":"2017-02-13 22:49:30.000000000","message":"s/and deleting handoffs/and delete handoff/","commit_id":"39b5fbc3643850e7ad9340843891e9fb39a87f33"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a01dd1d1a4e8ec4d5ab1c9146e31545c1dd749f6","unresolved":false,"context_lines":[{"line_number":304,"context_line":"# enabled the reconstrcutor will *only* revert rebalance fragments to primaries"},{"line_number":305,"context_line":"# and not attempt to sync any primary parts with neighbor primaries.  This will"},{"line_number":306,"context_line":"# force the reconstructor to sync and deleting handoffs fragments more quickly"},{"line_number":307,"context_line":"# and minimize rebuild.  The handoffs_only option is only for temporary use, it"},{"line_number":308,"context_line":"# should be disabled as soon as emergency situation is resolved."},{"line_number":309,"context_line":"# handoffs_only \u003d False"},{"line_number":310,"context_line":"#"}],"source_content_type":"application/octet-stream","patch_set":6,"id":"fa31d9ce_e91b5171","line":307,"in_reply_to":"fa31d9ce_431fdc6b","updated":"2017-02-14 07:27:36.000000000","message":"Done","commit_id":"39b5fbc3643850e7ad9340843891e9fb39a87f33"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a01dd1d1a4e8ec4d5ab1c9146e31545c1dd749f6","unresolved":false,"context_lines":[{"line_number":304,"context_line":"# enabled the reconstrcutor will *only* revert rebalance fragments to primaries"},{"line_number":305,"context_line":"# and not attempt to sync any primary parts with neighbor primaries.  This will"},{"line_number":306,"context_line":"# force the reconstructor to sync and deleting handoffs fragments more quickly"},{"line_number":307,"context_line":"# and minimize rebuild.  The handoffs_only option is only for temporary use, it"},{"line_number":308,"context_line":"# should be disabled as soon as emergency situation is resolved."},{"line_number":309,"context_line":"# handoffs_only \u003d False"},{"line_number":310,"context_line":"#"}],"source_content_type":"application/octet-stream","patch_set":6,"id":"fa31d9ce_2b739616","line":307,"in_reply_to":"fa31d9ce_f2eee3b2","updated":"2017-02-14 07:27:36.000000000","message":"Done","commit_id":"39b5fbc3643850e7ad9340843891e9fb39a87f33"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"6ac96cfa2860570a16d1330a362809653fe4ad56","unresolved":false,"context_lines":[{"line_number":305,"context_line":"# and not attempt to sync any primary parts with neighbor primaries.  This will"},{"line_number":306,"context_line":"# force the reconstructor to sync and deleting handoffs fragments more quickly"},{"line_number":307,"context_line":"# and minimize rebuild.  The handoffs_only option is only for temporary use, it"},{"line_number":308,"context_line":"# should be disabled as soon as emergency situation is resolved."},{"line_number":309,"context_line":"# handoffs_only \u003d False"},{"line_number":310,"context_line":"#"},{"line_number":311,"context_line":"# You can set scheduling priority of processes. Niceness values range from -20"}],"source_content_type":"application/octet-stream","patch_set":6,"id":"fa31d9ce_e3a8d022","line":308,"updated":"2017-02-13 21:43:41.000000000","message":"Shouldn\u0027t this have some reference to the now-deprecated handoffs_first option? Like,\n\n\u003e This used to be known as handoffs_first, but that *never* did what you wanted. If you thought you wanted handoffs_first, chances are you really wanted this. If you\u0027re hurting so badly that upgrading Swift in the middle of this emergency situation seems like a *good* idea, we\u0027re so incredibly sorry; we at least made it so you can upgrade configs in-place with handoffs_first enabling handoffs_only. However, handoffs_first will be removed in a future release.","commit_id":"39b5fbc3643850e7ad9340843891e9fb39a87f33"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a01dd1d1a4e8ec4d5ab1c9146e31545c1dd749f6","unresolved":false,"context_lines":[{"line_number":305,"context_line":"# and not attempt to sync any primary parts with neighbor primaries.  This will"},{"line_number":306,"context_line":"# force the reconstructor to sync and deleting handoffs fragments more quickly"},{"line_number":307,"context_line":"# and minimize rebuild.  The handoffs_only option is only for temporary use, it"},{"line_number":308,"context_line":"# should be disabled as soon as emergency situation is resolved."},{"line_number":309,"context_line":"# handoffs_only \u003d False"},{"line_number":310,"context_line":"#"},{"line_number":311,"context_line":"# You can set scheduling priority of processes. Niceness values range from -20"}],"source_content_type":"application/octet-stream","patch_set":6,"id":"fa31d9ce_eea8b722","line":308,"in_reply_to":"fa31d9ce_e3a8d022","updated":"2017-02-14 07:27:36.000000000","message":"I don\u0027t think that level of conversational prose is at all appropriate for an example config, no.\n\nWhen writing this up, I had reasoned the broken and previously undocumented handoffs_first option should not be described under the reconstructor at all (don\u0027t encourage the stupid).\n\nOTOH at some earlier point in the change history I had reasoned we should *leave this option named handoffs_first* to a) avoid developing a deprecation strategy inside a bug fix change and b) so that an operator with their hair on fire who might have experience using the replicator\u0027s handoffs_first mode (and is frantically searching for a similar option in the reconstructor) ... has some chance to find it!\n\n... so I\u0027ve decided I ought least concede that a grep for the rune handoffs_first should trip somewhere near the handoffs_only option in the reconstructor section.\n\nBut that\u0027s a much lower priority than trying to explain to the poor operator as tersely as possible - what they need to type in to make the pain go away.\n\nHopefully you like what I came up with :\\","commit_id":"39b5fbc3643850e7ad9340843891e9fb39a87f33"}],"swift/obj/reconstructor.py":[{"author":{"_account_id":2622,"name":"Samuel Merritt","email":"spam+launchpad@andcheese.org","username":"torgomatic"},"change_message_id":"9554aa3ff96559b6eaf329ffa6a5d3b0cfea35cf","unresolved":false,"context_lines":[{"line_number":952,"context_line":"                for job in jobs:"},{"line_number":953,"context_line":"                    if (self.handoffs_first and job[\u0027job_type\u0027] !\u003d REVERT):"},{"line_number":954,"context_line":"                        self.logger.debug(\u0027Skipping %s job for %s\u0027,"},{"line_number":955,"context_line":"                                          job[\u0027job_type\u0027], job[\u0027path\u0027])"},{"line_number":956,"context_line":"                        continue"},{"line_number":957,"context_line":"                    self.run_pool.spawn(self.process_job, job)"},{"line_number":958,"context_line":"            with Timeout(self.lockup_timeout):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a461143_b7fe1136","line":955,"updated":"2017-01-27 18:27:55.000000000","message":"Maybe add \"...because handoffs_first is on\"?","commit_id":"3eb5857ed36dd7ae296a46359a60ab42b3b300a1"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5e66eac29fb63c1b8801c7a82b205db2319dbe21","unresolved":false,"context_lines":[{"line_number":952,"context_line":"                for job in jobs:"},{"line_number":953,"context_line":"                    if (self.handoffs_first and job[\u0027job_type\u0027] !\u003d REVERT):"},{"line_number":954,"context_line":"                        self.logger.debug(\u0027Skipping %s job for %s\u0027,"},{"line_number":955,"context_line":"                                          job[\u0027job_type\u0027], job[\u0027path\u0027])"},{"line_number":956,"context_line":"                        continue"},{"line_number":957,"context_line":"                    self.run_pool.spawn(self.process_job, job)"},{"line_number":958,"context_line":"            with Timeout(self.lockup_timeout):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a461143_78942a8a","line":955,"in_reply_to":"3a461143_47aa3486","updated":"2017-01-31 23:35:11.000000000","message":"zomg, love it\n\n    object-6030: Skipping sync_only job for /srv/node3/sdb3/objects-1/959 while in handoffs_first mode.","commit_id":"3eb5857ed36dd7ae296a46359a60ab42b3b300a1"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"75b3323eb69554cded0ea1c30c9ac3c8ef8b9f29","unresolved":false,"context_lines":[{"line_number":952,"context_line":"                for job in jobs:"},{"line_number":953,"context_line":"                    if (self.handoffs_first and job[\u0027job_type\u0027] !\u003d REVERT):"},{"line_number":954,"context_line":"                        self.logger.debug(\u0027Skipping %s job for %s\u0027,"},{"line_number":955,"context_line":"                                          job[\u0027job_type\u0027], job[\u0027path\u0027])"},{"line_number":956,"context_line":"                        continue"},{"line_number":957,"context_line":"                    self.run_pool.spawn(self.process_job, job)"},{"line_number":958,"context_line":"            with Timeout(self.lockup_timeout):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a461143_47aa3486","line":955,"in_reply_to":"3a461143_9ebc8c07","updated":"2017-01-31 20:09:16.000000000","message":"@sam good call, #willfix\n\n@tburke that\u0027s right!  You are thinking about the right things!  In dev this call was helpful (to me) to understand that the patch was doing the right thing - now that i\u0027ve written it i could easily see it coming out if there was a reason to do so (but don\u0027t expect dev to run in handoffs first or rebalance enough to be a problem [1]) - obviously despite being potentially useful to an operator if they were confused what was going on when running in handoffs_first - we couldn\u0027t possibly let a message of this cardinality be info/warning - 100% correct!\n\n1. unlike that dumb listdir call in suffix hashes we started debug logging recently :P","commit_id":"3eb5857ed36dd7ae296a46359a60ab42b3b300a1"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"11c4bc75c0488e316ff62047a63882f545a824b9","unresolved":false,"context_lines":[{"line_number":952,"context_line":"                for job in jobs:"},{"line_number":953,"context_line":"                    if (self.handoffs_first and job[\u0027job_type\u0027] !\u003d REVERT):"},{"line_number":954,"context_line":"                        self.logger.debug(\u0027Skipping %s job for %s\u0027,"},{"line_number":955,"context_line":"                                          job[\u0027job_type\u0027], job[\u0027path\u0027])"},{"line_number":956,"context_line":"                        continue"},{"line_number":957,"context_line":"                    self.run_pool.spawn(self.process_job, job)"},{"line_number":958,"context_line":"            with Timeout(self.lockup_timeout):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a461143_9ebc8c07","line":955,"in_reply_to":"3a461143_b7fe1136","updated":"2017-01-30 23:56:54.000000000","message":"So we log every skipped job for every part? How noisy is this going to get? Eh, I suppose it\u0027s only debug...","commit_id":"3eb5857ed36dd7ae296a46359a60ab42b3b300a1"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"d8d17492482ef455791383ef1178f1f843e205da","unresolved":false,"context_lines":[{"line_number":893,"context_line":"        random.shuffle(jobs)"},{"line_number":894,"context_line":"        if self.handoffs_first:"},{"line_number":895,"context_line":"            # Move the handoff revert jobs to the front of the list"},{"line_number":896,"context_line":"            jobs.sort(key\u003dlambda job: job[\u0027job_type\u0027], reverse\u003dTrue)"},{"line_number":897,"context_line":"        self.job_count +\u003d len(jobs)"},{"line_number":898,"context_line":"        return jobs"},{"line_number":899,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3a461143_939d025c","side":"PARENT","line":896,"updated":"2017-02-02 06:55:48.000000000","message":"Ah, wait?\n\nLooking at current replicator code[1], the sort still exists there to make \"delete\" (\"revert\" in reconstructor) job in front of the list.\n\nExactly, it may be required because anyway, the non-revert job should be skipped in handoff first mode but... commit message obviously says \" just like handoffs_first mode in the replicator \" so that on the perspective, we may ought to keep the sort here?\n\nAh... I noticed the reason that the replicator will try to update_delete first and then *if something remaining*, it will abort the all replicate jobs in the list. Instead, the reconstructor with this patch choose the job at random and if it\u0027s not revert, skip it and try to the next.\n\nRight now, I cannot judge the diff is expected by Clay.\n\n1: https://github.com/openstack/swift/blob/master/swift/obj/replicator.py#L689","commit_id":"2f0ab78f9ff85483a157c9cbb17b50eeff539ef9"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b996871937b2090933072f6700d15d6cd19c76a3","unresolved":false,"context_lines":[{"line_number":893,"context_line":"        random.shuffle(jobs)"},{"line_number":894,"context_line":"        if self.handoffs_first:"},{"line_number":895,"context_line":"            # Move the handoff revert jobs to the front of the list"},{"line_number":896,"context_line":"            jobs.sort(key\u003dlambda job: job[\u0027job_type\u0027], reverse\u003dTrue)"},{"line_number":897,"context_line":"        self.job_count +\u003d len(jobs)"},{"line_number":898,"context_line":"        return jobs"},{"line_number":899,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3a461143_0c09de31","side":"PARENT","line":896,"in_reply_to":"3a461143_939d025c","updated":"2017-02-02 19:31:38.000000000","message":"Ah, yes - I\u0027m surprised this hasn\u0027t come up - it\u0027s a key difference between the reconstructor and replicator and I spent a long time agonizing over it!  I meant to say something.\n\nIt has to do with job organization.","commit_id":"2f0ab78f9ff85483a157c9cbb17b50eeff539ef9"},{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"0619aa01fb9553ccd5cfdc04c3f17f2d2dae6c87","unresolved":false,"context_lines":[{"line_number":669,"context_line":"            self.delete_reverted_objs("},{"line_number":670,"context_line":"                job, reverted_objs, job[\u0027frag_index\u0027])"},{"line_number":671,"context_line":"        else:"},{"line_number":672,"context_line":"            self.handoffs_remaining +\u003d 1"},{"line_number":673,"context_line":"        self.logger.timing_since(\u0027partition.delete.timing\u0027, begin)"},{"line_number":674,"context_line":""},{"line_number":675,"context_line":"    def _get_part_jobs(self, local_dev, part_path, partition, policy):"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a430d35_a2f3639d","line":672,"updated":"2017-02-06 05:23:44.000000000","message":"I was surprised at first to see this is never decremented in this class.  But I think this is because the true number of handoffs remains the same during the entire handling of the reconstruct call (who, at first, calls _reset_stats() that initializes this value to 0).  It is only as we loop over jobs that we find the handoffs that remain.","commit_id":"72dd3d1112a183bae6d24a27b3bab2c28f3304ac"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"5b315a059bb0dc676dfaf531a8fd0e769876ae16","unresolved":false,"context_lines":[{"line_number":669,"context_line":"            self.delete_reverted_objs("},{"line_number":670,"context_line":"                job, reverted_objs, job[\u0027frag_index\u0027])"},{"line_number":671,"context_line":"        else:"},{"line_number":672,"context_line":"            self.handoffs_remaining +\u003d 1"},{"line_number":673,"context_line":"        self.logger.timing_since(\u0027partition.delete.timing\u0027, begin)"},{"line_number":674,"context_line":""},{"line_number":675,"context_line":"    def _get_part_jobs(self, local_dev, part_path, partition, policy):"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a430d35_378a41c0","line":672,"in_reply_to":"1a430d35_a2f3639d","updated":"2017-02-06 15:56:47.000000000","message":"Yes, that reconstructor calls _reset_starts() at first.","commit_id":"72dd3d1112a183bae6d24a27b3bab2c28f3304ac"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b996871937b2090933072f6700d15d6cd19c76a3","unresolved":false,"context_lines":[{"line_number":704,"context_line":"        \"\"\""},{"line_number":705,"context_line":"        # find all the fi\u0027s in the part, and which suffixes have them"},{"line_number":706,"context_line":"        try:"},{"line_number":707,"context_line":"            hashes \u003d self._get_hashes(policy, part_path, do_listdir\u003dTrue)"},{"line_number":708,"context_line":"        except OSError as e:"},{"line_number":709,"context_line":"            if e.errno !\u003d errno.ENOTDIR:"},{"line_number":710,"context_line":"                raise"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a461143_3390b1ec","line":707,"updated":"2017-02-02 19:31:38.000000000","message":"this is he part where we finally find out which frag_indexes are in the partition - unlike in the replicator a part can be *assigned* to this node - but still be the *wrong* frag and generate a revert job.\n\nSince this is an IO intensive operation and has a tendency to get stale - we want to do it as close to when we process the job as possible - and we definitely *can\u0027t* do this in the top-level collect_jobs loop before we even get started doing any useful work - it could take hours if there is enough active policies/devices/parts on a node!\n\nBut this also means that we\u0027re doing IO to rehash primary partitions - every time we rehash a part that doesn\u0027t generate a sync job we\u0027ve done wasted work!  I think I\u0027d prefer to save that wasted work until the end!","commit_id":"72dd3d1112a183bae6d24a27b3bab2c28f3304ac"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b996871937b2090933072f6700d15d6cd19c76a3","unresolved":false,"context_lines":[{"line_number":875,"context_line":"                        remove_file(part_path)"},{"line_number":876,"context_line":"                        self.reconstruction_part_count +\u003d 1"},{"line_number":877,"context_line":"                        continue"},{"line_number":878,"context_line":"                    partition \u003d int(partition)"},{"line_number":879,"context_line":"                    if override_partitions and (partition not in"},{"line_number":880,"context_line":"                                                override_partitions):"},{"line_number":881,"context_line":"                        continue"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a461143_f3cd495e","line":878,"updated":"2017-02-02 19:31:38.000000000","message":"I think we *could* peek into the ring and annotate the part info a little bit here - but it\u0027d only be a hint.  A part assigned to the node could have the wrong frags in it and still generate revert jobs.\n\nI acctually wouldn\u0027t be against adding a \u0027handoff\u0027 flag to part_info... nor would I be against preferring to rehash handoff parts first.\n\nI\u0027m glad I looked at this again!","commit_id":"72dd3d1112a183bae6d24a27b3bab2c28f3304ac"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b996871937b2090933072f6700d15d6cd19c76a3","unresolved":false,"context_lines":[{"line_number":886,"context_line":"                        \u0027part_path\u0027: part_path,"},{"line_number":887,"context_line":"                    }"},{"line_number":888,"context_line":"                    all_parts.append(part_info)"},{"line_number":889,"context_line":"        random.shuffle(all_parts)"},{"line_number":890,"context_line":"        return all_parts"},{"line_number":891,"context_line":""},{"line_number":892,"context_line":"    def build_reconstruction_jobs(self, part_info):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a461143_b3f741f4","line":889,"updated":"2017-02-02 19:31:38.000000000","message":"Here at this point where we have all the parts in a list - we don\u0027t know yet which parts have fragments that need to be reverted and which will need to be sync\u0027d.\n\nThis is different from the replicator!  In the replicator, we call build_replication_jobs inside of collect_parts [1] - so we already know if it\u0027s a delete/revert job and can sort as soon as we shuffle.\n\n1. https://github.com/openstack/swift/blob/master/swift/obj/replicator.py#L686","commit_id":"72dd3d1112a183bae6d24a27b3bab2c28f3304ac"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"5b315a059bb0dc676dfaf531a8fd0e769876ae16","unresolved":false,"context_lines":[{"line_number":886,"context_line":"                        \u0027part_path\u0027: part_path,"},{"line_number":887,"context_line":"                    }"},{"line_number":888,"context_line":"                    all_parts.append(part_info)"},{"line_number":889,"context_line":"        random.shuffle(all_parts)"},{"line_number":890,"context_line":"        return all_parts"},{"line_number":891,"context_line":""},{"line_number":892,"context_line":"    def build_reconstruction_jobs(self, part_info):"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a430d35_b282bf50","line":889,"in_reply_to":"3a461143_b3f741f4","updated":"2017-02-06 15:56:47.000000000","message":"Ah, got it. So replicator can find obvious handoff *parts* but reconstructor is not able because some revert jobs can be in the primary parts.","commit_id":"72dd3d1112a183bae6d24a27b3bab2c28f3304ac"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b996871937b2090933072f6700d15d6cd19c76a3","unresolved":false,"context_lines":[{"line_number":938,"context_line":"                                       \"current reconstruction pass.\"))"},{"line_number":939,"context_line":"                    return"},{"line_number":940,"context_line":"                self.reconstruction_part_count +\u003d 1"},{"line_number":941,"context_line":"                jobs \u003d self.build_reconstruction_jobs(part_info)"},{"line_number":942,"context_line":"                if not jobs:"},{"line_number":943,"context_line":"                    # If this part belongs on this node, _get_part_jobs"},{"line_number":944,"context_line":"                    # will *always* build a sync_job - even if there\u0027s"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a461143_f36e2995","line":941,"updated":"2017-02-02 19:31:38.000000000","message":"In most cases I expect (I have no stats on this) most parts probably generate either only a sync job (if the part is assigned to this node, it\u0027s probably got the right fragments in it) or a revert job (if it\u0027s been reassigned to a new primary).  We could infer simply from the part number and the ring that we *expect* a part generate a sync job, and we could be *certain* if *no* part-fragments for a given part are assigned to this device that any fragments in said part *must* generate only revert job(s).  But we don\u0027t bother to check until here.","commit_id":"72dd3d1112a183bae6d24a27b3bab2c28f3304ac"},{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"0619aa01fb9553ccd5cfdc04c3f17f2d2dae6c87","unresolved":false,"context_lines":[{"line_number":953,"context_line":"                    if (self.handoffs_first and job[\u0027job_type\u0027] !\u003d REVERT):"},{"line_number":954,"context_line":"                        self.logger.debug(\u0027Skipping %s job for %s \u0027"},{"line_number":955,"context_line":"                                          \u0027while in handoffs_first mode.\u0027,"},{"line_number":956,"context_line":"                                          job[\u0027job_type\u0027], job[\u0027path\u0027])"},{"line_number":957,"context_line":"                        continue"},{"line_number":958,"context_line":"                    self.run_pool.spawn(self.process_job, job)"},{"line_number":959,"context_line":"            with Timeout(self.lockup_timeout):"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a430d35_024ef795","line":956,"updated":"2017-02-06 05:23:44.000000000","message":"Ok, so this is tested by keeping track of the job type in the fake_process_job, and asserting it was only REVERTs.","commit_id":"72dd3d1112a183bae6d24a27b3bab2c28f3304ac"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b996871937b2090933072f6700d15d6cd19c76a3","unresolved":false,"context_lines":[{"line_number":955,"context_line":"                                          \u0027while in handoffs_first mode.\u0027,"},{"line_number":956,"context_line":"                                          job[\u0027job_type\u0027], job[\u0027path\u0027])"},{"line_number":957,"context_line":"                        continue"},{"line_number":958,"context_line":"                    self.run_pool.spawn(self.process_job, job)"},{"line_number":959,"context_line":"            with Timeout(self.lockup_timeout):"},{"line_number":960,"context_line":"                self.run_pool.waitall()"},{"line_number":961,"context_line":"        except (Exception, Timeout):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a461143_eef562cb","line":958,"updated":"2017-02-02 19:31:38.000000000","message":"This is the majority of the work - if this spawn blocks because you\u0027ve exhausted concurrency everything stops - and rebuilding an entire partition to a primary can take a really long time - eventually you\u0027re reconstructor will be doing mostly that.\n\nOTOH, in the case where we have handoffs_first on we can avoid a ton of work by skipping the spawn of the SYNC jobs.  I think there are some additional opportunities to avoid some other work on non-handoff parts (like the _get_hashes call!) - but skipping this spawn for SYNC jobs is doing a very effective job of eliminating the bulk of the unwanted work.\n\nIn fact I would wager than iterating a list and skipping jobs you don\u0027t want is roughly the same as sorting a list to put the jobs you want first - and the first is preferable if it takes a long time to build the whole list; better to amortize the _get_hashes calls as we go.","commit_id":"72dd3d1112a183bae6d24a27b3bab2c28f3304ac"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"799d0ceb13c013ba5350e18dc0e08ff6cb2a1b67","unresolved":false,"context_lines":[{"line_number":955,"context_line":"                                          \u0027while in handoffs_first mode.\u0027,"},{"line_number":956,"context_line":"                                          job[\u0027job_type\u0027], job[\u0027path\u0027])"},{"line_number":957,"context_line":"                        continue"},{"line_number":958,"context_line":"                    self.run_pool.spawn(self.process_job, job)"},{"line_number":959,"context_line":"            with Timeout(self.lockup_timeout):"},{"line_number":960,"context_line":"                self.run_pool.waitall()"},{"line_number":961,"context_line":"        except (Exception, Timeout):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a461143_cbfecead","line":958,"in_reply_to":"3a461143_eef562cb","updated":"2017-02-02 22:52:42.000000000","message":"I\u0027ll go a step further -- in the general case, ignoring the stuff you don\u0027t care about is *strictly better* than sorting to get the stuff you want to the front, because we only ever have to go through that loop once. At least, provided that we really don\u0027t care about any of the other things.\n\nNow, *maybe* there\u0027s some hidden optimization in sort() that uses the fact that there are only two possible values for job_type to turn it into a radix sort. But we\u0027re still looking at a single part, and we want to prioritize looking for more revert jobs on other parts over starting to deal with the sync jobs in this one.\n\nMaybe it\u0027d be worth adding something after the waitall() like\n\n if self.handoffs_first:\n     for part_info in self.collect_parts(**kwargs):\n         ...\n\nrepeating this whole loop without the changes, or having a deque of failed REVERTs to keep retrying in between the two loops. But having this config option seems\n\n* better than where we *were* (since operators can now dig themselves out of this hole), and\n* a much smaller change to try to get in before a release.","commit_id":"72dd3d1112a183bae6d24a27b3bab2c28f3304ac"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"799d0ceb13c013ba5350e18dc0e08ff6cb2a1b67","unresolved":false,"context_lines":[{"line_number":956,"context_line":"                                          job[\u0027job_type\u0027], job[\u0027path\u0027])"},{"line_number":957,"context_line":"                        continue"},{"line_number":958,"context_line":"                    self.run_pool.spawn(self.process_job, job)"},{"line_number":959,"context_line":"            with Timeout(self.lockup_timeout):"},{"line_number":960,"context_line":"                self.run_pool.waitall()"},{"line_number":961,"context_line":"        except (Exception, Timeout):"},{"line_number":962,"context_line":"            self.logger.exception(_(\"Exception in top-level\""}],"source_content_type":"text/x-python","patch_set":3,"id":"3a461143_d91147b1","line":959,"updated":"2017-02-02 22:52:42.000000000","message":":-/\n\nOff topic, but I\u0027m less and less certain that this is enough to get us the behavior we want. Why isn\u0027t the spawn() in a Timeout?","commit_id":"72dd3d1112a183bae6d24a27b3bab2c28f3304ac"},{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"0619aa01fb9553ccd5cfdc04c3f17f2d2dae6c87","unresolved":false,"context_lines":[{"line_number":956,"context_line":"                                          job[\u0027job_type\u0027], job[\u0027path\u0027])"},{"line_number":957,"context_line":"                        continue"},{"line_number":958,"context_line":"                    self.run_pool.spawn(self.process_job, job)"},{"line_number":959,"context_line":"            with Timeout(self.lockup_timeout):"},{"line_number":960,"context_line":"                self.run_pool.waitall()"},{"line_number":961,"context_line":"        except (Exception, Timeout):"},{"line_number":962,"context_line":"            self.logger.exception(_(\"Exception in top-level\""}],"source_content_type":"text/x-python","patch_set":3,"id":"1a430d35_a29443e2","line":959,"in_reply_to":"3a461143_d91147b1","updated":"2017-02-06 05:23:44.000000000","message":"This is similar to what is done in the replicator for the threads.  I assumed that every thread spawned in the run_pool would now (after this waitall call) be under the timeout.  Is that not correct?","commit_id":"72dd3d1112a183bae6d24a27b3bab2c28f3304ac"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"799d0ceb13c013ba5350e18dc0e08ff6cb2a1b67","unresolved":false,"context_lines":[{"line_number":974,"context_line":"            else:"},{"line_number":975,"context_line":"                self.logger.warning(_("},{"line_number":976,"context_line":"                    \"Handoffs first mode found no handoffs remaining.  \""},{"line_number":977,"context_line":"                    \"You should disable handoffs first immediately.\"))"},{"line_number":978,"context_line":""},{"line_number":979,"context_line":"    def run_once(self, *args, **kwargs):"},{"line_number":980,"context_line":"        start \u003d time.time()"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a461143_4b857e05","line":977,"updated":"2017-02-02 22:52:42.000000000","message":"I think *this* is the bit that should make us a little nervous -- operators *need* to turn off this option once there are no more handoffs, or we\u0027re harming durability. In the replicator with the sorting, we\u0027ll still get to those SYNCs (or do we call them updates there? I forget) eventually.\n\nHowever, given the messaging around it (the Known-Issue in the commit message, the warning we log, the release notes for this which I\u0027m sure will be great...) I think it\u0027s an appropriate trade-off to make.","commit_id":"72dd3d1112a183bae6d24a27b3bab2c28f3304ac"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ddadbbd898c3769318ce5d6b3a9ceeee5652f6d7","unresolved":false,"context_lines":[{"line_number":974,"context_line":"            else:"},{"line_number":975,"context_line":"                self.logger.warning(_("},{"line_number":976,"context_line":"                    \"Handoffs first mode found no handoffs remaining.  \""},{"line_number":977,"context_line":"                    \"You should disable handoffs first immediately.\"))"},{"line_number":978,"context_line":""},{"line_number":979,"context_line":"    def run_once(self, *args, **kwargs):"},{"line_number":980,"context_line":"        start \u003d time.time()"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a430d35_d91cbd15","line":977,"in_reply_to":"1a430d35_6706d6cf","updated":"2017-02-06 19:10:43.000000000","message":"Mahati +2 - great idea","commit_id":"72dd3d1112a183bae6d24a27b3bab2c28f3304ac"},{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"55bfe3545aa447495c09c572064f81b508430ce0","unresolved":false,"context_lines":[{"line_number":974,"context_line":"            else:"},{"line_number":975,"context_line":"                self.logger.warning(_("},{"line_number":976,"context_line":"                    \"Handoffs first mode found no handoffs remaining.  \""},{"line_number":977,"context_line":"                    \"You should disable handoffs first immediately.\"))"},{"line_number":978,"context_line":""},{"line_number":979,"context_line":"    def run_once(self, *args, **kwargs):"},{"line_number":980,"context_line":"        start \u003d time.time()"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a430d35_6706d6cf","line":977,"in_reply_to":"1a430d35_cab6b989","updated":"2017-02-06 14:05:21.000000000","message":"A note in the reconstructor config is an excellent idea.  +1","commit_id":"72dd3d1112a183bae6d24a27b3bab2c28f3304ac"},{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"0619aa01fb9553ccd5cfdc04c3f17f2d2dae6c87","unresolved":false,"context_lines":[{"line_number":974,"context_line":"            else:"},{"line_number":975,"context_line":"                self.logger.warning(_("},{"line_number":976,"context_line":"                    \"Handoffs first mode found no handoffs remaining.  \""},{"line_number":977,"context_line":"                    \"You should disable handoffs first immediately.\"))"},{"line_number":978,"context_line":""},{"line_number":979,"context_line":"    def run_once(self, *args, **kwargs):"},{"line_number":980,"context_line":"        start \u003d time.time()"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a430d35_e231eb0c","line":977,"in_reply_to":"3a461143_4b857e05","updated":"2017-02-06 05:23:44.000000000","message":"Agreed, the \"handoffs ONLY\" part does make me a little nervous, but I don\u0027t have the same experience with deployments that you all do.  So I will defer to your judgement and trust that this will be ok.","commit_id":"72dd3d1112a183bae6d24a27b3bab2c28f3304ac"},{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"de7ee577e70898f76fc2e0f392a80125cc807d35","unresolved":false,"context_lines":[{"line_number":974,"context_line":"            else:"},{"line_number":975,"context_line":"                self.logger.warning(_("},{"line_number":976,"context_line":"                    \"Handoffs first mode found no handoffs remaining.  \""},{"line_number":977,"context_line":"                    \"You should disable handoffs first immediately.\"))"},{"line_number":978,"context_line":""},{"line_number":979,"context_line":"    def run_once(self, *args, **kwargs):"},{"line_number":980,"context_line":"        start \u003d time.time()"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a430d35_cab6b989","line":977,"in_reply_to":"3a461143_4b857e05","updated":"2017-02-06 11:24:27.000000000","message":"How about we add a note here - https://github.com/openstack/swift/blob/master/etc/object-server.conf-sample#L301 - reminding to watch out for a warning in the logs, to turn off the mode?\n\nSimilar to some notes we have in replicator section:https://github.com/openstack/swift/blob/master/etc/object-server.conf-sample#L245","commit_id":"72dd3d1112a183bae6d24a27b3bab2c28f3304ac"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"6ac96cfa2860570a16d1330a362809653fe4ad56","unresolved":false,"context_lines":[{"line_number":151,"context_line":"            \u0027user-agent\u0027: \u0027obj-reconstructor %s\u0027 % os.getpid()}"},{"line_number":152,"context_line":"        self.handoffs_only \u003d config_true_value(conf.get(\u0027handoffs_only\u0027,"},{"line_number":153,"context_line":"                                                        False))"},{"line_number":154,"context_line":"        if not self.handoffs_only:"},{"line_number":155,"context_line":"            # honor handoffs_first for backwards compatibility"},{"line_number":156,"context_line":"            self.handoffs_only \u003d config_true_value(conf.get(\u0027handoffs_first\u0027,"},{"line_number":157,"context_line":"                                                            False))"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa31d9ce_ee8ef777","line":154,"updated":"2017-02-13 21:43:41.000000000","message":"So if you have\n\n handoffs_only \u003d False\n handoffs_first \u003d True\n\nwe still turn on handoffs_only? I feel like we should only drop back to handoffs_first if handoffs_only wasn\u0027t set at all.","commit_id":"39b5fbc3643850e7ad9340843891e9fb39a87f33"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a01dd1d1a4e8ec4d5ab1c9146e31545c1dd749f6","unresolved":false,"context_lines":[{"line_number":151,"context_line":"            \u0027user-agent\u0027: \u0027obj-reconstructor %s\u0027 % os.getpid()}"},{"line_number":152,"context_line":"        self.handoffs_only \u003d config_true_value(conf.get(\u0027handoffs_only\u0027,"},{"line_number":153,"context_line":"                                                        False))"},{"line_number":154,"context_line":"        if not self.handoffs_only:"},{"line_number":155,"context_line":"            # honor handoffs_first for backwards compatibility"},{"line_number":156,"context_line":"            self.handoffs_only \u003d config_true_value(conf.get(\u0027handoffs_first\u0027,"},{"line_number":157,"context_line":"                                                            False))"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa31d9ce_6955a1f8","line":154,"in_reply_to":"fa31d9ce_ee8ef777","updated":"2017-02-14 07:27:36.000000000","message":"I feel like this is already a bunch of crufty code - I didn\u0027t want to write (or maintain) the code (and tests) to handle logic depending on if it was in the file or not.\n\nI coded it up anyway; and it\u0027s was as gross as I imagined - I tried to make it less gross.  My least favorite part was trying (probably unsuccessfully) to explain the new logic in the commit message.\n\nI hope you like what I came up with :\\","commit_id":"39b5fbc3643850e7ad9340843891e9fb39a87f33"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"6ac96cfa2860570a16d1330a362809653fe4ad56","unresolved":false,"context_lines":[{"line_number":157,"context_line":"                                                            False))"},{"line_number":158,"context_line":"            if self.handoffs_only:"},{"line_number":159,"context_line":"                self.logger.warning("},{"line_number":160,"context_line":"                    \u0027The handoffs_first option is deprecated, the \u0027"},{"line_number":161,"context_line":"                    \u0027reconstructor has enabled handoffs_only mode.\u0027)"},{"line_number":162,"context_line":"        if self.handoffs_only:"},{"line_number":163,"context_line":"            self.logger.warning("}],"source_content_type":"text/x-python","patch_set":6,"id":"fa31d9ce_0ee34b34","line":160,"range":{"start_line":160,"start_character":47,"end_line":160,"end_character":60},"updated":"2017-02-13 21:43:41.000000000","message":"\u003e ... and will be removed in a future release...\n\nMay as well start prepping people for that reality now.","commit_id":"39b5fbc3643850e7ad9340843891e9fb39a87f33"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a01dd1d1a4e8ec4d5ab1c9146e31545c1dd749f6","unresolved":false,"context_lines":[{"line_number":157,"context_line":"                                                            False))"},{"line_number":158,"context_line":"            if self.handoffs_only:"},{"line_number":159,"context_line":"                self.logger.warning("},{"line_number":160,"context_line":"                    \u0027The handoffs_first option is deprecated, the \u0027"},{"line_number":161,"context_line":"                    \u0027reconstructor has enabled handoffs_only mode.\u0027)"},{"line_number":162,"context_line":"        if self.handoffs_only:"},{"line_number":163,"context_line":"            self.logger.warning("}],"source_content_type":"text/x-python","patch_set":6,"id":"fa31d9ce_8971f5aa","line":160,"in_reply_to":"fa31d9ce_0ee34b34","updated":"2017-02-14 07:27:36.000000000","message":"it\u0027s *deprecated* - I don\u0027t think making predictions about the future is helpful - but in case the meaning of deprecated is unclear I\u0027ll define it in the log message as:\n\nmay be ignored in a future release","commit_id":"39b5fbc3643850e7ad9340843891e9fb39a87f33"},{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"e796dfcec24c89c6b85359a9299407a218fdf28b","unresolved":false,"context_lines":[{"line_number":986,"context_line":"            else:"},{"line_number":987,"context_line":"                self.logger.warning(_("},{"line_number":988,"context_line":"                    \"Handoffs only mode found no handoffs remaining.  \""},{"line_number":989,"context_line":"                    \"You should disable handoffs_only immediately.\"))"},{"line_number":990,"context_line":""},{"line_number":991,"context_line":"    def run_once(self, *args, **kwargs):"},{"line_number":992,"context_line":"        start \u003d time.time()"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa31d9ce_28c00ae0","line":989,"updated":"2017-02-13 22:49:30.000000000","message":"maybe change this a little to say something that communicates: \ndisable handoffs_only once all reconstructors agree that no handoffs remain cluster-wide.  If it were true for sure they could disable immediately, then we could do it in a more automated way.","commit_id":"39b5fbc3643850e7ad9340843891e9fb39a87f33"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a01dd1d1a4e8ec4d5ab1c9146e31545c1dd749f6","unresolved":false,"context_lines":[{"line_number":986,"context_line":"            else:"},{"line_number":987,"context_line":"                self.logger.warning(_("},{"line_number":988,"context_line":"                    \"Handoffs only mode found no handoffs remaining.  \""},{"line_number":989,"context_line":"                    \"You should disable handoffs_only immediately.\"))"},{"line_number":990,"context_line":""},{"line_number":991,"context_line":"    def run_once(self, *args, **kwargs):"},{"line_number":992,"context_line":"        start \u003d time.time()"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa31d9ce_8be642ae","line":989,"in_reply_to":"fa31d9ce_28c00ae0","updated":"2017-02-14 07:27:36.000000000","message":"I\u0027m worry that anything less than \"disable immediately\" will not convey the appropriate sense of urgency.  I don\u0027t think there is a helpful way to include an operational run book in the log messages.  I changed it anyway.\n\nI hope you like what I came up with :\\","commit_id":"39b5fbc3643850e7ad9340843891e9fb39a87f33"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"52a8ff4e12c2e2bc1cecb83f5f6b0c0b7fb55e0e","unresolved":false,"context_lines":[{"line_number":152,"context_line":"        if \u0027handoffs_first\u0027 in conf:"},{"line_number":153,"context_line":"            self.logger.warning("},{"line_number":154,"context_line":"                \u0027The handoffs_first option is deprecated in favor \u0027"},{"line_number":155,"context_line":"                \u0027of handoffs_only.  This option may be ignored in a \u0027"},{"line_number":156,"context_line":"                \u0027future release.\u0027)"},{"line_number":157,"context_line":"            # honor handoffs_first for backwards compatibility"},{"line_number":158,"context_line":"            default_handoffs_only \u003d config_true_value(conf[\u0027handoffs_first\u0027])"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa31d9ce_82f4325b","line":155,"range":{"start_line":155,"start_character":34,"end_line":155,"end_character":36},"updated":"2017-02-15 02:33:52.000000000","message":"nits) 2 spaces here but 1 space seems enough.","commit_id":"da557011ecc1ec46bce24bd896ec50cbed914ba6"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"52a8ff4e12c2e2bc1cecb83f5f6b0c0b7fb55e0e","unresolved":false,"context_lines":[{"line_number":985,"context_line":"        if self.handoffs_only:"},{"line_number":986,"context_line":"            if self.handoffs_remaining \u003e 0:"},{"line_number":987,"context_line":"                self.logger.info(_("},{"line_number":988,"context_line":"                    \"Handoffs only mode still has handoffs remaining.  \""},{"line_number":989,"context_line":"                    \"Next pass will continue to revert handoffs.\"))"},{"line_number":990,"context_line":"            else:"},{"line_number":991,"context_line":"                self.logger.warning(_("}],"source_content_type":"text/x-python","patch_set":7,"id":"fa31d9ce_689e3fba","line":988,"range":{"start_line":988,"start_character":70,"end_line":988,"end_character":71},"updated":"2017-02-15 02:33:52.000000000","message":"nits) here also redundant space","commit_id":"da557011ecc1ec46bce24bd896ec50cbed914ba6"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"52a8ff4e12c2e2bc1cecb83f5f6b0c0b7fb55e0e","unresolved":false,"context_lines":[{"line_number":989,"context_line":"                    \"Next pass will continue to revert handoffs.\"))"},{"line_number":990,"context_line":"            else:"},{"line_number":991,"context_line":"                self.logger.warning(_("},{"line_number":992,"context_line":"                    \"Handoffs only mode found no handoffs remaining.  \""},{"line_number":993,"context_line":"                    \"You should disable handoffs_only once all nodes \""},{"line_number":994,"context_line":"                    \"are reporting no handoffs remaining.\"))"},{"line_number":995,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"fa31d9ce_88a1b3fa","line":992,"range":{"start_line":992,"start_character":69,"end_line":992,"end_character":70},"updated":"2017-02-15 02:33:52.000000000","message":"ditto.","commit_id":"da557011ecc1ec46bce24bd896ec50cbed914ba6"}],"test/unit/obj/test_reconstructor.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"11c4bc75c0488e316ff62047a63882f545a824b9","unresolved":false,"context_lines":[{"line_number":734,"context_line":""},{"line_number":735,"context_line":"        def fake_process_job(job):"},{"line_number":736,"context_line":"            # increment failure counter"},{"line_number":737,"context_line":"            self.reconstructor.handoffs_remaining +\u003d 1"},{"line_number":738,"context_line":"            found_job_types.add(job[\u0027job_type\u0027])"},{"line_number":739,"context_line":""},{"line_number":740,"context_line":"        self.reconstructor.process_job \u003d fake_process_job"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a461143_7e7b6028","line":737,"range":{"start_line":737,"start_character":17,"end_line":737,"end_character":54},"updated":"2017-01-30 23:56:54.000000000","message":"I don\u0027t think we ever test that this gets incremented in _revert...","commit_id":"3eb5857ed36dd7ae296a46359a60ab42b3b300a1"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5e66eac29fb63c1b8801c7a82b205db2319dbe21","unresolved":false,"context_lines":[{"line_number":734,"context_line":""},{"line_number":735,"context_line":"        def fake_process_job(job):"},{"line_number":736,"context_line":"            # increment failure counter"},{"line_number":737,"context_line":"            self.reconstructor.handoffs_remaining +\u003d 1"},{"line_number":738,"context_line":"            found_job_types.add(job[\u0027job_type\u0027])"},{"line_number":739,"context_line":""},{"line_number":740,"context_line":"        self.reconstructor.process_job \u003d fake_process_job"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a461143_7870aa72","line":737,"in_reply_to":"3a461143_679bb066","updated":"2017-01-31 23:35:11.000000000","message":"ahhhhh\n\n\t\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\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\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\n\tFAIL: test_process_job_revert_is_handoff_fails (test.unit.obj.test_reconstructor.TestObjectReconstructor)\n\t----------------------------------------------------------------------\n\tTraceback (most recent call last):\n\t  File \"/home/ubuntu/swift/test/unit/obj/test_reconstructor.py\", line 2436, in test_process_job_revert_is_handoff_fails\n\t\tself.assertEqual(self.reconstructor.handoffs_remaining, 1)\n\tAssertionError: 0 !\u003d 1\n\n\t----------------------------------------------------------------------\n\tRan 78 tests in 1.999s\n\n\tFAILED (failures\u003d1)","commit_id":"3eb5857ed36dd7ae296a46359a60ab42b3b300a1"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"75b3323eb69554cded0ea1c30c9ac3c8ef8b9f29","unresolved":false,"context_lines":[{"line_number":734,"context_line":""},{"line_number":735,"context_line":"        def fake_process_job(job):"},{"line_number":736,"context_line":"            # increment failure counter"},{"line_number":737,"context_line":"            self.reconstructor.handoffs_remaining +\u003d 1"},{"line_number":738,"context_line":"            found_job_types.add(job[\u0027job_type\u0027])"},{"line_number":739,"context_line":""},{"line_number":740,"context_line":"        self.reconstructor.process_job \u003d fake_process_job"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a461143_679bb066","line":737,"in_reply_to":"3a461143_7e7b6028","updated":"2017-01-31 20:09:16.000000000","message":"ah, yeah I\u0027m only testing *if* the job increments failure - not that it *does*\n\nconfirmed:\n\n\tdiff --git a/swift/obj/reconstructor.py b/swift/obj/reconstructor.py\n\tindex 1c6d033..89503f8 100644\n\t--- a/swift/obj/reconstructor.py\n\t+++ b/swift/obj/reconstructor.py\n\t@@ -668,8 +668,6 @@ class ObjectReconstructor(Daemon):\n\t\t\t if syncd_with \u003e\u003d len(job[\u0027sync_to\u0027]):\n\t\t\t\t self.delete_reverted_objs(\n\t\t\t\t\t job, reverted_objs, job[\u0027frag_index\u0027])\n\t-        else:\n\t-            self.handoffs_remaining +\u003d 1\n\t\t\t self.logger.timing_since(\u0027partition.delete.timing\u0027, begin)\n\t \n\t\t def _get_part_jobs(self, local_dev, part_path, partition, policy):\n\n\n\n^ doesn\u0027t cause test failures  #willfix","commit_id":"3eb5857ed36dd7ae296a46359a60ab42b3b300a1"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"11c4869f740d91a42a77ccc8fbf6999afb13127c","unresolved":false,"context_lines":[{"line_number":750,"context_line":"        found_job_types \u003d set()"},{"line_number":751,"context_line":""},{"line_number":752,"context_line":"        def fake_process_job(job):"},{"line_number":753,"context_line":"            # increment failure counter"},{"line_number":754,"context_line":"            found_job_types.add(job[\u0027job_type\u0027])"},{"line_number":755,"context_line":""},{"line_number":756,"context_line":"        self.reconstructor.process_job \u003d fake_process_job"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a461143_3f8f3676","line":753,"updated":"2017-01-30 08:14:14.000000000","message":"Copy and paste error, no failure counter being incremented.","commit_id":"3eb5857ed36dd7ae296a46359a60ab42b3b300a1"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5e66eac29fb63c1b8801c7a82b205db2319dbe21","unresolved":false,"context_lines":[{"line_number":750,"context_line":"        found_job_types \u003d set()"},{"line_number":751,"context_line":""},{"line_number":752,"context_line":"        def fake_process_job(job):"},{"line_number":753,"context_line":"            # increment failure counter"},{"line_number":754,"context_line":"            found_job_types.add(job[\u0027job_type\u0027])"},{"line_number":755,"context_line":""},{"line_number":756,"context_line":"        self.reconstructor.process_job \u003d fake_process_job"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a461143_18c966ac","line":753,"in_reply_to":"3a461143_27bc48ba","updated":"2017-01-31 23:35:11.000000000","message":"Done","commit_id":"3eb5857ed36dd7ae296a46359a60ab42b3b300a1"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"75b3323eb69554cded0ea1c30c9ac3c8ef8b9f29","unresolved":false,"context_lines":[{"line_number":750,"context_line":"        found_job_types \u003d set()"},{"line_number":751,"context_line":""},{"line_number":752,"context_line":"        def fake_process_job(job):"},{"line_number":753,"context_line":"            # increment failure counter"},{"line_number":754,"context_line":"            found_job_types.add(job[\u0027job_type\u0027])"},{"line_number":755,"context_line":""},{"line_number":756,"context_line":"        self.reconstructor.process_job \u003d fake_process_job"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a461143_27bc48ba","line":753,"in_reply_to":"3a461143_3f8f3676","updated":"2017-01-31 20:09:16.000000000","message":"ah I see - this is the counter case of the above - yeah the comment should be pointing out the difference is we\u0027re *not* incrementing a failure counter (which we expect to trigger the warning)  #willfix","commit_id":"3eb5857ed36dd7ae296a46359a60ab42b3b300a1"},{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"0619aa01fb9553ccd5cfdc04c3f17f2d2dae6c87","unresolved":false,"context_lines":[{"line_number":735,"context_line":"        def fake_process_job(job):"},{"line_number":736,"context_line":"            # increment failure counter"},{"line_number":737,"context_line":"            self.reconstructor.handoffs_remaining +\u003d 1"},{"line_number":738,"context_line":"            found_job_types.add(job[\u0027job_type\u0027])"},{"line_number":739,"context_line":""},{"line_number":740,"context_line":"        self.reconstructor.process_job \u003d fake_process_job"},{"line_number":741,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1a430d35_653b45d2","line":738,"updated":"2017-02-06 05:23:44.000000000","message":"At first, I was surprised that we are mocking the method which leads to the coverage of the changes from the review itself.  But I do see how there are 2 tests later that do cover the real _revert method that increments (or doesn\u0027t) the handoffs_remaining.  It would be nice to have a method that could go through the sequence of having handoffs, to not having anymore, and still use the real _revert.  But that might not be suitable for unittests, I don\u0027t know.","commit_id":"72dd3d1112a183bae6d24a27b3bab2c28f3304ac"},{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"092ec34cf27ddf3d8992f5c84c89c0931797c6f3","unresolved":false,"context_lines":[{"line_number":735,"context_line":"        def fake_process_job(job):"},{"line_number":736,"context_line":"            # increment failure counter"},{"line_number":737,"context_line":"            self.reconstructor.handoffs_remaining +\u003d 1"},{"line_number":738,"context_line":"            found_job_types.add(job[\u0027job_type\u0027])"},{"line_number":739,"context_line":""},{"line_number":740,"context_line":"        self.reconstructor.process_job \u003d fake_process_job"},{"line_number":741,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"fa31d9ce_61443174","line":738,"in_reply_to":"1a430d35_355227de","updated":"2017-02-10 16:55:38.000000000","message":"i could attempt to do this in a follow on patch later.","commit_id":"72dd3d1112a183bae6d24a27b3bab2c28f3304ac"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ddadbbd898c3769318ce5d6b3a9ceeee5652f6d7","unresolved":false,"context_lines":[{"line_number":735,"context_line":"        def fake_process_job(job):"},{"line_number":736,"context_line":"            # increment failure counter"},{"line_number":737,"context_line":"            self.reconstructor.handoffs_remaining +\u003d 1"},{"line_number":738,"context_line":"            found_job_types.add(job[\u0027job_type\u0027])"},{"line_number":739,"context_line":""},{"line_number":740,"context_line":"        self.reconstructor.process_job \u003d fake_process_job"},{"line_number":741,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1a430d35_355227de","line":738,"in_reply_to":"1a430d35_653b45d2","updated":"2017-02-06 19:10:43.000000000","message":"I think that would be suitable for unittests certainly - but didn\u0027t write it like that (lazy?).","commit_id":"72dd3d1112a183bae6d24a27b3bab2c28f3304ac"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"5b315a059bb0dc676dfaf531a8fd0e769876ae16","unresolved":false,"context_lines":[{"line_number":757,"context_line":""},{"line_number":758,"context_line":"        # only revert jobs ... but all handoffs cleared out successfully"},{"line_number":759,"context_line":"        self.reconstructor.reconstruct()"},{"line_number":760,"context_line":"        self.assertEqual(found_job_types, {object_reconstructor.REVERT})"},{"line_number":761,"context_line":"        # it\u0027s time to turn off handoffs_first"},{"line_number":762,"context_line":"        msgs \u003d self.reconstructor.logger.get_lines_for_level(\u0027warning\u0027)"},{"line_number":763,"context_line":"        self.assertIn(\u0027You should disable handoffs first\u0027, msgs[-1])"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a430d35_d2f10b6c","line":760,"updated":"2017-02-06 15:56:47.000000000","message":"Hmm... I\u0027m still wondering the impact of handoff_first usage, here. As you described in the commit message, this change implements really *handoffs_only*, not a handoff_first, different from the replicator behavior.\n\nProbably you know, the sorting of parts affects both the priority and *handoffs_remaining*\n\nIn the replicator, we have the parts list to replicate like as follows:\n\nH: Handoff\nP: Primary\n\n [H0 ... Hn][P0 ... Pn]\n\n\nbecause it\u0027s sorted. And the replicator may abort the latter [P0 ... Pn] if it has handoffs_remaining which means failures on udpate_deleted jobs. i.e. the replicator can continue primary jobs (i.e. update) if no failures happen in the handoff synchronization. However, now reconstructor will have the mixture of the list of:\n\n [H0, P0, P1, H1 ... Hn, Pn ] (for an instance)\n\nAnd the recosntructor skips all of primary jobs.\n\nOk, the improvement is in follow up. In the follow-up you wrote [1], it will prior to the handoffs as well as replicator like as:\n\n [H0 ... Hn][P0 ... Pn]\n\nIt *looks* to address the issue I described above, *HOWEVER*, I\u0027m still not sure, as you probably know, the reconstructor may have handoff fragments in the primary list (i.e. more than 1 frag indexes exist in the object_hash dir, probably it happens in the mixture case with failures * rebalance) so that you are trying to skip all primaries instead of *just handoffs_first*.\n\nHmm... I\u0027m not sure if we should address with the handoffs_first option. Because that is completely different from the replicator behavior which is able to continue primaries if it doesn\u0027t get failed with the handoffs (first!) syncs so that the operators could be confused with the difference as well as without this change. (I know, the current master also behaves as different)\n\nAnd thinking of the use case of handoff_first, I\u0027m feeling it could be ok to do sync_job if *handoff parts* didn\u0027t fail because the case that mixed frag indexes exist in a disk should be much less cases than just handoff partitions? IIRC, the case should be triggered by... reconstructor??? and only if the case is that the reconstructor pushes sync to a primary who was a handoff node keeping older handoff fragments???\n\n1: https://review.openstack.org/#/c/428408/5","commit_id":"72dd3d1112a183bae6d24a27b3bab2c28f3304ac"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ddadbbd898c3769318ce5d6b3a9ceeee5652f6d7","unresolved":false,"context_lines":[{"line_number":757,"context_line":""},{"line_number":758,"context_line":"        # only revert jobs ... but all handoffs cleared out successfully"},{"line_number":759,"context_line":"        self.reconstructor.reconstruct()"},{"line_number":760,"context_line":"        self.assertEqual(found_job_types, {object_reconstructor.REVERT})"},{"line_number":761,"context_line":"        # it\u0027s time to turn off handoffs_first"},{"line_number":762,"context_line":"        msgs \u003d self.reconstructor.logger.get_lines_for_level(\u0027warning\u0027)"},{"line_number":763,"context_line":"        self.assertIn(\u0027You should disable handoffs first\u0027, msgs[-1])"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a430d35_75a58fb3","line":760,"in_reply_to":"1a430d35_d2f10b6c","updated":"2017-02-06 19:10:43.000000000","message":"Your understanding of the ordering of the jobs and the handling is 100% correct.  I don\u0027t think the followup patch is really fleshed out yet - it needs more thought and discussion.\n\nYes, I think reconstructor handoffs_first behavior on master is very very bad - and operations desperately needs reconstructor to have effective method to prioritize rebalance work.","commit_id":"72dd3d1112a183bae6d24a27b3bab2c28f3304ac"},{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"e0eba2e06c99f218a41a94046612b5c5561eb2ab","unresolved":false,"context_lines":[{"line_number":733,"context_line":"            self.conf, logger\u003dself.logger)"},{"line_number":734,"context_line":"        self.assertTrue(self.reconstructor.handoffs_only)"},{"line_number":735,"context_line":"        warnings \u003d self.logger.get_lines_for_level(\u0027warning\u0027)"},{"line_number":736,"context_line":"        warning1, warning2 \u003d warnings"},{"line_number":737,"context_line":"        self.assertIn(\u0027handoffs_first option is deprecated\u0027, warning1)"},{"line_number":738,"context_line":"        self.assertIn(\u0027not intended for normal operation\u0027, warning2)"},{"line_number":739,"context_line":"        self.assertIn(\u0027use handoffs_only with care\u0027, warning2)"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa31d9ce_cd81a74f","line":736,"updated":"2017-02-10 08:53:02.000000000","message":"nit: why an extra assignment?\n\nwarning1, warning2 \u003d self.logger.get_lines_for_level(\u0027warning\u0027) seems to work","commit_id":"d603e4be521a1076de323360b040c3b80e61cded"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"435ec14d4bbd5ff6cfdcf888c890b40bf3b174f4","unresolved":false,"context_lines":[{"line_number":733,"context_line":"            self.conf, logger\u003dself.logger)"},{"line_number":734,"context_line":"        self.assertTrue(self.reconstructor.handoffs_only)"},{"line_number":735,"context_line":"        warnings \u003d self.logger.get_lines_for_level(\u0027warning\u0027)"},{"line_number":736,"context_line":"        warning1, warning2 \u003d warnings"},{"line_number":737,"context_line":"        self.assertIn(\u0027handoffs_first option is deprecated\u0027, warning1)"},{"line_number":738,"context_line":"        self.assertIn(\u0027not intended for normal operation\u0027, warning2)"},{"line_number":739,"context_line":"        self.assertIn(\u0027use handoffs_only with care\u0027, warning2)"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa31d9ce_fbf52683","line":736,"in_reply_to":"fa31d9ce_cd81a74f","updated":"2017-02-10 22:12:36.000000000","message":"Done","commit_id":"d603e4be521a1076de323360b040c3b80e61cded"},{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"e796dfcec24c89c6b85359a9299407a218fdf28b","unresolved":false,"context_lines":[{"line_number":727,"context_line":"                             object_reconstructor.REVERT))"},{"line_number":728,"context_line":"            self.assert_expected_jobs(part_info[\u0027partition\u0027], jobs)"},{"line_number":729,"context_line":""},{"line_number":730,"context_line":"    def test_handoffs_first_enables_handoffs_only(self):"},{"line_number":731,"context_line":"        self.conf[\u0027handoffs_first\u0027] \u003d True"},{"line_number":732,"context_line":"        self.reconstructor \u003d object_reconstructor.ObjectReconstructor("},{"line_number":733,"context_line":"            self.conf, logger\u003dself.logger)"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa31d9ce_40f51255","line":730,"updated":"2017-02-13 22:49:30.000000000","message":"I see this is in the \u0027TestGlobalSetupObjectReconstructor\u0027 class.  I wondered how this is different than the \u0027TestObjectReconstructor\u0027 class.  The description there says it is for \"Tests for reconstructor using real objects in test partition directories.\"  In this case, the test doesn\u0027t require real objects.  Should it be in the \u0027TestObjectReconstructor\u0027 instead?  I understand that it might be better here so that it is near the \u0027test_handoffs_only\u0027 which does require the real directories.  But just a thought.","commit_id":"39b5fbc3643850e7ad9340843891e9fb39a87f33"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a01dd1d1a4e8ec4d5ab1c9146e31545c1dd749f6","unresolved":false,"context_lines":[{"line_number":727,"context_line":"                             object_reconstructor.REVERT))"},{"line_number":728,"context_line":"            self.assert_expected_jobs(part_info[\u0027partition\u0027], jobs)"},{"line_number":729,"context_line":""},{"line_number":730,"context_line":"    def test_handoffs_first_enables_handoffs_only(self):"},{"line_number":731,"context_line":"        self.conf[\u0027handoffs_first\u0027] \u003d True"},{"line_number":732,"context_line":"        self.reconstructor \u003d object_reconstructor.ObjectReconstructor("},{"line_number":733,"context_line":"            self.conf, logger\u003dself.logger)"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa31d9ce_46c99f25","line":730,"in_reply_to":"fa31d9ce_40f51255","updated":"2017-02-14 07:27:36.000000000","message":"I added a few other tests to cover config handling, and added some additional assertions about handoffs_only_enabled - so I at this point I think it\u0027s worth moving.","commit_id":"39b5fbc3643850e7ad9340843891e9fb39a87f33"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"52a8ff4e12c2e2bc1cecb83f5f6b0c0b7fb55e0e","unresolved":false,"context_lines":[{"line_number":1192,"context_line":""},{"line_number":1193,"context_line":"    def test_handoffs_first_enables_handoffs_only(self):"},{"line_number":1194,"context_line":"        self.conf.pop(\u0027handoffs_only\u0027, None)  # sanity"},{"line_number":1195,"context_line":"        self.conf[\u0027handoffs_first\u0027] \u003d True"},{"line_number":1196,"context_line":"        self.reconstructor \u003d object_reconstructor.ObjectReconstructor("},{"line_number":1197,"context_line":"            self.conf, logger\u003dself.logger)"},{"line_number":1198,"context_line":"        self.assertTrue(self.reconstructor.handoffs_only)"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa31d9ce_45ede4b5","line":1195,"range":{"start_line":1195,"start_character":38,"end_line":1195,"end_character":42},"updated":"2017-02-15 02:33:52.000000000","message":"I found the value should be string for emulating actual conf dict but it\u0027s ok in the follow up because all tests are passed even I changed those as string.","commit_id":"da557011ecc1ec46bce24bd896ec50cbed914ba6"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"f539926cb05633e4b6bf294fa92ab95ee6df767a","unresolved":false,"context_lines":[{"line_number":1231,"context_line":"            \u0027Handoff only mode is not intended for normal operation, \u0027"},{"line_number":1232,"context_line":"            \u0027use handoffs_only with care.\u0027,"},{"line_number":1233,"context_line":"        ]"},{"line_number":1234,"context_line":"        self.assertEqual(expected, warnings)"},{"line_number":1235,"context_line":""},{"line_number":1236,"context_line":"    def test_two_ec_policies(self):"},{"line_number":1237,"context_line":"        with patch_policies(["}],"source_content_type":"text/x-python","patch_set":7,"id":"fa31d9ce_45926bf7","line":1234,"updated":"2017-02-14 10:36:50.000000000","message":"I think my opinion is at nit picks but is that worth to add the tests in following cases to check the state greedy?\n\n- handoffs_first \u003d True, handoffs_only \u003d True (should it log deprecated warning? it looks no warning for now. And then working as handoffs only)\n- handoffs_first \u003d False, handoffs_only \u003d None (should it log depreacated warning? anyway both of them off)\n- handoffs_first \u003d False, handoffs_only \u003d False (should it log deprecated warning? both of them off)\n- handoffs_first \u003d None, handoffs_only \u003d False (no warnings and the handoffs_only off)\n\nIt could be ok in follow up, though.","commit_id":"da557011ecc1ec46bce24bd896ec50cbed914ba6"}]}