)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"4d74beb5e8321abe41402683b917576768f48a96","unresolved":false,"context_lines":[{"line_number":42,"context_line":"If step \"c\" doesn\u0027t happen because the index does not yet exist - the"},{"line_number":43,"context_line":"update is clearly lost."},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"In our case we\u0027d skip marking a suffix as invalid when the hashes.pkl"},{"line_number":46,"context_line":"does not exist because we know \"the next time we rehash\" we\u0027ll have to"},{"line_number":47,"context_line":"os.listdir suffixes anyway.  But if another process is *currently*"},{"line_number":48,"context_line":"rehashing (and has already done it\u0027s os.listdir) instead we\u0027ve just"},{"line_number":49,"context_line":"dropped an invalidation on the floor."},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"Don\u0027t do that."},{"line_number":52,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"5a3905b3_13e806e2","line":49,"range":{"start_line":45,"start_character":0,"end_line":49,"end_character":37},"updated":"2017-01-20 08:30:03.000000000","message":"Yes, this patch addresses this race state. And the other state Alistair describing is between adding entry to invalidation file and clearing the entries (once the entries applied to hash.pkl).","commit_id":"e1f37ade419c44e8f66bad9d43795488fe9f58a5"}],"swift/obj/diskfile.py":[{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"5bf09f846a898dfc245886536de35ae5299ac800","unresolved":false,"context_lines":[{"line_number":249,"context_line":"            # no hashes at all -\u003e everything\u0027s invalid, so empty the file with"},{"line_number":250,"context_line":"            # the invalid suffixes in it, if it exists"},{"line_number":251,"context_line":"            try:"},{"line_number":252,"context_line":"                with open(invalidations_file, \u0027wb\u0027):"},{"line_number":253,"context_line":"                    pass"},{"line_number":254,"context_line":"            except OSError as e:"},{"line_number":255,"context_line":"                if e.errno !\u003d errno.ENOENT:"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5201f7_7c5adb84","line":252,"updated":"2017-01-11 11:02:03.000000000","message":"I know this is away from this patch\u0027s purpose but while reviewing the race condition and possibility, I\u0027m realizing scared with this invalidation entry removal. That is because the invalidation entries will be deleted *before* the new pickle file stored into disk that means it will be lost if the process was restarted at that time. Maybe?\n\nI think the missing entries may be rescued via for each replication_cycle (i.e. force listdir) but it seems like similar case with this on the aspect of missing invalidation entries.","commit_id":"6b2be1ed24d845dd7b6db1a8fb105926828fa2b1"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"5bf09f846a898dfc245886536de35ae5299ac800","unresolved":false,"context_lines":[{"line_number":320,"context_line":"            # it\u0027s ok to create here; we\u0027re under lock_path"},{"line_number":321,"context_line":"            mkdirs(partition_dir)"},{"line_number":322,"context_line":"            with open(invalidations_file, \u0027wb\u0027) as inv_fh:"},{"line_number":323,"context_line":"                inv_fh.write(suffix + \"\\n\")"},{"line_number":324,"context_line":""},{"line_number":325,"context_line":""},{"line_number":326,"context_line":"class AuditLocation(object):"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5201f7_eefd3c3c","line":323,"updated":"2017-01-11 11:02:03.000000000","message":"Though I know it\u0027s LBYL style but thinking of we\u0027re under lock_path,\n\n mkdirs(partition_dir)\n with open(invalidations_file, \u0027ab\u0027) as inv_fh:\n   inv_fh.write(suffix + \"\\n\")\n\nseems simple to address the case if the partition dir deleted by another process before lock_path?\n\nThat is because mkdirs calls os.path.isdir(path).\n\n\nOh...wait? wait? We\u0027re under `lock_path(partition_dir)` and `mkdirs(partition_dir)`, here??? That\u0027s too weird because the dir should exist and should be never changed due to the lock_path, right? Am i missing something? \n\n\nAnd while I was checking the assumption above, I found we don\u0027t have the test on the branch after IOError raised when opening invalidation_file.","commit_id":"6b2be1ed24d845dd7b6db1a8fb105926828fa2b1"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"0ba275189c69b6b3f21c8f1c3af9b89bc4d2eb49","unresolved":false,"context_lines":[{"line_number":312,"context_line":"    hashes_file \u003d join(partition_dir, HASH_FILE)"},{"line_number":313,"context_line":"    if not os.path.exists(hashes_file):"},{"line_number":314,"context_line":"        return"},{"line_number":315,"context_line":""},{"line_number":316,"context_line":"    invalidations_file \u003d join(partition_dir, HASH_INVALIDATIONS_FILE)"},{"line_number":317,"context_line":"    with lock_path(partition_dir):"},{"line_number":318,"context_line":"        with open(invalidations_file, \u0027ab\u0027) as inv_fh:"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a3c09a3_89d3df37","side":"PARENT","line":315,"updated":"2017-01-16 05:55:34.000000000","message":"Yeah, i think this is small enough to fix.","commit_id":"e772cf95c65b98bb17b094585ff3ff2040c148c9"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"3a602f80865598dd94fa46f5ba32885214554c51","unresolved":false,"context_lines":[{"line_number":247,"context_line":"    if not os.path.exists(hashes_file):"},{"line_number":248,"context_line":"        if os.path.exists(invalidations_file):"},{"line_number":249,"context_line":"            # no hashes at all -\u003e everything\u0027s invalid, so empty the file with"},{"line_number":250,"context_line":"            # the invalid suffixes in it, if it exists"},{"line_number":251,"context_line":"            try:"},{"line_number":252,"context_line":"                with open(invalidations_file, \u0027wb\u0027):"},{"line_number":253,"context_line":"                    pass"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a3905b3_73689dbb","line":250,"updated":"2017-01-19 16:21:04.000000000","message":"not sure this is safe :/ (or ever was, so this is probably out of scope for this patch):\n\nin this patch we make sure invalidations (the journal) always gets written while hashes.pkl (index) modifications are locked out. But here it is truncated without locking w.r.t. the index.\n\n   replicator                      DELETE request     REPLICATE request\n  (a) hashes.pkl does not exist\n  (b) truncate hashes.invalid\n  (c) do listdir, hash suffixes\n                                  (d) invalidate\n                                                  (e) hashes.pkl does not exist\n                                                  (f) truncate hashes.invalid :/              \n                                                  (g) process fails\n  (h) write hashes.pkl","commit_id":"a6f7a7a8c679b08c6d22e7253b0d28b80849a140"},{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"729ad1250a1e7c7df568ac423bc66526aacbd1f9","unresolved":false,"context_lines":[{"line_number":247,"context_line":"    if not os.path.exists(hashes_file):"},{"line_number":248,"context_line":"        if os.path.exists(invalidations_file):"},{"line_number":249,"context_line":"            # no hashes at all -\u003e everything\u0027s invalid, so empty the file with"},{"line_number":250,"context_line":"            # the invalid suffixes in it, if it exists"},{"line_number":251,"context_line":"            try:"},{"line_number":252,"context_line":"                with open(invalidations_file, \u0027wb\u0027):"},{"line_number":253,"context_line":"                    pass"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a3905b3_1cd7fced","line":250,"in_reply_to":"5a3905b3_73689dbb","updated":"2017-01-21 20:02:01.000000000","message":"Thank you for the visual sequence.","commit_id":"a6f7a7a8c679b08c6d22e7253b0d28b80849a140"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"3a602f80865598dd94fa46f5ba32885214554c51","unresolved":false,"context_lines":[{"line_number":250,"context_line":"            # the invalid suffixes in it, if it exists"},{"line_number":251,"context_line":"            try:"},{"line_number":252,"context_line":"                with open(invalidations_file, \u0027wb\u0027):"},{"line_number":253,"context_line":"                    pass"},{"line_number":254,"context_line":"            except OSError as e:"},{"line_number":255,"context_line":"                if e.errno !\u003d errno.ENOENT:"},{"line_number":256,"context_line":"                    raise"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a3905b3_dafcf840","line":253,"updated":"2017-01-19 16:21:04.000000000","message":"but I\u0027m also no longer sure, with this patch, why we would now choose to ignore all the invalidations that we have been collecting - do we not trust the hashes.invalid journal to have a record of all suffixes*? why return None and force a listdir, why not just proceed to consolidate and generate hashes.pkl in this method.\n\n(*and even if it doesn\u0027t, we\u0027ll force a listdir every 10th cycle anyway)\n\nBefore it made sense because we weren\u0027t journaling invalidations until hashes.pkl existed, but now we are.\n\nOne reason I can think to justify this kind of behaviour is so that we still know that deleting hashes.pkl and running the replicator will force a complete rehash.","commit_id":"a6f7a7a8c679b08c6d22e7253b0d28b80849a140"},{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"729ad1250a1e7c7df568ac423bc66526aacbd1f9","unresolved":false,"context_lines":[{"line_number":250,"context_line":"            # the invalid suffixes in it, if it exists"},{"line_number":251,"context_line":"            try:"},{"line_number":252,"context_line":"                with open(invalidations_file, \u0027wb\u0027):"},{"line_number":253,"context_line":"                    pass"},{"line_number":254,"context_line":"            except OSError as e:"},{"line_number":255,"context_line":"                if e.errno !\u003d errno.ENOENT:"},{"line_number":256,"context_line":"                    raise"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a3905b3_3c14b89e","line":253,"in_reply_to":"5a3905b3_d3203e9e","updated":"2017-01-21 20:02:01.000000000","message":"I agree with Kota on making a separate patch - unless there is some possibility that a lost invalidation here would not later be recovered.","commit_id":"a6f7a7a8c679b08c6d22e7253b0d28b80849a140"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"34ce82382ce8fcec54bcba57aadf1d0cbc74e213","unresolved":false,"context_lines":[{"line_number":250,"context_line":"            # the invalid suffixes in it, if it exists"},{"line_number":251,"context_line":"            try:"},{"line_number":252,"context_line":"                with open(invalidations_file, \u0027wb\u0027):"},{"line_number":253,"context_line":"                    pass"},{"line_number":254,"context_line":"            except OSError as e:"},{"line_number":255,"context_line":"                if e.errno !\u003d errno.ENOENT:"},{"line_number":256,"context_line":"                    raise"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a3905b3_d3203e9e","line":253,"in_reply_to":"5a3905b3_dafcf840","updated":"2017-01-20 08:12:32.000000000","message":"I think you have same vision to me, see my comment at *patch set 1*[1] so that I\u0027m feeling you\u0027re correct that it seems not safe to me too.\n\nOTOH, this patch exactly fix *a race state* on the hash invalidation so I\u0027m thinking we can close the current bug(\t\nbug/1651530). And then, we are going to fix this one as another bug.\n\n1: https://review.openstack.org/#/c/418690/1/swift/obj/diskfile.py@252","commit_id":"a6f7a7a8c679b08c6d22e7253b0d28b80849a140"},{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"729ad1250a1e7c7df568ac423bc66526aacbd1f9","unresolved":false,"context_lines":[{"line_number":250,"context_line":"            # the invalid suffixes in it, if it exists"},{"line_number":251,"context_line":"            try:"},{"line_number":252,"context_line":"                with open(invalidations_file, \u0027wb\u0027):"},{"line_number":253,"context_line":"                    pass"},{"line_number":254,"context_line":"            except OSError as e:"},{"line_number":255,"context_line":"                if e.errno !\u003d errno.ENOENT:"},{"line_number":256,"context_line":"                    raise"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a3905b3_9f23dea6","line":253,"in_reply_to":"5a3905b3_dafcf840","updated":"2017-01-21 20:02:01.000000000","message":"Sounds like a good follow-up since listdir comes with a high cost. To address the possible reason for preserving current truncate, am I correct in thinking that one could also remove hashes.invalid in order to force a complete rehash? (so, remove hashes.pkl, hashes.invalid, and run replicate)","commit_id":"a6f7a7a8c679b08c6d22e7253b0d28b80849a140"}],"test/unit/obj/test_auditor.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"3a602f80865598dd94fa46f5ba32885214554c51","unresolved":false,"context_lines":[{"line_number":841,"context_line":"        self.assertEqual([\u0027%s.ts\u0027 % ts_tomb.internal],"},{"line_number":842,"context_line":"                         os.listdir(self.disk_file._datadir))"},{"line_number":843,"context_line":"        self.assertTrue(os.path.exists(os.path.join(part_dir, HASH_FILE)))"},{"line_number":844,"context_line":"        self.assertTrue(os.path.exists("},{"line_number":845,"context_line":"            os.path.join(part_dir, HASH_INVALIDATIONS_FILE)))"},{"line_number":846,"context_line":"        # Run auditor"},{"line_number":847,"context_line":"        self.auditor.run_audit(mode\u003d\u0027once\u0027, zero_byte_fps\u003dzero_byte_fps)"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a3905b3_91b0a0f6","line":844,"updated":"2017-01-19 16:21:04.000000000","message":"the check on this initial state was a basis for some of the later test assertions - i.e. no invalids file -\u003e no invalidate_hashes call. Now it is always true, we need to beef up the test assertions.","commit_id":"a6f7a7a8c679b08c6d22e7253b0d28b80849a140"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9cf94775f34878ebfe8353e4fc075bea03170e0b","unresolved":false,"context_lines":[{"line_number":841,"context_line":"        self.assertEqual([\u0027%s.ts\u0027 % ts_tomb.internal],"},{"line_number":842,"context_line":"                         os.listdir(self.disk_file._datadir))"},{"line_number":843,"context_line":"        self.assertTrue(os.path.exists(os.path.join(part_dir, HASH_FILE)))"},{"line_number":844,"context_line":"        self.assertTrue(os.path.exists("},{"line_number":845,"context_line":"            os.path.join(part_dir, HASH_INVALIDATIONS_FILE)))"},{"line_number":846,"context_line":"        # Run auditor"},{"line_number":847,"context_line":"        self.auditor.run_audit(mode\u003d\u0027once\u0027, zero_byte_fps\u003dzero_byte_fps)"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a3905b3_9d6d0a35","line":844,"in_reply_to":"5a3905b3_91b0a0f6","updated":"2017-01-19 16:25:31.000000000","message":"Done","commit_id":"a6f7a7a8c679b08c6d22e7253b0d28b80849a140"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"3a602f80865598dd94fa46f5ba32885214554c51","unresolved":false,"context_lines":[{"line_number":856,"context_line":"        part_dir, suffix \u003d self._audit_tombstone(self.conf, ts_tomb)"},{"line_number":857,"context_line":"        self.assertTrue(os.path.exists(os.path.join(part_dir, HASH_FILE)))"},{"line_number":858,"context_line":"        self.assertTrue(os.path.exists("},{"line_number":859,"context_line":"            os.path.join(part_dir, HASH_INVALIDATIONS_FILE)))"},{"line_number":860,"context_line":""},{"line_number":861,"context_line":"    def test_reclaimable_tombstone(self):"},{"line_number":862,"context_line":"        # audit with a reclaimable tombstone"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a3905b3_e9173a52","line":859,"updated":"2017-01-19 16:21:04.000000000","message":"So here we now need to assert that no invalidation has been made (before that was implied by there being no invalidations file) by checking the hashes.invalid is empty.\n\nOtherwise this test passes if I make the tombstone reclaimable at line 855","commit_id":"a6f7a7a8c679b08c6d22e7253b0d28b80849a140"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9cf94775f34878ebfe8353e4fc075bea03170e0b","unresolved":false,"context_lines":[{"line_number":856,"context_line":"        part_dir, suffix \u003d self._audit_tombstone(self.conf, ts_tomb)"},{"line_number":857,"context_line":"        self.assertTrue(os.path.exists(os.path.join(part_dir, HASH_FILE)))"},{"line_number":858,"context_line":"        self.assertTrue(os.path.exists("},{"line_number":859,"context_line":"            os.path.join(part_dir, HASH_INVALIDATIONS_FILE)))"},{"line_number":860,"context_line":""},{"line_number":861,"context_line":"    def test_reclaimable_tombstone(self):"},{"line_number":862,"context_line":"        # audit with a reclaimable tombstone"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a3905b3_bd6ac618","line":859,"in_reply_to":"5a3905b3_e9173a52","updated":"2017-01-19 16:25:31.000000000","message":"Done","commit_id":"a6f7a7a8c679b08c6d22e7253b0d28b80849a140"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"3a602f80865598dd94fa46f5ba32885214554c51","unresolved":false,"context_lines":[{"line_number":877,"context_line":"        part_dir, suffix \u003d self._audit_tombstone(conf, ts_tomb)"},{"line_number":878,"context_line":"        self.assertTrue(os.path.exists(os.path.join(part_dir, HASH_FILE)))"},{"line_number":879,"context_line":"        self.assertTrue(os.path.exists("},{"line_number":880,"context_line":"            os.path.join(part_dir, HASH_INVALIDATIONS_FILE)))"},{"line_number":881,"context_line":""},{"line_number":882,"context_line":"    def test_reclaimable_tombstone_with_custom_reclaim_age(self):"},{"line_number":883,"context_line":"        # audit with a tombstone older than custom reclaim age"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a3905b3_f1f1e4ce","line":880,"updated":"2017-01-19 16:21:04.000000000","message":"same as line 859","commit_id":"a6f7a7a8c679b08c6d22e7253b0d28b80849a140"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9cf94775f34878ebfe8353e4fc075bea03170e0b","unresolved":false,"context_lines":[{"line_number":877,"context_line":"        part_dir, suffix \u003d self._audit_tombstone(conf, ts_tomb)"},{"line_number":878,"context_line":"        self.assertTrue(os.path.exists(os.path.join(part_dir, HASH_FILE)))"},{"line_number":879,"context_line":"        self.assertTrue(os.path.exists("},{"line_number":880,"context_line":"            os.path.join(part_dir, HASH_INVALIDATIONS_FILE)))"},{"line_number":881,"context_line":""},{"line_number":882,"context_line":"    def test_reclaimable_tombstone_with_custom_reclaim_age(self):"},{"line_number":883,"context_line":"        # audit with a tombstone older than custom reclaim age"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a3905b3_dd779200","line":880,"in_reply_to":"5a3905b3_f1f1e4ce","updated":"2017-01-19 16:25:31.000000000","message":"Done","commit_id":"a6f7a7a8c679b08c6d22e7253b0d28b80849a140"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"3a602f80865598dd94fa46f5ba32885214554c51","unresolved":false,"context_lines":[{"line_number":900,"context_line":"            self.conf, ts_tomb, zero_byte_fps\u003d50)"},{"line_number":901,"context_line":"        self.assertTrue(os.path.exists(os.path.join(part_dir, HASH_FILE)))"},{"line_number":902,"context_line":"        self.assertTrue(os.path.exists("},{"line_number":903,"context_line":"            os.path.join(part_dir, HASH_INVALIDATIONS_FILE)))"},{"line_number":904,"context_line":""},{"line_number":905,"context_line":"    def _test_expired_object_is_ignored(self, zero_byte_fps):"},{"line_number":906,"context_line":"        # verify that an expired object does not get mistaken for a tombstone"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a3905b3_b1d11cfe","line":903,"updated":"2017-01-19 16:21:04.000000000","message":"ditto, this test now passes with or without zero_byte_fps","commit_id":"a6f7a7a8c679b08c6d22e7253b0d28b80849a140"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9cf94775f34878ebfe8353e4fc075bea03170e0b","unresolved":false,"context_lines":[{"line_number":900,"context_line":"            self.conf, ts_tomb, zero_byte_fps\u003d50)"},{"line_number":901,"context_line":"        self.assertTrue(os.path.exists(os.path.join(part_dir, HASH_FILE)))"},{"line_number":902,"context_line":"        self.assertTrue(os.path.exists("},{"line_number":903,"context_line":"            os.path.join(part_dir, HASH_INVALIDATIONS_FILE)))"},{"line_number":904,"context_line":""},{"line_number":905,"context_line":"    def _test_expired_object_is_ignored(self, zero_byte_fps):"},{"line_number":906,"context_line":"        # verify that an expired object does not get mistaken for a tombstone"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a3905b3_7d5f7e72","line":903,"in_reply_to":"5a3905b3_b1d11cfe","updated":"2017-01-19 16:25:31.000000000","message":"Done","commit_id":"a6f7a7a8c679b08c6d22e7253b0d28b80849a140"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"3a602f80865598dd94fa46f5ba32885214554c51","unresolved":false,"context_lines":[{"line_number":916,"context_line":"            audit.run_audit(mode\u003d\u0027once\u0027, zero_byte_fps\u003dzero_byte_fps)"},{"line_number":917,"context_line":"        self.assertTrue(os.path.exists(self.disk_file._datadir))"},{"line_number":918,"context_line":"        part_dir \u003d dirname(dirname(self.disk_file._datadir))"},{"line_number":919,"context_line":"        self.assertTrue(os.path.exists("},{"line_number":920,"context_line":"            os.path.join(part_dir, HASH_INVALIDATIONS_FILE)))"},{"line_number":921,"context_line":"        self.assertEqual(files, os.listdir(self.disk_file._datadir))"},{"line_number":922,"context_line":"        self.assertFalse(audit.logger.get_lines_for_level(\u0027error\u0027))"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a3905b3_d7adb832","line":919,"updated":"2017-01-19 16:21:04.000000000","message":"I think this test was already weak (mea culpa) but this assertion passes before the auditor is run (on master too) :/\n\non master, because there is no hashes.pkl, the invalidation file won\u0027t exist even if the auditor did call invalidate_hash for the expired object. With this patch, the invalidations file exists once the object is created, and still exists after the auditor run, so we\u0027re not really checking anything.","commit_id":"a6f7a7a8c679b08c6d22e7253b0d28b80849a140"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9cf94775f34878ebfe8353e4fc075bea03170e0b","unresolved":false,"context_lines":[{"line_number":916,"context_line":"            audit.run_audit(mode\u003d\u0027once\u0027, zero_byte_fps\u003dzero_byte_fps)"},{"line_number":917,"context_line":"        self.assertTrue(os.path.exists(self.disk_file._datadir))"},{"line_number":918,"context_line":"        part_dir \u003d dirname(dirname(self.disk_file._datadir))"},{"line_number":919,"context_line":"        self.assertTrue(os.path.exists("},{"line_number":920,"context_line":"            os.path.join(part_dir, HASH_INVALIDATIONS_FILE)))"},{"line_number":921,"context_line":"        self.assertEqual(files, os.listdir(self.disk_file._datadir))"},{"line_number":922,"context_line":"        self.assertFalse(audit.logger.get_lines_for_level(\u0027error\u0027))"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a3905b3_9d542a55","line":919,"in_reply_to":"5a3905b3_d7adb832","updated":"2017-01-19 16:25:31.000000000","message":"Done","commit_id":"a6f7a7a8c679b08c6d22e7253b0d28b80849a140"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"4d74beb5e8321abe41402683b917576768f48a96","unresolved":false,"context_lines":[{"line_number":925,"context_line":"        hash_invalid \u003d os.path.join(part_dir, HASH_INVALIDATIONS_FILE)"},{"line_number":926,"context_line":"        with open(hash_invalid, \u0027rb\u0027) as fp:"},{"line_number":927,"context_line":"            self.assertEqual(basename(dirname(self.disk_file._datadir)),"},{"line_number":928,"context_line":"                             fp.read().strip(\u0027\\n\u0027))  # sanity check"},{"line_number":929,"context_line":"        # this get_hashes call will truncate the invalid hashes entry"},{"line_number":930,"context_line":"        self.disk_file.manager.get_hashes("},{"line_number":931,"context_line":"            self.devices + \u0027/sda\u0027, \u00270\u0027, [], self.disk_file.policy)"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a3905b3_1330079e","line":928,"updated":"2017-01-20 08:30:03.000000000","message":"I\u0027d like to add one more state here like:\nhttps://gist.github.com/bloodeagle40234/a6566c3c1177c976cdd1d9ad67b65fcc\n\ni.e. run auditor here before calling get_hashes and it also touches nothing. Is it good?","commit_id":"e1f37ade419c44e8f66bad9d43795488fe9f58a5"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"1ed0b83243d0e3d10da353c0a6b834c398caf464","unresolved":false,"context_lines":[{"line_number":925,"context_line":"        hash_invalid \u003d os.path.join(part_dir, HASH_INVALIDATIONS_FILE)"},{"line_number":926,"context_line":"        with open(hash_invalid, \u0027rb\u0027) as fp:"},{"line_number":927,"context_line":"            self.assertEqual(basename(dirname(self.disk_file._datadir)),"},{"line_number":928,"context_line":"                             fp.read().strip(\u0027\\n\u0027))  # sanity check"},{"line_number":929,"context_line":"        # this get_hashes call will truncate the invalid hashes entry"},{"line_number":930,"context_line":"        self.disk_file.manager.get_hashes("},{"line_number":931,"context_line":"            self.devices + \u0027/sda\u0027, \u00270\u0027, [], self.disk_file.policy)"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a3905b3_fca2674a","line":928,"in_reply_to":"5a3905b3_1330079e","updated":"2017-01-20 09:42:41.000000000","message":"I don\u0027t see anything wrong with your gist, but it seems to be repeating same assertion i.e. auditor does not append any new content to the hashes.invalid.\n\nAssuming that invalidate_hash always appends to hashes.invalid, then we will detect regression with current checks. If invalidate_hash ever changed to append only if suffix is not already in hashes.invalid, then we still catch any regression when starting state is an empty hashes.invalid.\n\nah, ok so the starting state of hashes.invalid is different - here hashes.invalid has been truncated, in your gist it stall has the suffix? Is that your goal, to check both starting states?\n\nI don\u0027t mind either way so if you like to add the extra state/assertion please go ahead and push change, I will still +2.","commit_id":"e1f37ade419c44e8f66bad9d43795488fe9f58a5"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"e1f154d3f256f5de2d74cb300df6a57848a9a60a","unresolved":false,"context_lines":[{"line_number":925,"context_line":"        hash_invalid \u003d os.path.join(part_dir, HASH_INVALIDATIONS_FILE)"},{"line_number":926,"context_line":"        with open(hash_invalid, \u0027rb\u0027) as fp:"},{"line_number":927,"context_line":"            self.assertEqual(basename(dirname(self.disk_file._datadir)),"},{"line_number":928,"context_line":"                             fp.read().strip(\u0027\\n\u0027))  # sanity check"},{"line_number":929,"context_line":"        # this get_hashes call will truncate the invalid hashes entry"},{"line_number":930,"context_line":"        self.disk_file.manager.get_hashes("},{"line_number":931,"context_line":"            self.devices + \u0027/sda\u0027, \u00270\u0027, [], self.disk_file.policy)"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a3905b3_23cbda73","line":928,"in_reply_to":"5a3905b3_fca2674a","updated":"2017-01-20 15:25:25.000000000","message":"That\u0027s correct for my idea that the starting state is different. I pushed the change as patch set 7.","commit_id":"e1f37ade419c44e8f66bad9d43795488fe9f58a5"}],"test/unit/obj/test_diskfile.py":[{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"5bf09f846a898dfc245886536de35ae5299ac800","unresolved":false,"context_lines":[{"line_number":6091,"context_line":"                part_path, diskfile.HASH_INVALIDATIONS_FILE)"},{"line_number":6092,"context_line":"            with open(hashes_file, \u0027rb\u0027) as f:"},{"line_number":6093,"context_line":"                self.assertEqual(original_hashes, pickle.load(f))"},{"line_number":6094,"context_line":"            self.assertFalse(os.path.exists(invalidations_file))"},{"line_number":6095,"context_line":""},{"line_number":6096,"context_line":"            # invalidate the hash"},{"line_number":6097,"context_line":"            with mock.patch(\u0027swift.obj.diskfile.lock_path\u0027) as mock_lock:"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5201f7_bff6fcf6","side":"PARENT","line":6094,"updated":"2017-01-11 11:02:03.000000000","message":"OK, always we will create invalidations_file.","commit_id":"8a69410e0e1fbc9dd2831ad7f26f36caf214ab89"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"5bf09f846a898dfc245886536de35ae5299ac800","unresolved":false,"context_lines":[{"line_number":6051,"context_line":"            # ... and hashing suffixes finds (and hashes) the new suffix"},{"line_number":6052,"context_line":"            hashes \u003d df_mgr.get_hashes(\u0027sda1\u0027, \u00270\u0027, [], policy)"},{"line_number":6053,"context_line":"            self.assertTrue(os.path.exists(hashes_file))"},{"line_number":6054,"context_line":"            self.assertIn(os.path.basename(suffix_dir), hashes)"},{"line_number":6055,"context_line":""},{"line_number":6056,"context_line":"    def test_invalidate_hash_empty_file_exists(self):"},{"line_number":6057,"context_line":"        for policy in self.iter_policies():"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5201f7_3fa92c7c","line":6054,"updated":"2017-01-11 11:02:03.000000000","message":"minor nits)sanity for the existence of inv_file seems worth.","commit_id":"6b2be1ed24d845dd7b6db1a8fb105926828fa2b1"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"5bf09f846a898dfc245886536de35ae5299ac800","unresolved":false,"context_lines":[{"line_number":6135,"context_line":"            # gets appended, but we don\u0027t restart hashing suffixes (the"},{"line_number":6136,"context_line":"            # invalidation get\u0027s squashed in and the suffix gets rehashed on"},{"line_number":6137,"context_line":"            # the next REPLICATE call)"},{"line_number":6138,"context_line":"            df_mgr.get_hashes(\u0027sda1\u0027, \u00270\u0027, [], policy)"},{"line_number":6139,"context_line":"            orig_listdir \u003d os.listdir"},{"line_number":6140,"context_line":"            df \u003d df_mgr.get_diskfile(\u0027sda1\u0027, \u00270\u0027, \u0027a\u0027, \u0027c\u0027, \u0027o\u0027,"},{"line_number":6141,"context_line":"                                     policy\u003dpolicy)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5201f7_f96c1d9e","line":6138,"updated":"2017-01-11 11:02:03.000000000","message":"minor nits) worth if the assertion to check the pkl file exists.","commit_id":"6b2be1ed24d845dd7b6db1a8fb105926828fa2b1"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"5bf09f846a898dfc245886536de35ae5299ac800","unresolved":false,"context_lines":[{"line_number":6166,"context_line":"                hashes \u003d df_mgr.get_hashes(\u0027sda1\u0027, \u00270\u0027, [], policy)"},{"line_number":6167,"context_line":""},{"line_number":6168,"context_line":"            self.assertIn(suffix, hashes)"},{"line_number":6169,"context_line":"            self.assertIn(suffix2, hashes)"},{"line_number":6170,"context_line":""},{"line_number":6171,"context_line":"    def test_invalidate_hash_consolidation(self):"},{"line_number":6172,"context_line":"        def assert_consolidation(suffixes):"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5201f7_3963051c","line":6169,"updated":"2017-01-11 11:02:03.000000000","message":"ok, existing pkl file can trigger all invalidation files due to different mtime.","commit_id":"6b2be1ed24d845dd7b6db1a8fb105926828fa2b1"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"0ba275189c69b6b3f21c8f1c3af9b89bc4d2eb49","unresolved":false,"context_lines":[{"line_number":6051,"context_line":"            # ... and hashing suffixes finds (and hashes) the new suffix"},{"line_number":6052,"context_line":"            hashes \u003d df_mgr.get_hashes(\u0027sda1\u0027, \u00270\u0027, [], policy)"},{"line_number":6053,"context_line":"            self.assertTrue(os.path.exists(hashes_file))"},{"line_number":6054,"context_line":"            self.assertIn(os.path.basename(suffix_dir), hashes)"},{"line_number":6055,"context_line":""},{"line_number":6056,"context_line":"    def test_invalidate_hash_empty_file_exists(self):"},{"line_number":6057,"context_line":"        for policy in self.iter_policies():"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a3c09a3_29c66b49","line":6054,"updated":"2017-01-16 05:55:34.000000000","message":"I like to add more assertions like\n\n # ... because of the get_hashes call, the empty invalidtion file \n # exists there\n self.assertTrue(os.path.exists(inv_file))\n with open(inv_file) as f:\n   self.assertEqual(\u0027\u0027, f.read())\n\nBut it does never block to get merged the fix for critical bug, IMO.","commit_id":"a6f7a7a8c679b08c6d22e7253b0d28b80849a140"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"34ce82382ce8fcec54bcba57aadf1d0cbc74e213","unresolved":false,"context_lines":[{"line_number":6051,"context_line":"            # ... and hashing suffixes finds (and hashes) the new suffix"},{"line_number":6052,"context_line":"            hashes \u003d df_mgr.get_hashes(\u0027sda1\u0027, \u00270\u0027, [], policy)"},{"line_number":6053,"context_line":"            self.assertTrue(os.path.exists(hashes_file))"},{"line_number":6054,"context_line":"            self.assertIn(os.path.basename(suffix_dir), hashes)"},{"line_number":6055,"context_line":""},{"line_number":6056,"context_line":"    def test_invalidate_hash_empty_file_exists(self):"},{"line_number":6057,"context_line":"        for policy in self.iter_policies():"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a3905b3_93ca376d","line":6054,"in_reply_to":"5a3905b3_5d83a2cc","updated":"2017-01-20 08:12:32.000000000","message":"Oh, you has referred my comments, thanks.\n\nI thought you didn\u0027t notice but my wrong :/\n\nAnd thank you for expanding them for all assertions you add in the latest patch set ;-)","commit_id":"a6f7a7a8c679b08c6d22e7253b0d28b80849a140"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9cf94775f34878ebfe8353e4fc075bea03170e0b","unresolved":false,"context_lines":[{"line_number":6051,"context_line":"            # ... and hashing suffixes finds (and hashes) the new suffix"},{"line_number":6052,"context_line":"            hashes \u003d df_mgr.get_hashes(\u0027sda1\u0027, \u00270\u0027, [], policy)"},{"line_number":6053,"context_line":"            self.assertTrue(os.path.exists(hashes_file))"},{"line_number":6054,"context_line":"            self.assertIn(os.path.basename(suffix_dir), hashes)"},{"line_number":6055,"context_line":""},{"line_number":6056,"context_line":"    def test_invalidate_hash_empty_file_exists(self):"},{"line_number":6057,"context_line":"        for policy in self.iter_policies():"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a3905b3_5d83a2cc","line":6054,"in_reply_to":"7a3c09a3_29c66b49","updated":"2017-01-19 16:25:31.000000000","message":"Done","commit_id":"a6f7a7a8c679b08c6d22e7253b0d28b80849a140"},{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"729ad1250a1e7c7df568ac423bc66526aacbd1f9","unresolved":false,"context_lines":[{"line_number":6067,"context_line":"            hashes \u003d df_mgr.get_hashes(\u0027sda1\u0027, \u00270\u0027, [], policy)"},{"line_number":6068,"context_line":"            self.assertIn(suffix, hashes)  # sanity"},{"line_number":6069,"context_line":""},{"line_number":6070,"context_line":"    def test_invalidates_hashes_of_new_partition(self):"},{"line_number":6071,"context_line":"        # a suffix can be changed or created by second process when new pkl"},{"line_number":6072,"context_line":"        # is calculated - that suffix must be correct on next get_hashes call"},{"line_number":6073,"context_line":"        for policy in self.iter_policies():"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a3905b3_3eef9ee9","line":6070,"updated":"2017-01-21 20:02:01.000000000","message":"nit: keep name consistent with other tests by using \u0027test_invalidate_hash_....\"","commit_id":"a6f7a7a8c679b08c6d22e7253b0d28b80849a140"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"0ba275189c69b6b3f21c8f1c3af9b89bc4d2eb49","unresolved":false,"context_lines":[{"line_number":6081,"context_line":"                                               policy\u003dpolicy)"},{"line_number":6082,"context_line":"            suffix2 \u003d os.path.basename(os.path.dirname(df2._datadir))"},{"line_number":6083,"context_line":"            non_local \u003d {\u0027df2touched\u0027: False}"},{"line_number":6084,"context_line":"            df.delete(self.ts())"},{"line_number":6085,"context_line":""},{"line_number":6086,"context_line":"            def mock_listdir(*args, **kwargs):"},{"line_number":6087,"context_line":"                # simulating an invalidation occuring in another process while"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a3c09a3_29858bac","line":6084,"updated":"2017-01-16 05:55:34.000000000","message":"Yes, at first, only df has hash invalidation entry and the suffix dir.","commit_id":"a6f7a7a8c679b08c6d22e7253b0d28b80849a140"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"0ba275189c69b6b3f21c8f1c3af9b89bc4d2eb49","unresolved":false,"context_lines":[{"line_number":6100,"context_line":""},{"line_number":6101,"context_line":"            # second suffix added after directory listing, it\u0027s added later"},{"line_number":6102,"context_line":"            self.assertIn(suffix, hashes)"},{"line_number":6103,"context_line":"            self.assertNotIn(suffix2, hashes)"},{"line_number":6104,"context_line":"            # updates pkl file"},{"line_number":6105,"context_line":"            hashes \u003d df_mgr.get_hashes(\u0027sda1\u0027, \u00270\u0027, [], policy)"},{"line_number":6106,"context_line":"            self.assertIn(suffix, hashes)"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a3c09a3_09b1af8f","line":6103,"updated":"2017-01-16 05:55:34.000000000","message":"sanity) self.assertIs(True, non_local[\u0027df2touched\u0027])","commit_id":"a6f7a7a8c679b08c6d22e7253b0d28b80849a140"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"0ba275189c69b6b3f21c8f1c3af9b89bc4d2eb49","unresolved":false,"context_lines":[{"line_number":6107,"context_line":"            self.assertIn(suffix2, hashes)"},{"line_number":6108,"context_line":""},{"line_number":6109,"context_line":"    @mock.patch(\u0027swift.obj.diskfile.getmtime\u0027)"},{"line_number":6110,"context_line":"    @mock.patch(\u0027swift.obj.diskfile.write_pickle\u0027)"},{"line_number":6111,"context_line":"    def test_contains_hashes_of_existing_partition(self, mock_write_pickle,"},{"line_number":6112,"context_line":"                                                   mock_getmtime):"},{"line_number":6113,"context_line":"        # get_hashes must repeat path listing and return all hashes when"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a3c09a3_9b1c06c3","line":6110,"updated":"2017-01-16 05:55:34.000000000","message":"I\u0027m realizing we don\u0027t have to mock those 2 functions with the lines, L6118-L6127.\n\nThat is because the mock_getmtime_def looks to be prepared to raise OSError if no mtime in non_local, that means it called *before* write_pickle_def. That will happen [1] but, actually we should not raise the error because we already prepared the hash.pkl file from get_hashes call at L6139.\n\nTo think the get_hashes process w/o mocking, the get_hashes L6168 will work as follows:\n\n#\u003csequence\u003e operation (hash.pkl mtime)\n1. get_mtime from existing hash.pkl (T0)\n2. get invalidation file entries for df and rewrite hash.pkl (T1)\n3. call listdir for df, and it causes df2 and create a new invalidation file entry (T1)\n4. call _get_hashes recursively because T1 !\u003d T0\n5. get mtime from existing hash.pkl (t1)\n6. get invalidation file entries for df2 and rewrite hash.pk (T2)\n7. \u003c...snip, one more _get_hashes recursive\u003e\n\nI think the process is well either w/o mock and w/ mock and actually it works with [2]. If it\u0027s true I\u0027d like to test w/o mocking because it will fit with actual behavior rather than mocking.\n\n\n1: https://github.com/openstack/swift/blob/master/swift/obj/diskfile.py#L1048\n2: https://gist.github.com/bloodeagle40234/ea8679c6e07eed9227e275615455448d","commit_id":"a6f7a7a8c679b08c6d22e7253b0d28b80849a140"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"3bae3cf60a1165137e1fab8381850e74797607e0","unresolved":false,"context_lines":[{"line_number":6107,"context_line":"            self.assertIn(suffix2, hashes)"},{"line_number":6108,"context_line":""},{"line_number":6109,"context_line":"    @mock.patch(\u0027swift.obj.diskfile.getmtime\u0027)"},{"line_number":6110,"context_line":"    @mock.patch(\u0027swift.obj.diskfile.write_pickle\u0027)"},{"line_number":6111,"context_line":"    def test_contains_hashes_of_existing_partition(self, mock_write_pickle,"},{"line_number":6112,"context_line":"                                                   mock_getmtime):"},{"line_number":6113,"context_line":"        # get_hashes must repeat path listing and return all hashes when"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a3c09a3_ef242ec6","line":6110,"in_reply_to":"7a3c09a3_9b1c06c3","updated":"2017-01-17 02:40:10.000000000","message":"From Clay\u0027s comment, to make this test reliable, we should keep the mocks here. That is because Clay can hit a failure with my suggestion like [1] and it could happen either with a machine fast enough to return same mtime with different pickle file or, maybe, with file system worse precision, FWIW. I cannot reproduce though (maybe I used slower machine with better precision file system enough not to reproduce).\n\nAnd then, in the future work, we will avoid the case which could return same mtime from different file around [2], at that time, we will remove those mock because they are unused since that, so this could be ok until the patch landed.\n\n\n1: https://gist.github.com/clayg/81be81ef339c7cc276b34c04be6c218c\n\n2: https://review.openstack.org/#/c/419787","commit_id":"a6f7a7a8c679b08c6d22e7253b0d28b80849a140"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"34ce82382ce8fcec54bcba57aadf1d0cbc74e213","unresolved":false,"context_lines":[{"line_number":6107,"context_line":"            self.assertIn(suffix2, hashes)"},{"line_number":6108,"context_line":""},{"line_number":6109,"context_line":"    @mock.patch(\u0027swift.obj.diskfile.getmtime\u0027)"},{"line_number":6110,"context_line":"    @mock.patch(\u0027swift.obj.diskfile.write_pickle\u0027)"},{"line_number":6111,"context_line":"    def test_contains_hashes_of_existing_partition(self, mock_write_pickle,"},{"line_number":6112,"context_line":"                                                   mock_getmtime):"},{"line_number":6113,"context_line":"        # get_hashes must repeat path listing and return all hashes when"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a3905b3_93915755","line":6110,"in_reply_to":"7a3c09a3_ef242ec6","updated":"2017-01-20 08:12:32.000000000","message":"Done","commit_id":"a6f7a7a8c679b08c6d22e7253b0d28b80849a140"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"3a602f80865598dd94fa46f5ba32885214554c51","unresolved":false,"context_lines":[{"line_number":6112,"context_line":"                                                   mock_getmtime):"},{"line_number":6113,"context_line":"        # get_hashes must repeat path listing and return all hashes when"},{"line_number":6114,"context_line":"        # another concurrent process created new pkl before hashes are stored"},{"line_number":6115,"context_line":"        # by the first process - when pkl didn\u0027t exist"},{"line_number":6116,"context_line":"        non_local \u003d {}"},{"line_number":6117,"context_line":""},{"line_number":6118,"context_line":"        def mock_write_pickle_def(*args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a3905b3_8035a447","line":6115,"range":{"start_line":6115,"start_character":32,"end_line":6115,"end_character":54},"updated":"2017-01-19 16:21:04.000000000","message":"nit: no longer applies - pkl does exist","commit_id":"a6f7a7a8c679b08c6d22e7253b0d28b80849a140"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9cf94775f34878ebfe8353e4fc075bea03170e0b","unresolved":false,"context_lines":[{"line_number":6112,"context_line":"                                                   mock_getmtime):"},{"line_number":6113,"context_line":"        # get_hashes must repeat path listing and return all hashes when"},{"line_number":6114,"context_line":"        # another concurrent process created new pkl before hashes are stored"},{"line_number":6115,"context_line":"        # by the first process - when pkl didn\u0027t exist"},{"line_number":6116,"context_line":"        non_local \u003d {}"},{"line_number":6117,"context_line":""},{"line_number":6118,"context_line":"        def mock_write_pickle_def(*args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a3905b3_9ddb4aed","line":6115,"range":{"start_line":6115,"start_character":32,"end_line":6115,"end_character":54},"in_reply_to":"5a3905b3_8035a447","updated":"2017-01-19 16:25:31.000000000","message":"Done","commit_id":"a6f7a7a8c679b08c6d22e7253b0d28b80849a140"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"3a602f80865598dd94fa46f5ba32885214554c51","unresolved":false,"context_lines":[{"line_number":6156,"context_line":"                result \u003d orig_listdir(*args, **kwargs)"},{"line_number":6157,"context_line":"                if not non_local[\u0027df2touched\u0027]:"},{"line_number":6158,"context_line":"                    non_local[\u0027df2touched\u0027] \u003d True"},{"line_number":6159,"context_line":"                    df2.delete(self.ts())"},{"line_number":6160,"context_line":"                    # simulate pkl update by other process - mtime is updated"},{"line_number":6161,"context_line":"                    self.assertIn(\u0027mtime\u0027, non_local, \"hashes.pkl must exist\")"},{"line_number":6162,"context_line":"                    non_local[\u0027mtime\u0027] +\u003d 1"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a3905b3_0b1e3563","line":6159,"updated":"2017-01-19 16:21:04.000000000","message":"ok this is necessary to create some new state that can be used to verify the first process get_hashes did repeat.","commit_id":"a6f7a7a8c679b08c6d22e7253b0d28b80849a140"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"3a602f80865598dd94fa46f5ba32885214554c51","unresolved":false,"context_lines":[{"line_number":6159,"context_line":"                    df2.delete(self.ts())"},{"line_number":6160,"context_line":"                    # simulate pkl update by other process - mtime is updated"},{"line_number":6161,"context_line":"                    self.assertIn(\u0027mtime\u0027, non_local, \"hashes.pkl must exist\")"},{"line_number":6162,"context_line":"                    non_local[\u0027mtime\u0027] +\u003d 1"},{"line_number":6163,"context_line":"                return result"},{"line_number":6164,"context_line":""},{"line_number":6165,"context_line":"            with mock.patch(\u0027swift.obj.diskfile.os.listdir\u0027,"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a3905b3_8b26a566","line":6162,"updated":"2017-01-19 16:21:04.000000000","message":"ok. this then simulates some other process has run get_hashes and written a hashes.pkl. In reality that get_hashes would have consolidated the new state into hashes.pkl whereas here it is left in hashes.invalid, but this is not reality and the df2.delete could just as well have occurred after the second get_hashes had consolidated state.\n\nThe point is that there is new state that the first get_hashes should go back and discover.","commit_id":"a6f7a7a8c679b08c6d22e7253b0d28b80849a140"},{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"729ad1250a1e7c7df568ac423bc66526aacbd1f9","unresolved":false,"context_lines":[{"line_number":6204,"context_line":"            invalidations_file \u003d os.path.join("},{"line_number":6205,"context_line":"                part_path, diskfile.HASH_INVALIDATIONS_FILE)"},{"line_number":6206,"context_line":"            with open(hashes_file, \u0027rb\u0027) as f:"},{"line_number":6207,"context_line":"                self.assertEqual(original_hashes, pickle.load(f))"},{"line_number":6208,"context_line":""},{"line_number":6209,"context_line":"            # invalidate the hash"},{"line_number":6210,"context_line":"            with mock.patch(\u0027swift.obj.diskfile.lock_path\u0027) as mock_lock:"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a3905b3_27db46ef","line":6207,"updated":"2017-01-21 20:02:01.000000000","message":"just curious, is there a reason why we don\u0027t have an \"self.assertTrue(os.path.exists(invalidations_file))\" here in place of what used to be assertFalse?","commit_id":"a6f7a7a8c679b08c6d22e7253b0d28b80849a140"}]}