)]}'
{"swift/obj/reconstructor.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"2c535c8d44a99f5fa4dfa58bbc2d9a01ba657dbd","unresolved":false,"context_lines":[{"line_number":858,"context_line":"                    job_type\u003dREVERT,"},{"line_number":859,"context_line":"                    frag_index\u003dNone,"},{"line_number":860,"context_line":"                    suffixes\u003dnon_data_fragment_suffixes,"},{"line_number":861,"context_line":"                    sync_to\u003drandom.sample(part_nodes, policy.ec_nparity + 1)"},{"line_number":862,"context_line":"                ))"},{"line_number":863,"context_line":"        # return a list of jobs for this part"},{"line_number":864,"context_line":"        return jobs"}],"source_content_type":"text/x-python","patch_set":9,"id":"1f013ff3_ea685722","line":861,"updated":"2017-05-12 19:15:18.000000000","message":"you forgot the duplication factor ;)\n\nHonestly I wish we could go lower - but this may be the safest answer.","commit_id":"6ec1df2c18b69482909b8110f85a46cadc750340"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"d33fa53069926e03e90e3ebc0359417e14fad89e","unresolved":false,"context_lines":[{"line_number":859,"context_line":"                    frag_index\u003dNone,"},{"line_number":860,"context_line":"                    suffixes\u003dnon_data_fragment_suffixes,"},{"line_number":861,"context_line":"                    sync_to\u003drandom.sample(part_nodes, ("},{"line_number":862,"context_line":"                        policy.ec_nparity + 1) * policy.ec_duplication_factor)"},{"line_number":863,"context_line":"                ))"},{"line_number":864,"context_line":"        # return a list of jobs for this part"},{"line_number":865,"context_line":"        return jobs"}],"source_content_type":"text/x-python","patch_set":10,"id":"df140735_f6a6be53","line":862,"range":{"start_line":862,"start_character":24,"end_line":862,"end_character":77},"updated":"2017-05-31 07:21:21.000000000","message":"From the discussion in the test module, the last piece is how many number of primaries is sufficient to ensure the tombstone existence. Exactly small number of primaries saves the connection costs and it would be sufficient but same number of quorum for DELETE could be intuitive for us?\n\nHow do you think of?","commit_id":"ad25ee6237199f7fcfc6a1d64b3b3cea997e3418"}],"test/probe/test_reconstructor_revert.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"bea18f3398919cf74d96f4d8ef6d4a3326755618","unresolved":false,"context_lines":[{"line_number":175,"context_line":"                          self.object_name, contents\u003dcontents)"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"        # now lets shut down one primary"},{"line_number":178,"context_line":"        failed_nodes \u003d random.sample(onodes, 1)"},{"line_number":179,"context_line":"        for node in failed_nodes:"},{"line_number":180,"context_line":"            self.kill_drive(self.device_dir(\u0027object\u0027, node))"},{"line_number":181,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9a30ddce_b512e260","line":178,"updated":"2017-03-07 18:09:52.000000000","message":"I *think* in our default 4+2, we should be able to still push the tombstone with a couple of primaries down (I think we\u0027d need 3 up) - but it requires getting a little more lucky","commit_id":"f6b897848bb3c7dccf40d8167ffe138ac4166a81"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"bea18f3398919cf74d96f4d8ef6d4a3326755618","unresolved":false,"context_lines":[{"line_number":201,"context_line":"        # primaries each time"},{"line_number":202,"context_line":"        timeout \u003d time.time() + 10"},{"line_number":203,"context_line":"        while time.time() \u003c timeout:"},{"line_number":204,"context_line":"            self.reconstructor.once(number\u003dself.config_number(hnodes[0]))"},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"        # make sure tombstone is reverted"},{"line_number":207,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a30ddce_f589bacb","line":204,"updated":"2017-03-07 18:09:52.000000000","message":"we should break out of this loop as soon as we find the tombstone is reverted, and only fail if it\u0027s still unavailable after a timeout","commit_id":"f6b897848bb3c7dccf40d8167ffe138ac4166a81"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c9a92c16b7a5c8f4a3841e59fc56da636f247f4d","unresolved":false,"context_lines":[{"line_number":167,"context_line":"        opart, onodes \u003d self.object_ring.get_nodes("},{"line_number":168,"context_line":"            self.account, self.container_name, self.object_name)"},{"line_number":169,"context_line":"        hnodes \u003d list(itertools.islice("},{"line_number":170,"context_line":"            self.object_ring.get_more_nodes(opart), 1))"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"        # PUT object"},{"line_number":173,"context_line":"        contents \u003d Body()"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a30ddce_18004334","line":170,"updated":"2017-03-07 18:35:34.000000000","message":"this is apparently fine, but not needed?  and akward to cary around a list with one element?","commit_id":"625174e17c3e4617e94c14519275a904aab434be"},{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"d45ca2adf3c3ea8e2c8dba1748607aa47ed7361a","unresolved":false,"context_lines":[{"line_number":167,"context_line":"        opart, onodes \u003d self.object_ring.get_nodes("},{"line_number":168,"context_line":"            self.account, self.container_name, self.object_name)"},{"line_number":169,"context_line":"        hnodes \u003d list(itertools.islice("},{"line_number":170,"context_line":"            self.object_ring.get_more_nodes(opart), 1))"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"        # PUT object"},{"line_number":173,"context_line":"        contents \u003d Body()"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a30ddce_614c1071","line":170,"in_reply_to":"9a30ddce_18004334","updated":"2017-03-08 09:55:13.000000000","message":"Done. No more incorrect, now that we\u0027re checking tombstone on the other handoff as well.","commit_id":"625174e17c3e4617e94c14519275a904aab434be"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c9a92c16b7a5c8f4a3841e59fc56da636f247f4d","unresolved":false,"context_lines":[{"line_number":174,"context_line":"        client.put_object(self.url, self.token, self.container_name,"},{"line_number":175,"context_line":"                          self.object_name, contents\u003dcontents)"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"        # now lets shut down one primary"},{"line_number":178,"context_line":"        failed_nodes \u003d random.sample(onodes, 2)"},{"line_number":179,"context_line":"        for node in failed_nodes:"},{"line_number":180,"context_line":"            self.kill_drive(self.device_dir(\u0027object\u0027, node))"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a30ddce_98f35373","line":177,"updated":"2017-03-07 18:35:34.000000000","message":"oh, i guess this comment is wrong now","commit_id":"625174e17c3e4617e94c14519275a904aab434be"},{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"d45ca2adf3c3ea8e2c8dba1748607aa47ed7361a","unresolved":false,"context_lines":[{"line_number":174,"context_line":"        client.put_object(self.url, self.token, self.container_name,"},{"line_number":175,"context_line":"                          self.object_name, contents\u003dcontents)"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"        # now lets shut down one primary"},{"line_number":178,"context_line":"        failed_nodes \u003d random.sample(onodes, 2)"},{"line_number":179,"context_line":"        for node in failed_nodes:"},{"line_number":180,"context_line":"            self.kill_drive(self.device_dir(\u0027object\u0027, node))"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a30ddce_bd30ea02","line":177,"in_reply_to":"9a30ddce_98f35373","updated":"2017-03-08 09:55:13.000000000","message":"Done","commit_id":"625174e17c3e4617e94c14519275a904aab434be"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c9a92c16b7a5c8f4a3841e59fc56da636f247f4d","unresolved":false,"context_lines":[{"line_number":214,"context_line":"                    break"},{"line_number":215,"context_line":"        else:"},{"line_number":216,"context_line":"            self.fail(\u0027Still found tombstone on %r after %s iterations\u0027 % ("},{"line_number":217,"context_line":"                hnodes[0], iterations))"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"        # sanity make sure proxy get can\u0027t find it"},{"line_number":220,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a30ddce_db25c5ab","line":217,"updated":"2017-03-07 18:35:34.000000000","message":"maybe for lagniappe we could assert the tombstone is *still* on the *other* handoff we were *not* calling the reconstructor on and then repair the primary and assert the tombstone is reverted on the first pass!","commit_id":"625174e17c3e4617e94c14519275a904aab434be"},{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"d45ca2adf3c3ea8e2c8dba1748607aa47ed7361a","unresolved":false,"context_lines":[{"line_number":214,"context_line":"                    break"},{"line_number":215,"context_line":"        else:"},{"line_number":216,"context_line":"            self.fail(\u0027Still found tombstone on %r after %s iterations\u0027 % ("},{"line_number":217,"context_line":"                hnodes[0], iterations))"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"        # sanity make sure proxy get can\u0027t find it"},{"line_number":220,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a30ddce_5d3feed5","line":217,"in_reply_to":"9a30ddce_db25c5ab","updated":"2017-03-08 09:55:13.000000000","message":"Done","commit_id":"625174e17c3e4617e94c14519275a904aab434be"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"e4254afbb86856e6ec657194de19e99a8f97df10","unresolved":false,"context_lines":[{"line_number":229,"context_line":"            self.fail(\u0027Found obj data on %r\u0027 % hnodes[1])"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"        # repair the second primary"},{"line_number":232,"context_line":"        self.revive_drive(self.device_dir(\u0027object\u0027, failed_nodes[1]))"},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"        # run reconstructor on second handoff"},{"line_number":235,"context_line":"        self.reconstructor.once(number\u003dself.config_number(hnodes[1]))"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a30ddce_c7d6ac26","line":232,"updated":"2017-03-08 10:48:46.000000000","message":"perhaps to guarantee the first pass we have to repair *both* primaries (makes sense, unlucky sync_to could prevent successful revert and cleanup)","commit_id":"5d2527fdb7304f6bdadb0a5259f70314d78e17c6"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"0dd87500448b144b8eb2f7ca4c77f438740ecf20","unresolved":false,"context_lines":[{"line_number":216,"context_line":"                    break"},{"line_number":217,"context_line":"        else:"},{"line_number":218,"context_line":"            self.fail(\u0027Still found tombstone on %r after %s iterations\u0027 % ("},{"line_number":219,"context_line":"                hnodes[0], iterations))"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"        # tombstone is still on the *second* handoff"},{"line_number":222,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a30ddce_8cecb590","line":219,"updated":"2017-03-09 06:32:20.000000000","message":"Not super convinced trying to run the reconstructor as many times until it\u0027s gone for 30 seconds is the best. Time in tests are sucky. But I guess probe tests are OK if they do end up taking longer. Could we just run it \u0027x\u0027 times, or are we worried it\u0027ll hang?\nAnd if so, can we try \u0027x\u0027 times and then just timeout each call?\n\n  retries \u003d 10\n  for _ in range(retries):\n      try:\n          with Timeout(10):\n              self.reconstructor.once(number\u003d...\n              ...\n          if \u0027X-Backend-Timestamp\u0027 not in err.http_headers:\n              break\n      except Timeout:\n          continue\n  else:\n      ...\n\nObviously we\u0027d need to pick numbers that work.\n\nI think I need to think on this some more. Maybe there is a reason, or it\u0027s just eaiser and then I\u0027m happy to be convinced ;)","commit_id":"c3c20f5c6fe6ed6659c8a2e2a70796f32142ae16"},{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"8f9d5a988ce78eb8dde2bab032ee6215fad55624","unresolved":false,"context_lines":[{"line_number":216,"context_line":"                    break"},{"line_number":217,"context_line":"        else:"},{"line_number":218,"context_line":"            self.fail(\u0027Still found tombstone on %r after %s iterations\u0027 % ("},{"line_number":219,"context_line":"                hnodes[0], iterations))"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"        # tombstone is still on the *second* handoff"},{"line_number":222,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a22e58a_59e74512","line":219,"in_reply_to":"7a25e98e_b7812750","updated":"2017-03-14 12:57:34.000000000","message":"yes, it makes sense, thanks for the details!\n\ngiving \"retries \u003d 15\" failed the test, I settled for 25. Let\u0027s see what Jenkins has to say.","commit_id":"c3c20f5c6fe6ed6659c8a2e2a70796f32142ae16"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"81a2d02fff043dfe7c9e9c75c72c7e00eb6b5d2c","unresolved":false,"context_lines":[{"line_number":216,"context_line":"                    break"},{"line_number":217,"context_line":"        else:"},{"line_number":218,"context_line":"            self.fail(\u0027Still found tombstone on %r after %s iterations\u0027 % ("},{"line_number":219,"context_line":"                hnodes[0], iterations))"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"        # tombstone is still on the *second* handoff"},{"line_number":222,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":6,"id":"7a25e98e_b7812750","line":219,"in_reply_to":"9a30ddce_402d1494","updated":"2017-03-14 00:08:31.000000000","message":"Sure, yeah I get that, my point is. We wait 30 seconds, which is a random number of iterations. Some peoples machine may make 10 iterations before it fails, some may make 5 (these are numbers I\u0027m pulling out of the air not based on the actual code).\nSo I feel like, just having a flat 30 seconds and running the reconstructor as many times as it can, could potentially lead to a flaky test. Ie tends to fail more quickly on different peoples machines depending on resources.\n\nwhereas my approach was saying lets run it 10 times (I guess with a some timeout) so we always try at least x times before a failure. But still move on once the timestamp is removed.\n\nThough however, sure maybe I\u0027m being a tad nit picky, as peoples machines still may fail based on the timeouts. I guess i\u0027m not quite liking, run this thing as much as you can for the next 30 seconds or until its fixed. I\u0027d ranther run it x times or until is\u0027s fixed. But if no one else feels the same way. I\u0027m happy to change my vote :)","commit_id":"c3c20f5c6fe6ed6659c8a2e2a70796f32142ae16"},{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"9a0435f36a095bae6c68c0f4a6df4efb5e3d7c4d","unresolved":false,"context_lines":[{"line_number":216,"context_line":"                    break"},{"line_number":217,"context_line":"        else:"},{"line_number":218,"context_line":"            self.fail(\u0027Still found tombstone on %r after %s iterations\u0027 % ("},{"line_number":219,"context_line":"                hnodes[0], iterations))"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"        # tombstone is still on the *second* handoff"},{"line_number":222,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a30ddce_402d1494","line":219,"in_reply_to":"9a30ddce_8cecb590","updated":"2017-03-09 13:11:30.000000000","message":"Not sure I get the point - because we\u0027re breaking the loop (line#216) once we know the tombstone is gone and are not really waiting for 30 seconds to finish right?","commit_id":"c3c20f5c6fe6ed6659c8a2e2a70796f32142ae16"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c14d666be6e58c322327674fb9d368ba3866af68","unresolved":false,"context_lines":[{"line_number":202,"context_line":"        # tombstone is pushed out - each handoff node syncs to a few"},{"line_number":203,"context_line":"        # primaries each time"},{"line_number":204,"context_line":"        iterations \u003d 0"},{"line_number":205,"context_line":"        retries \u003d 25"},{"line_number":206,"context_line":"        for _ in range(retries):"},{"line_number":207,"context_line":"            iterations +\u003d 1"},{"line_number":208,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a1ff146_d9d25534","line":205,"updated":"2017-03-15 02:22:57.000000000","message":"So I\u0027m definitely fine with this; one bit of \"obviousness\" that I think is lost is the total wall time a developer might think the test should last in the failure case...\n\nAround 4 mins I suppose assuming the Timeout works reliably [1].  Unless the reconstructor can cycle quickly enough - in which case failure might only take a few seconds?  Hrm....\n\n1. IME that timeout does not work like that, but if I made reconstructor\u0027s run_once go to sleep something like this seemed to do the trick:\n\n    diff --git a/test/probe/test_reconstructor_revert.py b/test/probe/test_reconstructor_revert.py\n    index 6892096..2da1a05 100644\n    --- a/test/probe/test_reconstructor_revert.py\n    +++ b/test/probe/test_reconstructor_revert.py\n    @@ -20,7 +20,7 @@ import unittest\n     import uuid\n     import random\n     import shutil\n    -from eventlet import Timeout\n    +from eventlet import Timeout, spawn\n     from collections import defaultdict\n     \n     from test.probe.common import ECProbeTest, Body\n    @@ -207,8 +207,9 @@ class TestReconstructorRevert(ECProbeTest):\n                 iterations +\u003d 1\n                 try:\n                     with Timeout(10):\n    -                    self.reconstructor.once(\n    -                        number\u003dself.config_number(hnodes[0]))\n    +                    gt \u003d spawn(self.reconstructor.once,\n    +                               number\u003dself.config_number(hnodes[0]))\n    +                    gt.wait()\n                     try:\n                         self.direct_get(hnodes[0], opart)\n                     except direct_client.DirectClientException as err:\n    @@ -216,6 +217,9 @@ class TestReconstructorRevert(ECProbeTest):\n                         if \u0027X-Backend-Timestamp\u0027 not in err.http_headers:\n                             break\n                 except Timeout:\n    +                print \u0027*\u0027 * 50\n    +                print \u0027hit timeout!\u0027\n    +                print \u0027*\u0027 * 50\n                     continue\n             else:\n                 self.fail(\u0027Still found tombstone on %r after %s iterations\u0027 % (","commit_id":"824494fd1f3c40599a47d53e94706e7e926b5c1c"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c14d666be6e58c322327674fb9d368ba3866af68","unresolved":false,"context_lines":[{"line_number":204,"context_line":"        iterations \u003d 0"},{"line_number":205,"context_line":"        retries \u003d 25"},{"line_number":206,"context_line":"        for _ in range(retries):"},{"line_number":207,"context_line":"            iterations +\u003d 1"},{"line_number":208,"context_line":"            try:"},{"line_number":209,"context_line":"                with Timeout(10):"},{"line_number":210,"context_line":"                    self.reconstructor.once("}],"source_content_type":"text/x-python","patch_set":7,"id":"3a1ff146_f9d71121","line":207,"updated":"2017-03-15 02:22:57.000000000","message":"the value of iterations and `_` are *very* similar.","commit_id":"824494fd1f3c40599a47d53e94706e7e926b5c1c"},{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"eee0880b0fcd3f5c414af529d4b1145a1b975566","unresolved":false,"context_lines":[{"line_number":204,"context_line":"        iterations \u003d 0"},{"line_number":205,"context_line":"        retries \u003d 25"},{"line_number":206,"context_line":"        for _ in range(retries):"},{"line_number":207,"context_line":"            iterations +\u003d 1"},{"line_number":208,"context_line":"            try:"},{"line_number":209,"context_line":"                with Timeout(10):"},{"line_number":210,"context_line":"                    self.reconstructor.once("}],"source_content_type":"text/x-python","patch_set":7,"id":"3a1ff146_192bf0ec","line":207,"in_reply_to":"3a1ff146_f9d71121","updated":"2017-03-15 12:35:44.000000000","message":"ugh. my bad! I wanted to use this when failing the test, but as Matt points out, it\u0027s not needed (else reaches only when it finishes anyway)","commit_id":"824494fd1f3c40599a47d53e94706e7e926b5c1c"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c14d666be6e58c322327674fb9d368ba3866af68","unresolved":false,"context_lines":[{"line_number":214,"context_line":"                except direct_client.DirectClientException as err:"},{"line_number":215,"context_line":"                    self.assertEqual(err.http_status, 404)"},{"line_number":216,"context_line":"                    if \u0027X-Backend-Timestamp\u0027 not in err.http_headers:"},{"line_number":217,"context_line":"                        break"},{"line_number":218,"context_line":"            except Timeout:"},{"line_number":219,"context_line":"                continue"},{"line_number":220,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a1ff146_99f1bdd0","line":217,"updated":"2017-03-15 02:22:57.000000000","message":"as long as we still break as soon as we notice the expected condition I\u0027m happy!","commit_id":"824494fd1f3c40599a47d53e94706e7e926b5c1c"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"153dadcf228d2131dc2a7a72127ed2812266d969","unresolved":false,"context_lines":[{"line_number":219,"context_line":"                continue"},{"line_number":220,"context_line":"        else:"},{"line_number":221,"context_line":"            self.fail(\u0027Still found tombstone on %r after %s iterations\u0027 % ("},{"line_number":222,"context_line":"                hnodes[0], iterations))"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"        # tombstone is still on the *second* handoff"},{"line_number":225,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a1ff146_1dc132e5","line":222,"updated":"2017-03-15 01:29:38.000000000","message":"This else is matched with the for loop, so the else will only trigger if it finishes.. so the iterations will always be retries, so there is no need to count interations.\n\nWe also could play around with the timeout a bit. If it takes 25 retries, on a 10 second timeout we could think about extending the timeout. I might have a play with this on an SAIO.","commit_id":"824494fd1f3c40599a47d53e94706e7e926b5c1c"},{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"eee0880b0fcd3f5c414af529d4b1145a1b975566","unresolved":false,"context_lines":[{"line_number":219,"context_line":"                continue"},{"line_number":220,"context_line":"        else:"},{"line_number":221,"context_line":"            self.fail(\u0027Still found tombstone on %r after %s iterations\u0027 % ("},{"line_number":222,"context_line":"                hnodes[0], iterations))"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"        # tombstone is still on the *second* handoff"},{"line_number":225,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a1ff146_197930fd","line":222,"in_reply_to":"3a1ff146_1dc132e5","updated":"2017-03-15 12:35:44.000000000","message":"Right about the iterations part, my bad.","commit_id":"824494fd1f3c40599a47d53e94706e7e926b5c1c"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"324747a8202aaca724b313d5c6382860b06c0e16","unresolved":false,"context_lines":[{"line_number":201,"context_line":"        # tombstone is pushed out - each handoff node syncs to a few"},{"line_number":202,"context_line":"        # primaries each time"},{"line_number":203,"context_line":"        iterations \u003d 0"},{"line_number":204,"context_line":"        while iterations \u003c 52:"},{"line_number":205,"context_line":"            self.reconstructor.once(number\u003dself.config_number(hnodes[0]))"},{"line_number":206,"context_line":"            iterations +\u003d 1"},{"line_number":207,"context_line":"            # see if the tombstone is reverted"}],"source_content_type":"text/x-python","patch_set":9,"id":"1f013ff3_6bff7b01","line":204,"updated":"2017-05-16 06:21:29.000000000","message":"We should mention in the comment why we picked 52:\n\n  # A max of 52 iterations was chosen as in testing it gave us 5x9 (99.999%) assurance\n  # that everything would have been moved. Although in most cases only \u003c 10 would be\n  # needed. This will ensure no intermitant failures.","commit_id":"6ec1df2c18b69482909b8110f85a46cadc750340"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"a77c85a83a0f1d8724dbfb8e4b9c342cf292610a","unresolved":false,"context_lines":[{"line_number":214,"context_line":"                    break"},{"line_number":215,"context_line":"        else:"},{"line_number":216,"context_line":"            self.fail(\u0027Still found tombstone on %r after %s iterations\u0027 % ("},{"line_number":217,"context_line":"                hnodes[0], iterations))"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"        # tombstone is still on the *second* handoff"},{"line_number":220,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":10,"id":"1f013ff3_026555e1","line":217,"updated":"2017-05-17 09:25:42.000000000","message":"I\u0027m curious why we didn\u0027t have the assertion for client side (i.e. self.proxy_get() or client.get_object) around here.\n\nI\u0027m feeling this change is on the border of the trade-off things for the client effect vs reconstructor efficiency.\n\nThat is because 404s less than quorum will be a 503, right?? [1]\n\nHowever, when I tried self.proxy_get() here, it still be able to capture 404 correctly, maybe, the proxy refers handoffs those are sufficient number of 404s. (not yet dig deeply, though)\n\n\nHmm.... I may be still missing something. I tried also the case to decrease request_node_count as 1 * replica but the proxy always return 404 even before the drives revive.\n\n\n1: https://github.com/openstack/swift/blob/master/swift/proxy/controllers/obj.py#L2285-L2290","commit_id":"ad25ee6237199f7fcfc6a1d64b3b3cea997e3418"},{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"b4b5a0368e9c0382df912371ffdd3dbbae8441c4","unresolved":false,"context_lines":[{"line_number":214,"context_line":"                    break"},{"line_number":215,"context_line":"        else:"},{"line_number":216,"context_line":"            self.fail(\u0027Still found tombstone on %r after %s iterations\u0027 % ("},{"line_number":217,"context_line":"                hnodes[0], iterations))"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"        # tombstone is still on the *second* handoff"},{"line_number":220,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":10,"id":"ff0f0b1f_ba03c60f","line":217,"in_reply_to":"1f013ff3_026555e1","updated":"2017-05-17 16:10:50.000000000","message":"Not sure I completely capture your question: we\u0027re only running reconstructor until we don\u0027t find tombstone on the first handoff. We do the self.proxy_get on line#247 when we push off the tombstone from second handoff node as well.","commit_id":"ad25ee6237199f7fcfc6a1d64b3b3cea997e3418"},{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"998ad16ae1351176aa026274e14ec9bfd6ecb0cc","unresolved":false,"context_lines":[{"line_number":214,"context_line":"                    break"},{"line_number":215,"context_line":"        else:"},{"line_number":216,"context_line":"            self.fail(\u0027Still found tombstone on %r after %s iterations\u0027 % ("},{"line_number":217,"context_line":"                hnodes[0], iterations))"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"        # tombstone is still on the *second* handoff"},{"line_number":220,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":10,"id":"ff0f0b1f_3146b423","line":217,"in_reply_to":"ff0f0b1f_22f4eb85","updated":"2017-05-26 02:34:47.000000000","message":"Hi Kota,\n\nLet me try to address your two points:\n\n1) Your original concern that we might return 503 when we have less than quorum responses (in this case parity + 1 requests) might be a trade off in this change, leaning towards reconstructor efficiency IMO. But maybe we could have a broader discussion on that involving Clay.\n\n2) I agree the test is not as straight forward - to test the random sampling of parity + 1 nodes, the test is checking that within a given an interval (which was picked out from testing, see Matt\u0027s comment), there is no tombstone on the handoff, even before reviving the primaries that were down - which was not possible previously since we were trying to talk to *all* primaries prior to this change. So in effect, the change is being tested IMO.","commit_id":"ad25ee6237199f7fcfc6a1d64b3b3cea997e3418"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"d33fa53069926e03e90e3ebc0359417e14fad89e","unresolved":false,"context_lines":[{"line_number":214,"context_line":"                    break"},{"line_number":215,"context_line":"        else:"},{"line_number":216,"context_line":"            self.fail(\u0027Still found tombstone on %r after %s iterations\u0027 % ("},{"line_number":217,"context_line":"                hnodes[0], iterations))"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"        # tombstone is still on the *second* handoff"},{"line_number":220,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":10,"id":"df140735_9b144b50","line":217,"in_reply_to":"ff0f0b1f_3146b423","updated":"2017-05-31 07:21:21.000000000","message":"re: 1)\nThat\u0027s what i concerned but I\u0027ve not yet reach out if I am correct or not. Looking at the code, it will be 503 but I did never get the response with my custom (emulating that case) testing environment so I\u0027m not sure, I may be misunderstanding on that. Perhaps, pushing the tombstone to \"quorum nodes\" could be safe. \n\nre: 2)\nOIC, that sounds totally effective in the running cluster. With this change, the reconstuctor can remove the handoff tombstone even if a (or some) primary(ies) down. That makes me sense, it would be nice to have this change. Could you add the purpose of this patch in the commit message? The link for lp bug report could be enough but I like to have that in the commit message to avoid misunderstanding like me.","commit_id":"ad25ee6237199f7fcfc6a1d64b3b3cea997e3418"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"77dba0b1232b63fadb96f1a934ce9f0e3cacb82c","unresolved":false,"context_lines":[{"line_number":214,"context_line":"                    break"},{"line_number":215,"context_line":"        else:"},{"line_number":216,"context_line":"            self.fail(\u0027Still found tombstone on %r after %s iterations\u0027 % ("},{"line_number":217,"context_line":"                hnodes[0], iterations))"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"        # tombstone is still on the *second* handoff"},{"line_number":220,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":10,"id":"ff0f0b1f_22f4eb85","line":217,"in_reply_to":"ff0f0b1f_ba03c60f","updated":"2017-05-19 09:55:50.000000000","message":"Er - OK. My misunderstanding is from that I thought 2 servers shutdown (not 2 disks) but the test actually kill 2 drives. Therefore 4 disks are available for primaries and 2 handoffs will be used. And the timing at line 247, the handoff already pushed out to the nodes which was down at the DELETE timing.\n\nHmm... I don\u0027t feel this test asserts the change (i.e. random parity+1 node sampling to push the tombstone) straight forward for now.\n\nMy original concern is that swift requires more than quorum responses to handle the error response IIRC (sorry, not sure need to make sure) [1].\n\n\nExactly pushing the tombstone to parities + 1 nodes can cause to prevent successful GET for the older object but... does it always enable to return 404???\n\nI suppose pushing quorum nodes safe but absolutely that is more than current change.\n\nHow do you think of it?\n\n1: https://github.com/openstack/swift/blob/master/swift/proxy/controllers/base.py#L1726","commit_id":"ad25ee6237199f7fcfc6a1d64b3b3cea997e3418"}]}