)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4151341f3e786d8ec4c974eae6a7742991b08d98","unresolved":false,"context_lines":[{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Using getmtime has always had somewhat dubious resolution for race"},{"line_number":17,"context_line":"detection - the only way to be sure the content of the file is the same"},{"line_number":18,"context_line":"as when we read it without locking is to open the file up and check."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Along the way, we accidently fix a serious performance regression with"},{"line_number":21,"context_line":"hash invalidations..."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"3a461143_2b5912ff","line":18,"updated":"2017-01-26 18:49:57.000000000","message":"this is true, but content being the same doesn\u0027t mean there have not been subsequent invalidations of the same suffix (I think this is what Pavel points out) I think?\n\n#brainstorming - do we actually need to write something other than None to indicate that a suffix is out of date, something unique per process? that\u0027s hard because the hashes dict values are already different types according to policy (string for replication, dict for EC).\n\nor add a non-suffix-dir-like key to the hashes dict under which we can record an epoch (random number or timestamp) for the content. [1]\n\n[1] quick hack... https://gist.github.com/alistairncoles/2591e6015ba93d5f7b5e800f41feddf1","commit_id":"24276337fc1b0ac4bc91ce5838750c8a93af8420"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4f4bdd1418187ab1f05b0426bec003e37aac5354","unresolved":false,"context_lines":[{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Using getmtime has always had somewhat dubious resolution for race"},{"line_number":17,"context_line":"detection - the only way to be sure the content of the file is the same"},{"line_number":18,"context_line":"as when we read it without locking is to open the file up and check."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Along the way, we accidently fix a serious performance regression with"},{"line_number":21,"context_line":"hash invalidations..."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"3a461143_62ab251e","line":18,"in_reply_to":"3a461143_2b5912ff","updated":"2017-01-27 06:17:47.000000000","message":"I like it!","commit_id":"24276337fc1b0ac4bc91ce5838750c8a93af8420"}],"swift/obj/diskfile.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"fa784fb713a7fe434e6a8769dba51c7f76c0fa6c","unresolved":false,"context_lines":[{"line_number":1037,"context_line":"        hashed \u003d 0"},{"line_number":1038,"context_line":"        hashes_file \u003d join(partition_path, HASH_FILE)"},{"line_number":1039,"context_line":"        modified \u003d False"},{"line_number":1040,"context_line":"        force_rewrite \u003d False"},{"line_number":1041,"context_line":"        hashes \u003d {}"},{"line_number":1042,"context_line":"        mtime \u003d -1"},{"line_number":1043,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"5a3905b3_bf99d570","side":"PARENT","line":1040,"updated":"2017-01-19 18:41:24.000000000","message":"simply redundant","commit_id":"e772cf95c65b98bb17b094585ff3ff2040c148c9"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"fa784fb713a7fe434e6a8769dba51c7f76c0fa6c","unresolved":false,"context_lines":[{"line_number":1081,"context_line":"                modified \u003d True"},{"line_number":1082,"context_line":"        if modified:"},{"line_number":1083,"context_line":"            with lock_path(partition_path):"},{"line_number":1084,"context_line":"                if force_rewrite or not exists(hashes_file) or \\"},{"line_number":1085,"context_line":"                        getmtime(hashes_file) \u003d\u003d mtime:"},{"line_number":1086,"context_line":"                    write_pickle("},{"line_number":1087,"context_line":"                        hashes, hashes_file, partition_path, PICKLE_PROTOCOL)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a3905b3_bffe3569","side":"PARENT","line":1084,"updated":"2017-01-19 18:41:24.000000000","message":"force_rewrite \u003d\u003d True implies either hases.pkl did not exist (which is second condition) or hashes.pkl did exist in which case mtime is valid to compare with getmtime (which is third condition) so first condition is unnecessary.","commit_id":"e772cf95c65b98bb17b094585ff3ff2040c148c9"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"fa784fb713a7fe434e6a8769dba51c7f76c0fa6c","unresolved":false,"context_lines":[{"line_number":1053,"context_line":"        if recalculate is None:"},{"line_number":1054,"context_line":"            recalculate \u003d []"},{"line_number":1055,"context_line":""},{"line_number":1056,"context_line":"        inode \u003d safe_get_inode(hashes_file)"},{"line_number":1057,"context_line":""},{"line_number":1058,"context_line":"        try:"},{"line_number":1059,"context_line":"            hashes \u003d self.consolidate_hashes(partition_path)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a3905b3_1f3d892b","line":1056,"updated":"2017-01-19 18:41:24.000000000","message":"inode \u003d None if no hashes.pkl or an int","commit_id":"c518df1bce922cf68f2ccab1aa5a8a1b580eab88"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"bdd9672f722e601030e44e8d7d829fcc05913f99","unresolved":false,"context_lines":[{"line_number":1056,"context_line":"        inode \u003d safe_get_inode(hashes_file)"},{"line_number":1057,"context_line":""},{"line_number":1058,"context_line":"        try:"},{"line_number":1059,"context_line":"            hashes \u003d self.consolidate_hashes(partition_path)"},{"line_number":1060,"context_line":"        except Exception:"},{"line_number":1061,"context_line":"            do_listdir \u003d True"},{"line_number":1062,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a3905b3_8212feb2","line":1059,"updated":"2017-01-19 18:42:21.000000000","message":"at this point, inode will always be an int if the hashes.pkl exists\n\ntest_consolidate_hashes_raises_exception will catch a regression that moves line 1056 to after here","commit_id":"c518df1bce922cf68f2ccab1aa5a8a1b580eab88"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"fa784fb713a7fe434e6a8769dba51c7f76c0fa6c","unresolved":false,"context_lines":[{"line_number":1084,"context_line":"                modified \u003d True"},{"line_number":1085,"context_line":"        if modified:"},{"line_number":1086,"context_line":"            with lock_path(partition_path):"},{"line_number":1087,"context_line":"                if safe_get_inode(hashes_file) \u003d\u003d inode:"},{"line_number":1088,"context_line":"                    write_pickle("},{"line_number":1089,"context_line":"                        hashes, hashes_file, partition_path, PICKLE_PROTOCOL)"},{"line_number":1090,"context_line":"                    return hashed, hashes"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a3905b3_9f6899f7","line":1087,"updated":"2017-01-19 18:41:24.000000000","message":"ok\n\n  safe_get_inode()   inode   outcome\n  ----------------   ------  --------\n    None              None   write pickle\n    int               None   don\u0027t write, someone else got there first\n    None              int    don\u0027t write pickle, someone deleted it??\n    int               int    write pickle, it\u0027s same one we read\n\nre. 3rd row: IF hashes.pkl was deleted between the read and test for write, we loop back and will reach this test again with inode\u003dNone","commit_id":"c518df1bce922cf68f2ccab1aa5a8a1b580eab88"},{"author":{"_account_id":23449,"name":"Pavel Kvasnička","email":"pavel.kvasnicka@firma.seznam.cz","username":"PavelK"},"change_message_id":"468730fbbb5603bc8e5cc9cae7c2e1e2179bcbb9","unresolved":false,"context_lines":[{"line_number":275,"context_line":"            # no hashes at all -\u003e everything\u0027s invalid, so empty the file with"},{"line_number":276,"context_line":"            # the invalid suffixes in it, if it exists"},{"line_number":277,"context_line":"            try:"},{"line_number":278,"context_line":"                with open(invalidations_file, \u0027wb\u0027):"},{"line_number":279,"context_line":"                    pass"},{"line_number":280,"context_line":"            except OSError as e:"},{"line_number":281,"context_line":"                if e.errno !\u003d errno.ENOENT:"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a461143_3c3c1293","line":278,"range":{"start_line":278,"start_character":21,"end_line":278,"end_character":51},"updated":"2017-01-26 09:52:54.000000000","message":"If you accept to notice new invalidation entry (update pkl) when \"modified or found_invalidation_entry\" below, this truncation should be solved to.\n\nWhen consolidate_hashes is called on not existing pkl during parallel hashes calculation and a suffix (new or existing) was invalidated then the suffix is not included in hashes or invalid (old) value is in hashes.","commit_id":"24276337fc1b0ac4bc91ce5838750c8a93af8420"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0934b503aeccd8c8355c4ecae9ed2cc618732bb6","unresolved":false,"context_lines":[{"line_number":280,"context_line":"            except OSError as e:"},{"line_number":281,"context_line":"                if e.errno !\u003d errno.ENOENT:"},{"line_number":282,"context_line":"                    raise"},{"line_number":283,"context_line":"        return None"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"    with lock_path(partition_dir):"},{"line_number":286,"context_line":"        hashes \u003d read_hashes(hashes_file)"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a461143_38ca7e50","line":283,"updated":"2017-01-25 19:17:31.000000000","message":"follow-up: I think it\u0027d be good to try and take out all this special casing here.  If hashes.pkl does not exist or is invalid - there some good reasons to create it.","commit_id":"24276337fc1b0ac4bc91ce5838750c8a93af8420"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0934b503aeccd8c8355c4ecae9ed2cc618732bb6","unresolved":false,"context_lines":[{"line_number":292,"context_line":"                for line in inv_fh:"},{"line_number":293,"context_line":"                    found_invalidation_entry \u003d True"},{"line_number":294,"context_line":"                    suffix \u003d line.strip()"},{"line_number":295,"context_line":"                    if hashes is not None and \\"},{"line_number":296,"context_line":"                            hashes.get(suffix, \u0027\u0027) is not None:"},{"line_number":297,"context_line":"                        hashes[suffix] \u003d None"},{"line_number":298,"context_line":"                        modified \u003d True"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a461143_b8110eb2","line":295,"updated":"2017-01-25 19:17:31.000000000","message":"followup: if hashes is None we don\u0027t really have a reason to evaluate every line in the inv_fh individually.","commit_id":"24276337fc1b0ac4bc91ce5838750c8a93af8420"},{"author":{"_account_id":23449,"name":"Pavel Kvasnička","email":"pavel.kvasnicka@firma.seznam.cz","username":"PavelK"},"change_message_id":"a2698f5c914cba827830c1e1d41ad9a80b15ce9b","unresolved":false,"context_lines":[{"line_number":300,"context_line":"            if e.errno !\u003d errno.ENOENT:"},{"line_number":301,"context_line":"                raise"},{"line_number":302,"context_line":""},{"line_number":303,"context_line":"        if modified:"},{"line_number":304,"context_line":"            write_pickle(hashes, hashes_file, partition_dir, PICKLE_PROTOCOL)"},{"line_number":305,"context_line":""},{"line_number":306,"context_line":"        # Now that all the invalidations are reflected in hashes.pkl, it\u0027s"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a461143_2b268eba","line":303,"range":{"start_line":303,"start_character":11,"end_line":303,"end_character":19},"updated":"2017-01-26 08:32:11.000000000","message":"modified or found_invalidation_entry\n\n- When invalidation entry found and hash for that suffix is already None we need notice that other process can\u0027t write new pkl with old hash (see better description below)","commit_id":"24276337fc1b0ac4bc91ce5838750c8a93af8420"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4151341f3e786d8ec4c974eae6a7742991b08d98","unresolved":false,"context_lines":[{"line_number":300,"context_line":"            if e.errno !\u003d errno.ENOENT:"},{"line_number":301,"context_line":"                raise"},{"line_number":302,"context_line":""},{"line_number":303,"context_line":"        if modified:"},{"line_number":304,"context_line":"            write_pickle(hashes, hashes_file, partition_dir, PICKLE_PROTOCOL)"},{"line_number":305,"context_line":""},{"line_number":306,"context_line":"        # Now that all the invalidations are reflected in hashes.pkl, it\u0027s"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a461143_79913352","line":303,"range":{"start_line":303,"start_character":11,"end_line":303,"end_character":19},"in_reply_to":"3a461143_2b268eba","updated":"2017-01-26 18:49:57.000000000","message":"agree there is an issue here but not sure how adding found_invalidation_entry as a condition helps - the hashes.pkl content will be identical even if is rewritten.","commit_id":"24276337fc1b0ac4bc91ce5838750c8a93af8420"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0934b503aeccd8c8355c4ecae9ed2cc618732bb6","unresolved":false,"context_lines":[{"line_number":307,"context_line":"        # safe to clear out the invalidations file."},{"line_number":308,"context_line":"        if found_invalidation_entry:"},{"line_number":309,"context_line":"            with open(invalidations_file, \u0027wb\u0027) as inv_fh:"},{"line_number":310,"context_line":"                pass"},{"line_number":311,"context_line":""},{"line_number":312,"context_line":"        return hashes"},{"line_number":313,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"3a461143_d8089a94","line":310,"updated":"2017-01-25 19:17:31.000000000","message":"followup: I think it\u0027s sub-optimal to have to open the file again - inv_fh.truncate would be better.","commit_id":"24276337fc1b0ac4bc91ce5838750c8a93af8420"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0934b503aeccd8c8355c4ecae9ed2cc618732bb6","unresolved":false,"context_lines":[{"line_number":1035,"context_line":"        :param do_listdir: force existence check for all hashes in the"},{"line_number":1036,"context_line":"                           partition"},{"line_number":1037,"context_line":""},{"line_number":1038,"context_line":"        :returns: tuple of (number of suffix dirs hashed, dictionary of hashes)"},{"line_number":1039,"context_line":"        \"\"\""},{"line_number":1040,"context_line":"        hashed \u003d 0"},{"line_number":1041,"context_line":"        hashes_file \u003d join(partition_path, HASH_FILE)"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a461143_c7c5165c","line":1038,"updated":"2017-01-25 19:17:31.000000000","message":"followup: this function can also raise a few exceptions - most notable is LockTimeout","commit_id":"24276337fc1b0ac4bc91ce5838750c8a93af8420"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0934b503aeccd8c8355c4ecae9ed2cc618732bb6","unresolved":false,"context_lines":[{"line_number":1049,"context_line":"            orig_hashes \u003d self.consolidate_hashes(partition_path)"},{"line_number":1050,"context_line":"        except Exception:"},{"line_number":1051,"context_line":"            self.logger.warning(\u0027Unable to read %r\u0027, hashes_file,"},{"line_number":1052,"context_line":"                                exc_info\u003dTrue)"},{"line_number":1053,"context_line":""},{"line_number":1054,"context_line":"        if orig_hashes is None:"},{"line_number":1055,"context_line":"            hashes \u003d {}"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a461143_67a3829f","line":1052,"updated":"2017-01-25 19:17:31.000000000","message":"followup: I\u0027ve grown increasing uncomfortable with our handling of a LockTimeout from consolidate_hashes - it seems like going forward and rehashing *everything* is the worst possible handling for that error :\\","commit_id":"24276337fc1b0ac4bc91ce5838750c8a93af8420"},{"author":{"_account_id":23449,"name":"Pavel Kvasnička","email":"pavel.kvasnicka@firma.seznam.cz","username":"PavelK"},"change_message_id":"a2698f5c914cba827830c1e1d41ad9a80b15ce9b","unresolved":false,"context_lines":[{"line_number":1069,"context_line":"            for suff in os.listdir(partition_path):"},{"line_number":1070,"context_line":"                if len(suff) \u003d\u003d 3:"},{"line_number":1071,"context_line":"                    hashes.setdefault(suff, None)"},{"line_number":1072,"context_line":"            modified \u003d True"},{"line_number":1073,"context_line":"            self.logger.debug(\u0027Run listdir on %s\u0027, partition_path)"},{"line_number":1074,"context_line":"        hashes.update((suffix, None) for suffix in recalculate)"},{"line_number":1075,"context_line":"        for suffix, hash_ in hashes.items():"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a461143_cde7e23e","line":1072,"range":{"start_line":1072,"start_character":12,"end_line":1072,"end_character":27},"updated":"2017-01-26 08:32:11.000000000","message":"follow-up: I\u0027m sure that setting `modified` flag is wrong - if any hash appears then `modified` will be set after hash calculation below.\n\nReplicator sets do_listdir for each 10th partition and we believe that hashes are usually correct and not changed.","commit_id":"24276337fc1b0ac4bc91ce5838750c8a93af8420"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0934b503aeccd8c8355c4ecae9ed2cc618732bb6","unresolved":false,"context_lines":[{"line_number":1069,"context_line":"            for suff in os.listdir(partition_path):"},{"line_number":1070,"context_line":"                if len(suff) \u003d\u003d 3:"},{"line_number":1071,"context_line":"                    hashes.setdefault(suff, None)"},{"line_number":1072,"context_line":"            modified \u003d True"},{"line_number":1073,"context_line":"            self.logger.debug(\u0027Run listdir on %s\u0027, partition_path)"},{"line_number":1074,"context_line":"        hashes.update((suffix, None) for suffix in recalculate)"},{"line_number":1075,"context_line":"        for suffix, hash_ in hashes.items():"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a461143_27589afa","line":1072,"updated":"2017-01-25 19:17:31.000000000","message":"followup: I\u0027m never quite been able to convince myself that do_listdir should always trigger a writeback - it may not even invalidate any hashes?","commit_id":"24276337fc1b0ac4bc91ce5838750c8a93af8420"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4f4bdd1418187ab1f05b0426bec003e37aac5354","unresolved":false,"context_lines":[{"line_number":1069,"context_line":"            for suff in os.listdir(partition_path):"},{"line_number":1070,"context_line":"                if len(suff) \u003d\u003d 3:"},{"line_number":1071,"context_line":"                    hashes.setdefault(suff, None)"},{"line_number":1072,"context_line":"            modified \u003d True"},{"line_number":1073,"context_line":"            self.logger.debug(\u0027Run listdir on %s\u0027, partition_path)"},{"line_number":1074,"context_line":"        hashes.update((suffix, None) for suffix in recalculate)"},{"line_number":1075,"context_line":"        for suffix, hash_ in hashes.items():"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a461143_0216e154","line":1072,"in_reply_to":"3a461143_cde7e23e","updated":"2017-01-27 06:17:47.000000000","message":"good to have some validation!\n\nlet\u0027s file a bug for this separately - it\u0027s an optimization but a good one - and it\u0027ll be nice to have test.","commit_id":"24276337fc1b0ac4bc91ce5838750c8a93af8420"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0934b503aeccd8c8355c4ecae9ed2cc618732bb6","unresolved":false,"context_lines":[{"line_number":1076,"context_line":"            if not hash_:"},{"line_number":1077,"context_line":"                suffix_dir \u003d join(partition_path, suffix)"},{"line_number":1078,"context_line":"                try:"},{"line_number":1079,"context_line":"                    hashes[suffix] \u003d self._hash_suffix(suffix_dir)"},{"line_number":1080,"context_line":"                    hashed +\u003d 1"},{"line_number":1081,"context_line":"                except PathNotDir:"},{"line_number":1082,"context_line":"                    del hashes[suffix]"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a461143_d8a5fab5","line":1079,"updated":"2017-01-25 19:17:31.000000000","message":"I think the way this is updated we *could* get by w/o the *deep* copy - but it seems brittle to avoid and cheap enough to be safe/correct.","commit_id":"24276337fc1b0ac4bc91ce5838750c8a93af8420"},{"author":{"_account_id":23449,"name":"Pavel Kvasnička","email":"pavel.kvasnicka@firma.seznam.cz","username":"PavelK"},"change_message_id":"a2698f5c914cba827830c1e1d41ad9a80b15ce9b","unresolved":false,"context_lines":[{"line_number":1086,"context_line":""},{"line_number":1087,"context_line":"        if modified:"},{"line_number":1088,"context_line":"            with lock_path(partition_path):"},{"line_number":1089,"context_line":"                if read_hashes(hashes_file) \u003d\u003d orig_hashes:"},{"line_number":1090,"context_line":"                    write_pickle("},{"line_number":1091,"context_line":"                        hashes, hashes_file, partition_path, PICKLE_PROTOCOL)"},{"line_number":1092,"context_line":"                    return hashed, hashes"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a461143_4d0cd294","line":1089,"range":{"start_line":1089,"start_character":47,"end_line":1089,"end_character":58},"updated":"2017-01-26 08:32:11.000000000","message":"safe_get_inode or getmtime is necessary!\n\nobj-server            | replicate1              | replicate2\n----------------------|-------------------------|------------\ninvalidate suffix abc |                         |\n                      | consolidate_hashes      |\n                      | calculate hash for abc  |\ninvalidate suffix abc |                         |\n                      |                         | consolidate_hashes\nwrite pkl file        |                         | calculate hash for abc\n                      |                         | read_hashes() !\u003d orig_hahes\n                      |                         |  -\u003e _get_hashes called recursively\n                      |                         | consolidate_hashes returns hashes without invalidations\n\n\nSecond suffix invalidation is lost because second replicate job clears invalidations file, first \"replicate\" seems \"abc\" suffix hash to be None and writes its result. The second \"replicate\" founds changed hashes, leaves its work and repeats _get_hashes where no invalidation entry exists.","commit_id":"24276337fc1b0ac4bc91ce5838750c8a93af8420"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"b13c8865c599daed4603324ec639a81f7628fb81","unresolved":false,"context_lines":[{"line_number":1086,"context_line":""},{"line_number":1087,"context_line":"        if modified:"},{"line_number":1088,"context_line":"            with lock_path(partition_path):"},{"line_number":1089,"context_line":"                if read_hashes(hashes_file) \u003d\u003d orig_hashes:"},{"line_number":1090,"context_line":"                    write_pickle("},{"line_number":1091,"context_line":"                        hashes, hashes_file, partition_path, PICKLE_PROTOCOL)"},{"line_number":1092,"context_line":"                    return hashed, hashes"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a461143_85792aff","line":1089,"range":{"start_line":1089,"start_character":47,"end_line":1089,"end_character":58},"in_reply_to":"3a461143_4d0cd294","updated":"2017-01-26 11:13:59.000000000","message":"Before this patch we ensured that the hashes.pkl itself was unmodified (using getmtime). However, this also didn\u0027t detect if invalidate_hash was called after consolidate_hashes a few lines above. So it\u0027s not worse than before, but could need a follow up?\n\nRegarding mtime - actually I think this might either need to check the mtime of the dir itself, or check if there is a (new) invalidation file at this time.","commit_id":"24276337fc1b0ac4bc91ce5838750c8a93af8420"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4f4bdd1418187ab1f05b0426bec003e37aac5354","unresolved":false,"context_lines":[{"line_number":1086,"context_line":""},{"line_number":1087,"context_line":"        if modified:"},{"line_number":1088,"context_line":"            with lock_path(partition_path):"},{"line_number":1089,"context_line":"                if read_hashes(hashes_file) \u003d\u003d orig_hashes:"},{"line_number":1090,"context_line":"                    write_pickle("},{"line_number":1091,"context_line":"                        hashes, hashes_file, partition_path, PICKLE_PROTOCOL)"},{"line_number":1092,"context_line":"                    return hashed, hashes"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a461143_425aa951","line":1089,"in_reply_to":"3a461143_706e9d30","updated":"2017-01-27 06:17:47.000000000","message":"as ever I stand on the backs of giants.","commit_id":"24276337fc1b0ac4bc91ce5838750c8a93af8420"},{"author":{"_account_id":23449,"name":"Pavel Kvasnička","email":"pavel.kvasnicka@firma.seznam.cz","username":"PavelK"},"change_message_id":"f5b461f294950ab0702bb678fe84b93a7dcad2b1","unresolved":false,"context_lines":[{"line_number":1086,"context_line":""},{"line_number":1087,"context_line":"        if modified:"},{"line_number":1088,"context_line":"            with lock_path(partition_path):"},{"line_number":1089,"context_line":"                if read_hashes(hashes_file) \u003d\u003d orig_hashes:"},{"line_number":1090,"context_line":"                    write_pickle("},{"line_number":1091,"context_line":"                        hashes, hashes_file, partition_path, PICKLE_PROTOCOL)"},{"line_number":1092,"context_line":"                    return hashed, hashes"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a461143_ebe2bf02","line":1089,"range":{"start_line":1089,"start_character":47,"end_line":1089,"end_character":58},"in_reply_to":"3a461143_85792aff","updated":"2017-01-26 11:55:36.000000000","message":"Right, that\u0027s new bug.\n\nIt\u0027s correct to change invalidation file when hashes are calculated - we just must know that invalidation entry appeared and another process cleared it.","commit_id":"24276337fc1b0ac4bc91ce5838750c8a93af8420"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4151341f3e786d8ec4c974eae6a7742991b08d98","unresolved":false,"context_lines":[{"line_number":1086,"context_line":""},{"line_number":1087,"context_line":"        if modified:"},{"line_number":1088,"context_line":"            with lock_path(partition_path):"},{"line_number":1089,"context_line":"                if read_hashes(hashes_file) \u003d\u003d orig_hashes:"},{"line_number":1090,"context_line":"                    write_pickle("},{"line_number":1091,"context_line":"                        hashes, hashes_file, partition_path, PICKLE_PROTOCOL)"},{"line_number":1092,"context_line":"                    return hashed, hashes"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a461143_ede005cf","line":1089,"range":{"start_line":1089,"start_character":47,"end_line":1089,"end_character":58},"in_reply_to":"3a461143_ebe2bf02","updated":"2017-01-26 18:49:57.000000000","message":"I think Pavel has a point - identical hashes.pkl content does not mean that nothing has changed in the suffix dirs since hashes.pkl was last read. A suffix entry may still be None but the suffix dir has been invalidated since the current process called hash_suffix().\n\nThis test passes on master but fails with this patch:\n\nhttps://gist.github.com/alistairncoles/43b3e9f266cce0be13793b6dd97aaaa5","commit_id":"24276337fc1b0ac4bc91ce5838750c8a93af8420"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"36df8e4bc5810e6670ca9441bc96b80bba2d4032","unresolved":false,"context_lines":[{"line_number":1086,"context_line":""},{"line_number":1087,"context_line":"        if modified:"},{"line_number":1088,"context_line":"            with lock_path(partition_path):"},{"line_number":1089,"context_line":"                if read_hashes(hashes_file) \u003d\u003d orig_hashes:"},{"line_number":1090,"context_line":"                    write_pickle("},{"line_number":1091,"context_line":"                        hashes, hashes_file, partition_path, PICKLE_PROTOCOL)"},{"line_number":1092,"context_line":"                    return hashed, hashes"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a461143_706e9d30","line":1089,"in_reply_to":"3a461143_ede005cf","updated":"2017-01-26 20:30:00.000000000","message":"In my frustration with the failing of the inode check - I obviously missed this.  I am so grateful to Pavel for finding it - acoles test looks perfect - I\u0027ll definitely roll it into the next revision.  We should not merge this change - it is wrong/broken/steaming-pile-of-shit.  :\u0027(\n\n*finally* \"acceptance\"\n\nYou can do this Clay.  Don\u0027t give up.","commit_id":"24276337fc1b0ac4bc91ce5838750c8a93af8420"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0934b503aeccd8c8355c4ecae9ed2cc618732bb6","unresolved":false,"context_lines":[{"line_number":1284,"context_line":"        if not os.path.exists(partition_path):"},{"line_number":1285,"context_line":"            mkdirs(partition_path)"},{"line_number":1286,"context_line":"        _junk, hashes \u003d tpool_reraise("},{"line_number":1287,"context_line":"            self._get_hashes, partition_path, recalculate\u003dsuffixes)"},{"line_number":1288,"context_line":"        return hashes"},{"line_number":1289,"context_line":""},{"line_number":1290,"context_line":"    def _listdir(self, path):"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a461143_876a2e19","line":1287,"updated":"2017-01-25 19:17:31.000000000","message":"followup: rather than reraise here out to the REPLICATE handler (and generic __call__ exception handling) I sort of wish this would pokemon and re-raise some more generic DiskError that we could cleanly translate into a 503 of some kind.  Related; I\u0027d like to *remove* some of exception handling in _get_hashes and let that method have a reasonable out if it thinks it be safer to halt than proceed blindly.","commit_id":"24276337fc1b0ac4bc91ce5838750c8a93af8420"},{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"82509af1327c2a657a531fb7f3c0a66b37c9fad0","unresolved":false,"context_lines":[{"line_number":256,"context_line":"            # input is invalid."},{"line_number":257,"context_line":"            pass"},{"line_number":258,"context_line":"    if hashes is not None:"},{"line_number":259,"context_line":"        # hashes.pkl w/o updated key if \"forever old\""},{"line_number":260,"context_line":"        hashes.setdefault(\u0027updated\u0027, -1)"},{"line_number":261,"context_line":"    return hashes"},{"line_number":262,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"3a461143_da366fe6","line":259,"updated":"2017-01-27 14:20:45.000000000","message":"nit: s/if/is?","commit_id":"c8a628e75ad75141a5472f23820be9fe47e89308"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3930ddcf0c97d919c7e018dd19e5119e87ae0cb","unresolved":false,"context_lines":[{"line_number":268,"context_line":"    The updated key is added to hashes before it is written."},{"line_number":269,"context_line":"    \"\"\""},{"line_number":270,"context_line":"    hashes_file \u003d join(partition_dir, HASH_FILE)"},{"line_number":271,"context_line":"    hashes[\u0027updated\u0027] \u003d time.time()"},{"line_number":272,"context_line":"    return write_pickle(hashes, hashes_file, partition_dir, PICKLE_PROTOCOL)"},{"line_number":273,"context_line":""},{"line_number":274,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"3a461143_aeb16591","line":271,"updated":"2017-01-27 16:08:58.000000000","message":"Thanks for sanitising the gist...you won\u0027t believe me but I used time.time() first then switched to uuid for some reason I don\u0027t recall :) This is good and \u0027updated\u0027 is a better key than \u0027magic\u0027.","commit_id":"c8a628e75ad75141a5472f23820be9fe47e89308"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"1c2da30226ce85143daae4a62a86820087cdcd4f","unresolved":false,"context_lines":[{"line_number":269,"context_line":"    \"\"\""},{"line_number":270,"context_line":"    hashes_file \u003d join(partition_dir, HASH_FILE)"},{"line_number":271,"context_line":"    hashes[\u0027updated\u0027] \u003d time.time()"},{"line_number":272,"context_line":"    return write_pickle(hashes, hashes_file, partition_dir, PICKLE_PROTOCOL)"},{"line_number":273,"context_line":""},{"line_number":274,"context_line":""},{"line_number":275,"context_line":"def consolidate_hashes(partition_dir):"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a461143_489c8b69","line":272,"updated":"2017-01-27 06:46:19.000000000","message":"something about the existence of strip_self in this module makes me feel bad about making this toplevel functions.  But at the same time I recognize I\u0027d feel just as bad if I made them methods when they really don\u0027t need to have the instance state.\n\n\u003cshrugs\u003e","commit_id":"c8a628e75ad75141a5472f23820be9fe47e89308"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3930ddcf0c97d919c7e018dd19e5119e87ae0cb","unresolved":false,"context_lines":[{"line_number":269,"context_line":"    \"\"\""},{"line_number":270,"context_line":"    hashes_file \u003d join(partition_dir, HASH_FILE)"},{"line_number":271,"context_line":"    hashes[\u0027updated\u0027] \u003d time.time()"},{"line_number":272,"context_line":"    return write_pickle(hashes, hashes_file, partition_dir, PICKLE_PROTOCOL)"},{"line_number":273,"context_line":""},{"line_number":274,"context_line":""},{"line_number":275,"context_line":"def consolidate_hashes(partition_dir):"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a461143_e3caa66d","line":272,"in_reply_to":"3a461143_489c8b69","updated":"2017-01-27 16:08:58.000000000","message":"this wfm, strip_self makes me have to make two steps to navigate to actual function definition in my IDE","commit_id":"c8a628e75ad75141a5472f23820be9fe47e89308"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3930ddcf0c97d919c7e018dd19e5119e87ae0cb","unresolved":false,"context_lines":[{"line_number":313,"context_line":"            if e.errno !\u003d errno.ENOENT:"},{"line_number":314,"context_line":"                raise"},{"line_number":315,"context_line":""},{"line_number":316,"context_line":"        if found_invalidation_entry:"},{"line_number":317,"context_line":"            write_hashes(partition_dir, hashes)"},{"line_number":318,"context_line":""},{"line_number":319,"context_line":"        # Now that all the invalidations are reflected in hashes.pkl, it\u0027s"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a461143_4390da43","line":316,"updated":"2017-01-27 16:08:58.000000000","message":"yep, modified became redundant","commit_id":"c8a628e75ad75141a5472f23820be9fe47e89308"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"674f108b6ea10ff2b982f212caa76d237c7f1111","unresolved":false,"context_lines":[{"line_number":313,"context_line":"            if e.errno !\u003d errno.ENOENT:"},{"line_number":314,"context_line":"                raise"},{"line_number":315,"context_line":""},{"line_number":316,"context_line":"        if found_invalidation_entry:"},{"line_number":317,"context_line":"            write_hashes(partition_dir, hashes)"},{"line_number":318,"context_line":""},{"line_number":319,"context_line":"        # Now that all the invalidations are reflected in hashes.pkl, it\u0027s"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a430d35_de93244e","line":316,"in_reply_to":"1a430d35_0a11eeac","updated":"2017-02-03 08:59:00.000000000","message":"Pavel discovered a race here if you don\u0027t write back even when None becomes None.\n\n\n    process A                        | process B               | process C\n    ---------------------------------|-------------------------|--------------------------------\n    consolidate_hashes (sufA \u003d None) |                         |\n    _hash_suffix (sufA)              | invalidate_hash (sufA)  |\n    ...                              |                         | conslidate_hashes (sufA \u003d None)\n    write_hashes                     |                         |\n\nthat write_hashes call can\u0027t write down the calculated hash for sufA - it was invalidated after the call to _hash_suffix.  write_hashes need to trip race detection and rehash.  If process C conslidate_hashes call writes_back (with new updated time) that sufA \u003d None (even though when it read it it was *already* None) then process A will detect the race.","commit_id":"c8a628e75ad75141a5472f23820be9fe47e89308"},{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"d012bd8dd8d12448f3d0ca13e942d9f8760ec0bc","unresolved":false,"context_lines":[{"line_number":313,"context_line":"            if e.errno !\u003d errno.ENOENT:"},{"line_number":314,"context_line":"                raise"},{"line_number":315,"context_line":""},{"line_number":316,"context_line":"        if found_invalidation_entry:"},{"line_number":317,"context_line":"            write_hashes(partition_dir, hashes)"},{"line_number":318,"context_line":""},{"line_number":319,"context_line":"        # Now that all the invalidations are reflected in hashes.pkl, it\u0027s"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a430d35_0a11eeac","line":316,"in_reply_to":"3a461143_4390da43","updated":"2017-02-03 06:06:05.000000000","message":"I am having a hard time seeing why it is completely redundant.  \n\nI tried adding the flag back with the same type of logic, and with this - cases where hashes[suffix] \u003d\u003d None, the write_hashes would not be called.  In this case, some unit tests fail.  Is the reason because of other changes made - we now need to call write_hashes for even the case when hashes[suffix] \u003d\u003d None?","commit_id":"c8a628e75ad75141a5472f23820be9fe47e89308"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"1c2da30226ce85143daae4a62a86820087cdcd4f","unresolved":false,"context_lines":[{"line_number":1041,"context_line":"        hashes.pop(\u0027updated\u0027, None)"},{"line_number":1042,"context_line":"        return hashed, hashes"},{"line_number":1043,"context_line":""},{"line_number":1044,"context_line":"    def __get_hashes(self, partition_path, recalculate\u003dNone, do_listdir\u003dFalse):"},{"line_number":1045,"context_line":"        \"\"\""},{"line_number":1046,"context_line":"        Get hashes for each suffix dir in a partition.  do_listdir causes it to"},{"line_number":1047,"context_line":"        mistrust the hash cache for suffix existence at the (unexpectedly high)"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a461143_c8e15b00","line":1044,"updated":"2017-01-27 06:46:19.000000000","message":"I felt like we just didn\u0027t have enough functions called \"get_hashes\"\n\nYou mean the \"get_hashes\" function?  No the \"get_hashes\" *method*.  Oh the one REPLICATE calls?  No the one that *that* wraps in a tpool.  Oh the one that calls conslidate_hashes and _hash_suffix?  No the one in between there.  Oh... yeah - ^\u0026*$ing clayg did that.","commit_id":"c8a628e75ad75141a5472f23820be9fe47e89308"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3930ddcf0c97d919c7e018dd19e5119e87ae0cb","unresolved":false,"context_lines":[{"line_number":1041,"context_line":"        hashes.pop(\u0027updated\u0027, None)"},{"line_number":1042,"context_line":"        return hashed, hashes"},{"line_number":1043,"context_line":""},{"line_number":1044,"context_line":"    def __get_hashes(self, partition_path, recalculate\u003dNone, do_listdir\u003dFalse):"},{"line_number":1045,"context_line":"        \"\"\""},{"line_number":1046,"context_line":"        Get hashes for each suffix dir in a partition.  do_listdir causes it to"},{"line_number":1047,"context_line":"        mistrust the hash cache for suffix existence at the (unexpectedly high)"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a461143_6e84ddd3","line":1044,"in_reply_to":"3a461143_c8e15b00","updated":"2017-01-27 16:08:58.000000000","message":"I was wondering how we\u0027d clean up needing to pop updated before the multiple return points from _get_hashes. This works.\n\nIDK, _get_hashes_state ???","commit_id":"c8a628e75ad75141a5472f23820be9fe47e89308"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"58a4d6b8c8c2bbcf28f5aae616b821c37ecf2d66","unresolved":false,"context_lines":[{"line_number":1103,"context_line":"                modified \u003d True"},{"line_number":1104,"context_line":"        if modified:"},{"line_number":1105,"context_line":"            with lock_path(partition_path):"},{"line_number":1106,"context_line":"                if read_hashes(partition_path) \u003d\u003d orig_hashes:"},{"line_number":1107,"context_line":"                    write_hashes(partition_path, hashes)"},{"line_number":1108,"context_line":"                    return hashed, hashes"},{"line_number":1109,"context_line":"            return self._get_hashes(partition_path, recalculate, do_listdir)"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a461143_6e7a2ff3","line":1106,"updated":"2017-01-27 08:46:57.000000000","message":"Nice trick including the update key in the hashes - so this is only true if it\u0027s the same time.\n\nI\u0027m still wondering a bit about the state before this patch. If the hashes.pkl wasn\u0027t modified, but the invalidation file was added after consolidation but before this part- this would slip trough before and now? If the invalidation file is added, the hashes.pkl might not be updated - so do I miss something here or is there another possible race condition?","commit_id":"c8a628e75ad75141a5472f23820be9fe47e89308"},{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"82509af1327c2a657a531fb7f3c0a66b37c9fad0","unresolved":false,"context_lines":[{"line_number":1103,"context_line":"                modified \u003d True"},{"line_number":1104,"context_line":"        if modified:"},{"line_number":1105,"context_line":"            with lock_path(partition_path):"},{"line_number":1106,"context_line":"                if read_hashes(partition_path) \u003d\u003d orig_hashes:"},{"line_number":1107,"context_line":"                    write_hashes(partition_path, hashes)"},{"line_number":1108,"context_line":"                    return hashed, hashes"},{"line_number":1109,"context_line":"            return self._get_hashes(partition_path, recalculate, do_listdir)"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a461143_fa1ecb61","line":1106,"in_reply_to":"3a461143_6e7a2ff3","updated":"2017-01-27 14:20:45.000000000","message":"But wouldn\u0027t the other process (which ever has triggered get_hashes) take care of it? I think it is ok if hashes.pkl by another process is not yet updated while current process has reached this point (line#1106). Whenever there is an invalidation and get_hashes is called, it would take care of reading the latest hashes.pkl.","commit_id":"c8a628e75ad75141a5472f23820be9fe47e89308"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3930ddcf0c97d919c7e018dd19e5119e87ae0cb","unresolved":false,"context_lines":[{"line_number":1103,"context_line":"                modified \u003d True"},{"line_number":1104,"context_line":"        if modified:"},{"line_number":1105,"context_line":"            with lock_path(partition_path):"},{"line_number":1106,"context_line":"                if read_hashes(partition_path) \u003d\u003d orig_hashes:"},{"line_number":1107,"context_line":"                    write_hashes(partition_path, hashes)"},{"line_number":1108,"context_line":"                    return hashed, hashes"},{"line_number":1109,"context_line":"            return self._get_hashes(partition_path, recalculate, do_listdir)"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a461143_6e369d53","line":1106,"in_reply_to":"3a461143_6e7a2ff3","updated":"2017-01-27 16:08:58.000000000","message":"I think it is safe...\n\nAssume process X consolidated hashes and it\u0027s orig_hashes updated time is t1.\n\nIf invalidate_hash is called in process Y between process X consolidating hashes and reaching this point, then the invalidated suffix will remain in hashes.invalid until the next get_hashes. We don\u0027t yet have the up to date suffix hash state in hashes.pkl, but we have not lost the record of the suffix invalidation from hashes.invalid.\n\nIf invalidate_hash is called in process Y between process X consolidating hashes and reaching this point, AND process Z then calls get_hashes and consolidates the invalidated suffix into a new hashes.pkl (under lock_path) writing updated\u003dt2, then we now notice that when process X reached this point because the updated time t2 from read_hashes !\u003d t1 in orig_hashes. Process X will then re-enter __get_hashes and try again.\n\nProcess Z may succeed in re-hashing suffixes and writing the latest valid state to hashes.pkl at t3, in which case process X will find no invalid suffixes in hashes.pkl. \n\nOr process X may re-enter __get_hashes, read the hashes.pkl written at t2 before process Y complete re-hashing, and process X then proceeds to re-hash as well. Process X and Y will now be re-hashing the same set of invalidated suffixes. Whichever gets to this line first will write valid suffixes with fresh updated time. The other process will discover it lost the race and re-enter __get_hashes.","commit_id":"c8a628e75ad75141a5472f23820be9fe47e89308"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b3930ddcf0c97d919c7e018dd19e5119e87ae0cb","unresolved":false,"context_lines":[{"line_number":1106,"context_line":"                if read_hashes(partition_path) \u003d\u003d orig_hashes:"},{"line_number":1107,"context_line":"                    write_hashes(partition_path, hashes)"},{"line_number":1108,"context_line":"                    return hashed, hashes"},{"line_number":1109,"context_line":"            return self._get_hashes(partition_path, recalculate, do_listdir)"},{"line_number":1110,"context_line":"        else:"},{"line_number":1111,"context_line":"            return hashed, hashes"},{"line_number":1112,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"3a461143_634eb697","line":1109,"updated":"2017-01-27 16:08:58.000000000","message":"we can re-enter self.__get_hashes here, no need to go back out to _get_hashes. It makes no difference by may save future us trying to figure out why we need to do the pop of updated again (when we don\u0027t).","commit_id":"c8a628e75ad75141a5472f23820be9fe47e89308"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"950dd6b2e9012f26f986d4d5f407ca2ef99cb58e","unresolved":false,"context_lines":[{"line_number":1044,"context_line":"            hashes \u003d self.consolidate_hashes(partition_path)"},{"line_number":1045,"context_line":"        except Exception:"},{"line_number":1046,"context_line":"            self.logger.warning(\u0027Unable to read %r\u0027, hashes_file,"},{"line_number":1047,"context_line":"                                exc_info\u003dTrue)"},{"line_number":1048,"context_line":"            do_listdir \u003d True"},{"line_number":1049,"context_line":"            force_rewrite \u003d True"},{"line_number":1050,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a461143_f01c1fc3","side":"PARENT","line":1047,"updated":"2017-01-27 19:55:53.000000000","message":"... and it gets caught down here.  Probably some test hits this condition and then does the right thing (treat all suffix as invalid/do_listdir - sort of on accident?)\n\nsigh... nope!\n\n\tdiff --git a/test/unit/obj/test_diskfile.py b/test/unit/obj/test_diskfile.py\n\tindex 1a2f6af..f7847a5 100644\n\t--- a/test/unit/obj/test_diskfile.py\n\t+++ b/test/unit/obj/test_diskfile.py\n\t@@ -6300,6 +6300,21 @@ class TestSuffixHashes(unittest.TestCase):\n\t\t def test_hash_invalidations_racing_get_hashes_same_suffix_existing(self):\n\t\t\t self._test_hash_invalidations_racing_get_hashes_same_suffix(True)\n\t \n\t+    def test_unpickle_error_with_invalidations(self):\n\t+        for policy in self.iter_policies():\n\t+            df_mgr \u003d self.df_router[policy]\n\t+            df \u003d df_mgr.get_diskfile(\u0027sda1\u0027, \u00270\u0027, \u0027a\u0027, \u0027c\u0027, \u0027o\u0027,\n\t+                                     policy\u003dpolicy)\n\t+            suffix \u003d os.path.basename(os.path.dirname(df._datadir))\n\t+            df.delete(self.ts())\n\t+            part_path \u003d os.path.join(self.devices, \u0027sda1\u0027,\n\t+                                     diskfile.get_data_dir(policy), \u00270\u0027)\n\t+            hashes_file \u003d os.path.join(part_path, diskfile.HASH_FILE)\n\t+            open(hashes_file, \u0027w\u0027)\n\t+            hashes \u003d df_mgr.get_hashes(\u0027sda1\u0027, \u00270\u0027, [], policy)\n\t+            self.assertIn(suffix, hashes)\n\t+            self.assertEqual([], df_mgr.logger.get_lines_for_level(\u0027warning\u0027))\n\t+\n\t\t def test_invalidate_hash_consolidation(self):\n\t\t\t def assert_consolidation(suffixes):\n\t\t\t\t # verify that suffixes are invalidated after consolidation","commit_id":"2914e0449311482c91ccd61fc0f335a0579afa33"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"46a06230a2c3eb852a5f57105db4fce1714dc2a9","unresolved":false,"context_lines":[{"line_number":298,"context_line":"        return None"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"    with lock_path(partition_dir):"},{"line_number":301,"context_line":"        hashes \u003d read_hashes(partition_dir)"},{"line_number":302,"context_line":""},{"line_number":303,"context_line":"        found_invalidation_entry \u003d False"},{"line_number":304,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a461143_172c4501","line":301,"updated":"2017-01-27 18:43:32.000000000","message":"read_hashes can return None. so we do not expect that given the check at line 288, but if something changed since then...line 317 will blow up because hashes is till None and write_hashes assumes it is a dict.\n\nShould we have \n\n  if hashes is None:\n      return None\n\nor some other conditions later around writing pickle?\n\n(I noticed this while trying to fix the other race [1] in lines 288-298 whereby has invalidations get wiped out when there is no hashes.pkl)\n\n[1] https://review.openstack.org/#/c/426336","commit_id":"767bcf1caaf7145655e9530af69cd7b0ac7d49b0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"46a06230a2c3eb852a5f57105db4fce1714dc2a9","unresolved":false,"context_lines":[{"line_number":306,"context_line":"                for line in inv_fh:"},{"line_number":307,"context_line":"                    found_invalidation_entry \u003d True"},{"line_number":308,"context_line":"                    suffix \u003d line.strip()"},{"line_number":309,"context_line":"                    if hashes is not None and \\"},{"line_number":310,"context_line":"                            hashes.get(suffix, \u0027\u0027) is not None:"},{"line_number":311,"context_line":"                        hashes[suffix] \u003d None"},{"line_number":312,"context_line":"        except (IOError, OSError) as e:"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a461143_5a10b401","line":309,"updated":"2017-01-27 18:43:32.000000000","message":"if hashes IS None then write_hashes is going to fail at line 316 because we just set found_invalidation_entry\u003dTrue\n\nDo we need to make this whole loop conditional on hashes!\u003dNone?","commit_id":"767bcf1caaf7145655e9530af69cd7b0ac7d49b0"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"950dd6b2e9012f26f986d4d5f407ca2ef99cb58e","unresolved":false,"context_lines":[{"line_number":314,"context_line":"                raise"},{"line_number":315,"context_line":""},{"line_number":316,"context_line":"        if found_invalidation_entry:"},{"line_number":317,"context_line":"            write_hashes(partition_dir, hashes)"},{"line_number":318,"context_line":""},{"line_number":319,"context_line":"        # Now that all the invalidations are reflected in hashes.pkl, it\u0027s"},{"line_number":320,"context_line":"        # safe to clear out the invalidations file."}],"source_content_type":"text/x-python","patch_set":9,"id":"3a461143_70102fa9","line":317,"updated":"2017-01-27 19:55:53.000000000","message":"yeah I agree, if we get say an unpickle error in read hashes (and there\u0027s an invalidations entry) - we probably get all the way down here and end up raising TypeError ... (see below)","commit_id":"767bcf1caaf7145655e9530af69cd7b0ac7d49b0"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ada1a6c651d2389f47b36216474dd8a04db6caf5","unresolved":false,"context_lines":[{"line_number":1044,"context_line":"            hashes \u003d self.consolidate_hashes(partition_path)"},{"line_number":1045,"context_line":"        except Exception:"},{"line_number":1046,"context_line":"            self.logger.warning(\u0027Unable to read %r\u0027, hashes_file,"},{"line_number":1047,"context_line":"                                exc_info\u003dTrue)"},{"line_number":1048,"context_line":"            do_listdir \u003d True"},{"line_number":1049,"context_line":"            force_rewrite \u003d True"},{"line_number":1050,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a461143_a5dfb467","side":"PARENT","line":1047,"updated":"2017-01-27 23:05:40.000000000","message":"Still convinced this is just more and more pain - can\u0027t wait to let these errors bubble out!","commit_id":"2914e0449311482c91ccd61fc0f335a0579afa33"},{"author":{"_account_id":23449,"name":"Pavel Kvasnička","email":"pavel.kvasnicka@firma.seznam.cz","username":"PavelK"},"change_message_id":"67c85567988b615adc291862fe0d7526922579b8","unresolved":false,"context_lines":[{"line_number":1044,"context_line":"            hashes \u003d self.consolidate_hashes(partition_path)"},{"line_number":1045,"context_line":"        except Exception:"},{"line_number":1046,"context_line":"            self.logger.warning(\u0027Unable to read %r\u0027, hashes_file,"},{"line_number":1047,"context_line":"                                exc_info\u003dTrue)"},{"line_number":1048,"context_line":"            do_listdir \u003d True"},{"line_number":1049,"context_line":"            force_rewrite \u003d True"},{"line_number":1050,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a461143_fa7e9c17","side":"PARENT","line":1047,"in_reply_to":"3a461143_a5dfb467","updated":"2017-01-30 09:27:57.000000000","message":"Lets buble exception out except pickle parsing errors. If hashes.pkl is invalid (IDK when it may happen) and there is no disk error then we should repair hashes.","commit_id":"2914e0449311482c91ccd61fc0f335a0579afa33"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ada1a6c651d2389f47b36216474dd8a04db6caf5","unresolved":false,"context_lines":[{"line_number":268,"context_line":"    The updated key is added to hashes before it is written."},{"line_number":269,"context_line":"    \"\"\""},{"line_number":270,"context_line":"    hashes_file \u003d join(partition_dir, HASH_FILE)"},{"line_number":271,"context_line":"    hashes \u003d hashes or {}"},{"line_number":272,"context_line":"    hashes[\u0027updated\u0027] \u003d time.time()"},{"line_number":273,"context_line":"    return write_pickle(hashes, hashes_file, partition_dir, PICKLE_PROTOCOL)"},{"line_number":274,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"3a461143_855b98f5","line":271,"updated":"2017-01-27 23:05:40.000000000","message":"this is the big change - if you try to write None you get {\u0027updated\u0027: time.time()}\n\nSo if you\u0027re not careful to read back you\u0027ll end up double hashing again :\u0027(\n\nOTOH if we just *return* the {\u0027updated\u0027: time.time()} dict we have to find a new condition by which we can trigger \u0027do_listdir\u0027\n\nhrmm... maybe I just convinced myself there\u0027s still a bug here :\\","commit_id":"14c2d5713bb6a73c6b199e1a121b051d47308c9e"},{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"3468042f4320b42e14f00fd431116e9ac07db1b4","unresolved":false,"context_lines":[{"line_number":268,"context_line":"    The updated key is added to hashes before it is written."},{"line_number":269,"context_line":"    \"\"\""},{"line_number":270,"context_line":"    hashes_file \u003d join(partition_dir, HASH_FILE)"},{"line_number":271,"context_line":"    hashes \u003d hashes or {}"},{"line_number":272,"context_line":"    hashes[\u0027updated\u0027] \u003d time.time()"},{"line_number":273,"context_line":"    return write_pickle(hashes, hashes_file, partition_dir, PICKLE_PROTOCOL)"},{"line_number":274,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"3a461143_7a53856b","line":271,"in_reply_to":"3a461143_855b98f5","updated":"2017-01-30 13:49:16.000000000","message":"A couple of things:\n\n1) I\u0027m not sure if this will still be necessary if we have a check at line#303 for hashes!\u003dNone. We already do hashes \u003d{}at line#1074 and on any error read_hashes *will* return None, so maybe this line is not needed with the additional check below on line#303?\n\n2) Why even write a pickle when the hashes are None? Clearly I think I\u0027m missing something","commit_id":"14c2d5713bb6a73c6b199e1a121b051d47308c9e"},{"author":{"_account_id":23449,"name":"Pavel Kvasnička","email":"pavel.kvasnicka@firma.seznam.cz","username":"PavelK"},"change_message_id":"67c85567988b615adc291862fe0d7526922579b8","unresolved":false,"context_lines":[{"line_number":268,"context_line":"    The updated key is added to hashes before it is written."},{"line_number":269,"context_line":"    \"\"\""},{"line_number":270,"context_line":"    hashes_file \u003d join(partition_dir, HASH_FILE)"},{"line_number":271,"context_line":"    hashes \u003d hashes or {}"},{"line_number":272,"context_line":"    hashes[\u0027updated\u0027] \u003d time.time()"},{"line_number":273,"context_line":"    return write_pickle(hashes, hashes_file, partition_dir, PICKLE_PROTOCOL)"},{"line_number":274,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"3a461143_e00b4ed4","line":271,"in_reply_to":"3a461143_855b98f5","updated":"2017-01-30 09:27:57.000000000","message":"You are right - it is necessary to trigger \"do_listdir\" then hashes was None.\n\nIf there are partitions with no hashes then I prefer to store \"hashes\" in separate key and do not listing empty partitions every REPLICATE call. But there are usually not empty partitions, so I prefer to just add \"updated\" key.\n\nDo it as feeling better.","commit_id":"14c2d5713bb6a73c6b199e1a121b051d47308c9e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"bf2ab9477c66d7d0c5835c1ec97997d1482de802","unresolved":false,"context_lines":[{"line_number":268,"context_line":"    The updated key is added to hashes before it is written."},{"line_number":269,"context_line":"    \"\"\""},{"line_number":270,"context_line":"    hashes_file \u003d join(partition_dir, HASH_FILE)"},{"line_number":271,"context_line":"    hashes \u003d hashes or {}"},{"line_number":272,"context_line":"    hashes[\u0027updated\u0027] \u003d time.time()"},{"line_number":273,"context_line":"    return write_pickle(hashes, hashes_file, partition_dir, PICKLE_PROTOCOL)"},{"line_number":274,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"3a461143_6612040b","line":271,"in_reply_to":"3a461143_b55dfe16","updated":"2017-01-30 18:13:42.000000000","message":"BTW, the issue is exposed by this test [1] in my gist which generates a hashes.pkl with empty {} and suffixes don\u0027t get hashed\n\n[1] https://gist.github.com/alistairncoles/648c0d8e7f35f1bc4d5cf8994d8a9ce0#file-gist-diff-L120","commit_id":"14c2d5713bb6a73c6b199e1a121b051d47308c9e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"236713327b38272b6c48aa3360011c09ca84eb79","unresolved":false,"context_lines":[{"line_number":268,"context_line":"    The updated key is added to hashes before it is written."},{"line_number":269,"context_line":"    \"\"\""},{"line_number":270,"context_line":"    hashes_file \u003d join(partition_dir, HASH_FILE)"},{"line_number":271,"context_line":"    hashes \u003d hashes or {}"},{"line_number":272,"context_line":"    hashes[\u0027updated\u0027] \u003d time.time()"},{"line_number":273,"context_line":"    return write_pickle(hashes, hashes_file, partition_dir, PICKLE_PROTOCOL)"},{"line_number":274,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"3a461143_b55dfe16","line":271,"in_reply_to":"3a461143_e00b4ed4","updated":"2017-01-30 18:11:33.000000000","message":"I don\u0027t think this is safe: if hashes is None we commit to disk an empty dict that is same result as a hashed empty partition; then we rely on passing None back to __get_hashes to trigger the suffix hashing and write another pkl with populated hashes dict. But if that fails then we have left incorrect state on disk - any subsequent call to get_hashes will find hashes\u003d{} in the hashes.pkl and do nothing.\n\nWe need the on disk state committed when hashes is None to represent to subsequent readers that do_listdir is needed.","commit_id":"14c2d5713bb6a73c6b199e1a121b051d47308c9e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ada1a6c651d2389f47b36216474dd8a04db6caf5","unresolved":false,"context_lines":[{"line_number":270,"context_line":"    hashes_file \u003d join(partition_dir, HASH_FILE)"},{"line_number":271,"context_line":"    hashes \u003d hashes or {}"},{"line_number":272,"context_line":"    hashes[\u0027updated\u0027] \u003d time.time()"},{"line_number":273,"context_line":"    return write_pickle(hashes, hashes_file, partition_dir, PICKLE_PROTOCOL)"},{"line_number":274,"context_line":""},{"line_number":275,"context_line":""},{"line_number":276,"context_line":"def consolidate_hashes(partition_dir):"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a461143_a5a5f4a2","line":273,"updated":"2017-01-27 23:05:40.000000000","message":"I\u0027m milling over if I\u0027m going to try writing\n\n    {\u0027updated\u0027: time.time(), \u0027hashes\u0027: hashes}\n\nand let hashes \u003d\u003d None sometimes","commit_id":"14c2d5713bb6a73c6b199e1a121b051d47308c9e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"236713327b38272b6c48aa3360011c09ca84eb79","unresolved":false,"context_lines":[{"line_number":270,"context_line":"    hashes_file \u003d join(partition_dir, HASH_FILE)"},{"line_number":271,"context_line":"    hashes \u003d hashes or {}"},{"line_number":272,"context_line":"    hashes[\u0027updated\u0027] \u003d time.time()"},{"line_number":273,"context_line":"    return write_pickle(hashes, hashes_file, partition_dir, PICKLE_PROTOCOL)"},{"line_number":274,"context_line":""},{"line_number":275,"context_line":""},{"line_number":276,"context_line":"def consolidate_hashes(partition_dir):"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a461143_1507b218","line":273,"range":{"start_line":273,"start_character":4,"end_line":273,"end_character":10},"updated":"2017-01-30 18:11:33.000000000","message":"nit: the return here is unnecessary - write_pickle has not explicit return value","commit_id":"14c2d5713bb6a73c6b199e1a121b051d47308c9e"},{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"3468042f4320b42e14f00fd431116e9ac07db1b4","unresolved":false,"context_lines":[{"line_number":299,"context_line":"        return None"},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"    with lock_path(partition_dir):"},{"line_number":302,"context_line":"        hashes \u003d read_hashes(partition_dir)"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"        found_invalidation_entry \u003d False"},{"line_number":305,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a461143_f5c226e8","line":302,"updated":"2017-01-30 13:49:16.000000000","message":"why don\u0027t we have a check like \"if hashes is not None\" at line#303? Am I missing something?","commit_id":"14c2d5713bb6a73c6b199e1a121b051d47308c9e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"236713327b38272b6c48aa3360011c09ca84eb79","unresolved":false,"context_lines":[{"line_number":299,"context_line":"        return None"},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"    with lock_path(partition_dir):"},{"line_number":302,"context_line":"        hashes \u003d read_hashes(partition_dir)"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"        found_invalidation_entry \u003d False"},{"line_number":305,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a461143_f515bce4","line":302,"in_reply_to":"3a461143_f5c226e8","updated":"2017-01-30 18:11:33.000000000","message":"we still want to go ahead an truncate the hashes.invalid if hashes is None, because we are going to populate hashes.pkl in get_hashes and don\u0027t want stale invalidations left on disk","commit_id":"14c2d5713bb6a73c6b199e1a121b051d47308c9e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"236713327b38272b6c48aa3360011c09ca84eb79","unresolved":false,"context_lines":[{"line_number":307,"context_line":"                for line in inv_fh:"},{"line_number":308,"context_line":"                    found_invalidation_entry \u003d True"},{"line_number":309,"context_line":"                    suffix \u003d line.strip()"},{"line_number":310,"context_line":"                    if hashes is not None and \\"},{"line_number":311,"context_line":"                            hashes.get(suffix, \u0027\u0027) is not None:"},{"line_number":312,"context_line":"                        hashes[suffix] \u003d None"},{"line_number":313,"context_line":"        except (IOError, OSError) as e:"},{"line_number":314,"context_line":"            if e.errno !\u003d errno.ENOENT:"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a461143_35a8248d","line":311,"range":{"start_line":310,"start_character":42,"end_line":311,"end_character":62},"updated":"2017-01-30 18:11:33.000000000","message":"nit: we don\u0027t need this condition anymore - before it gated the \"modified\" flag but now we always write hashes.pkl if we found an invalidation so there\u0027s no hard just updating hashes even if we are replacing None with None","commit_id":"14c2d5713bb6a73c6b199e1a121b051d47308c9e"},{"author":{"_account_id":23449,"name":"Pavel Kvasnička","email":"pavel.kvasnicka@firma.seznam.cz","username":"PavelK"},"change_message_id":"67c85567988b615adc291862fe0d7526922579b8","unresolved":false,"context_lines":[{"line_number":314,"context_line":"            if e.errno !\u003d errno.ENOENT:"},{"line_number":315,"context_line":"                raise"},{"line_number":316,"context_line":""},{"line_number":317,"context_line":"        if found_invalidation_entry:"},{"line_number":318,"context_line":"            write_hashes(partition_dir, hashes)"},{"line_number":319,"context_line":""},{"line_number":320,"context_line":"        # Now that all the invalidations are reflected in hashes.pkl, it\u0027s"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a461143_5a4a50ad","line":317,"range":{"start_line":317,"start_character":11,"end_line":317,"end_character":35},"updated":"2017-01-30 09:27:57.000000000","message":"+1, thanks","commit_id":"14c2d5713bb6a73c6b199e1a121b051d47308c9e"},{"author":{"_account_id":23449,"name":"Pavel Kvasnička","email":"pavel.kvasnicka@firma.seznam.cz","username":"PavelK"},"change_message_id":"67c85567988b615adc291862fe0d7526922579b8","unresolved":false,"context_lines":[{"line_number":319,"context_line":""},{"line_number":320,"context_line":"        # Now that all the invalidations are reflected in hashes.pkl, it\u0027s"},{"line_number":321,"context_line":"        # safe to clear out the invalidations file."},{"line_number":322,"context_line":"        if found_invalidation_entry:"},{"line_number":323,"context_line":"            with open(invalidations_file, \u0027wb\u0027) as inv_fh:"},{"line_number":324,"context_line":"                pass"},{"line_number":325,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"3a461143_ba58948b","line":322,"range":{"start_line":322,"start_character":11,"end_line":322,"end_character":35},"updated":"2017-01-30 09:27:57.000000000","message":"this line is unnecessary","commit_id":"14c2d5713bb6a73c6b199e1a121b051d47308c9e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"236713327b38272b6c48aa3360011c09ca84eb79","unresolved":false,"context_lines":[{"line_number":319,"context_line":""},{"line_number":320,"context_line":"        # Now that all the invalidations are reflected in hashes.pkl, it\u0027s"},{"line_number":321,"context_line":"        # safe to clear out the invalidations file."},{"line_number":322,"context_line":"        if found_invalidation_entry:"},{"line_number":323,"context_line":"            with open(invalidations_file, \u0027wb\u0027) as inv_fh:"},{"line_number":324,"context_line":"                pass"},{"line_number":325,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"3a461143_947f28aa","line":322,"range":{"start_line":322,"start_character":11,"end_line":322,"end_character":35},"in_reply_to":"3a461143_ba58948b","updated":"2017-01-30 18:11:33.000000000","message":"right, but maintaining the indentation of following lines 323-324 :)","commit_id":"14c2d5713bb6a73c6b199e1a121b051d47308c9e"},{"author":{"_account_id":23449,"name":"Pavel Kvasnička","email":"pavel.kvasnicka@firma.seznam.cz","username":"PavelK"},"change_message_id":"67c85567988b615adc291862fe0d7526922579b8","unresolved":false,"context_lines":[{"line_number":1037,"context_line":"        \"\"\""},{"line_number":1038,"context_line":"        raise NotImplementedError"},{"line_number":1039,"context_line":""},{"line_number":1040,"context_line":"    def _get_hashes(self, *args, **kwargs):"},{"line_number":1041,"context_line":"        hashed, hashes \u003d self.__get_hashes(*args, **kwargs)"},{"line_number":1042,"context_line":"        hashes.pop(\u0027updated\u0027, None)"},{"line_number":1043,"context_line":"        return hashed, hashes"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a461143_3afbc448","line":1040,"range":{"start_line":1040,"start_character":8,"end_line":1040,"end_character":19},"updated":"2017-01-30 09:27:57.000000000","message":"Is it really necessary to cover __get_hashes method? It returns value only once on last line so why not to pop \u0027updated\u0027 key there?","commit_id":"14c2d5713bb6a73c6b199e1a121b051d47308c9e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"236713327b38272b6c48aa3360011c09ca84eb79","unresolved":false,"context_lines":[{"line_number":1037,"context_line":"        \"\"\""},{"line_number":1038,"context_line":"        raise NotImplementedError"},{"line_number":1039,"context_line":""},{"line_number":1040,"context_line":"    def _get_hashes(self, *args, **kwargs):"},{"line_number":1041,"context_line":"        hashed, hashes \u003d self.__get_hashes(*args, **kwargs)"},{"line_number":1042,"context_line":"        hashes.pop(\u0027updated\u0027, None)"},{"line_number":1043,"context_line":"        return hashed, hashes"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a461143_d42c5004","line":1040,"range":{"start_line":1040,"start_character":8,"end_line":1040,"end_character":19},"in_reply_to":"3a461143_3afbc448","updated":"2017-01-30 18:11:33.000000000","message":"__get_hashes returns at line 1111 and 1114. It might be simple to reduce that to one return however.","commit_id":"14c2d5713bb6a73c6b199e1a121b051d47308c9e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ada1a6c651d2389f47b36216474dd8a04db6caf5","unresolved":false,"context_lines":[{"line_number":1040,"context_line":"    def _get_hashes(self, *args, **kwargs):"},{"line_number":1041,"context_line":"        hashed, hashes \u003d self.__get_hashes(*args, **kwargs)"},{"line_number":1042,"context_line":"        hashes.pop(\u0027updated\u0027, None)"},{"line_number":1043,"context_line":"        return hashed, hashes"},{"line_number":1044,"context_line":""},{"line_number":1045,"context_line":"    def __get_hashes(self, partition_path, recalculate\u003dNone, do_listdir\u003dFalse):"},{"line_number":1046,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":10,"id":"3a461143_25d2e45c","line":1043,"updated":"2017-01-27 23:05:40.000000000","message":"This may turn out to be the sane place to convert an error in __get_hashes into a DiskError for the REPLICATE method!","commit_id":"14c2d5713bb6a73c6b199e1a121b051d47308c9e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ada1a6c651d2389f47b36216474dd8a04db6caf5","unresolved":false,"context_lines":[{"line_number":1082,"context_line":"            # conditions - so try not to get overly caught up trying to"},{"line_number":1083,"context_line":"            # optimize it out unless you manage to convince yourself there\u0027s a"},{"line_number":1084,"context_line":"            # bad behavior."},{"line_number":1085,"context_line":"            orig_hashes \u003d read_hashes(partition_path)"},{"line_number":1086,"context_line":"        else:"},{"line_number":1087,"context_line":"            hashes \u003d copy.deepcopy(orig_hashes)"},{"line_number":1088,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"3a461143_45f0d0fe","line":1085,"updated":"2017-01-27 23:05:40.000000000","message":"this might just be too subtle to be a good idea.  The tests will help it not regress though.  If we can drop the exception handling later we don\u0027t have to deal with the initialization condition of hashes \u003d None so the conditional here could look much more sane.","commit_id":"14c2d5713bb6a73c6b199e1a121b051d47308c9e"},{"author":{"_account_id":23449,"name":"Pavel Kvasnička","email":"pavel.kvasnicka@firma.seznam.cz","username":"PavelK"},"change_message_id":"67c85567988b615adc291862fe0d7526922579b8","unresolved":false,"context_lines":[{"line_number":1082,"context_line":"            # conditions - so try not to get overly caught up trying to"},{"line_number":1083,"context_line":"            # optimize it out unless you manage to convince yourself there\u0027s a"},{"line_number":1084,"context_line":"            # bad behavior."},{"line_number":1085,"context_line":"            orig_hashes \u003d read_hashes(partition_path)"},{"line_number":1086,"context_line":"        else:"},{"line_number":1087,"context_line":"            hashes \u003d copy.deepcopy(orig_hashes)"},{"line_number":1088,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"3a461143_ba8174c5","line":1085,"in_reply_to":"3a461143_45f0d0fe","updated":"2017-01-30 09:27:57.000000000","message":"Good idea. If consolidate_hashes doesn\u0027t hide errors and returns None only if read_hashes returned None then this condition is unnecessary.\n\n\nI think that consolidate_hashes doesn\u0027t need change to delete this condition. Am I wrong?","commit_id":"14c2d5713bb6a73c6b199e1a121b051d47308c9e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a211e4dd262246787d9adc65b7ccd2fe8813ea70","unresolved":false,"context_lines":[{"line_number":238,"context_line":"    Read the existing hashes.pkl"},{"line_number":239,"context_line":""},{"line_number":240,"context_line":"    :returns: a dict, the suffix hashes (if any), the key \u0027valid\u0027 will be False"},{"line_number":241,"context_line":"              if hashes.pkl is corrupt, cannot be read or does not exist"},{"line_number":242,"context_line":"    \"\"\""},{"line_number":243,"context_line":"    hashes_file \u003d join(partition_dir, HASH_FILE)"},{"line_number":244,"context_line":"    hashes \u003d {\u0027valid\u0027: False}"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a461143_af958a33","line":241,"updated":"2017-01-31 13:51:38.000000000","message":"or if hashes.pkl has not yet been written with valid suffix hashes.\n\ni.e. after first consolidate_hashes on a new part, before get_hashes completes, read_hashes can return a hashes dict with valid\u003dFalse","commit_id":"a6ee421ab5c84bf2a5076ad4deec8881d7cc34c6"},{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"6b32522e5eccb017d2f86247fdca7e6c52781ec9","unresolved":false,"context_lines":[{"line_number":238,"context_line":"    Read the existing hashes.pkl"},{"line_number":239,"context_line":""},{"line_number":240,"context_line":"    :returns: a dict, the suffix hashes (if any), the key \u0027valid\u0027 will be False"},{"line_number":241,"context_line":"              if hashes.pkl is corrupt, cannot be read or does not exist"},{"line_number":242,"context_line":"    \"\"\""},{"line_number":243,"context_line":"    hashes_file \u003d join(partition_dir, HASH_FILE)"},{"line_number":244,"context_line":"    hashes \u003d {\u0027valid\u0027: False}"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a461143_ca17f6a0","line":241,"in_reply_to":"3a461143_af958a33","updated":"2017-02-01 12:03:03.000000000","message":"yes. If there\u0027s gonna be another patchset, adding this line would be good.\n\nI think the rest of the comments in this file are pretty good at explaining the new changes.","commit_id":"a6ee421ab5c84bf2a5076ad4deec8881d7cc34c6"},{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"6b32522e5eccb017d2f86247fdca7e6c52781ec9","unresolved":false,"context_lines":[{"line_number":257,"context_line":"            pass"},{"line_number":258,"context_line":"    # hashes.pkl w/o valid updated key is \"valid\" but \"forever old\""},{"line_number":259,"context_line":"    hashes.setdefault(\u0027valid\u0027, True)"},{"line_number":260,"context_line":"    hashes.setdefault(\u0027updated\u0027, -1)"},{"line_number":261,"context_line":"    return hashes"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"3a461143_254eeb3a","line":260,"updated":"2017-02-01 12:03:03.000000000","message":"+1 Tests fail tampering with any of these hashes values","commit_id":"a6ee421ab5c84bf2a5076ad4deec8881d7cc34c6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a211e4dd262246787d9adc65b7ccd2fe8813ea70","unresolved":false,"context_lines":[{"line_number":270,"context_line":"    hashes_file \u003d join(partition_dir, HASH_FILE)"},{"line_number":271,"context_line":"    # \u0027valid\u0027 key should always be set by the caller; however, if there\u0027s a bug"},{"line_number":272,"context_line":"    # setting invalid is most safe"},{"line_number":273,"context_line":"    hashes.setdefault(\u0027valid\u0027, False)"},{"line_number":274,"context_line":"    hashes[\u0027updated\u0027] \u003d time.time()"},{"line_number":275,"context_line":"    write_pickle(hashes, hashes_file, partition_dir, PICKLE_PROTOCOL)"},{"line_number":276,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"3a461143_b1d08fca","line":273,"updated":"2017-01-31 13:51:38.000000000","message":"ok. this seems to be sufficient for:\n\n  (a) every write will have unique updated time and\n  (b) every write will differentiate between valid or invalid hashes\n\n(I was questioning overnight why I needed created\u003dtime.time() in my gist - the unique updated time should have been sufficient to differentiate two invalid hashes written in different processes.)","commit_id":"a6ee421ab5c84bf2a5076ad4deec8881d7cc34c6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a211e4dd262246787d9adc65b7ccd2fe8813ea70","unresolved":false,"context_lines":[{"line_number":309,"context_line":"                for line in inv_fh:"},{"line_number":310,"context_line":"                    found_invalidation_entry \u003d True"},{"line_number":311,"context_line":"                    suffix \u003d line.strip()"},{"line_number":312,"context_line":"                    hashes[suffix] \u003d None"},{"line_number":313,"context_line":"        except (IOError, OSError) as e:"},{"line_number":314,"context_line":"            if e.errno !\u003d errno.ENOENT:"},{"line_number":315,"context_line":"                raise"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a461143_1453116c","line":312,"updated":"2017-01-31 13:51:38.000000000","message":"ok. so even if hashes is \"invalid\" we apply invalidations to the dict. that\u0027s fine. we are going to do listdir anyway. Almost means that if an invalidation has occurred while we are doing listdir and another process consolidates, then it will be recorded in hashes.pkl addition to a unique updated time.","commit_id":"a6ee421ab5c84bf2a5076ad4deec8881d7cc34c6"},{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"6b32522e5eccb017d2f86247fdca7e6c52781ec9","unresolved":false,"context_lines":[{"line_number":309,"context_line":"                for line in inv_fh:"},{"line_number":310,"context_line":"                    found_invalidation_entry \u003d True"},{"line_number":311,"context_line":"                    suffix \u003d line.strip()"},{"line_number":312,"context_line":"                    hashes[suffix] \u003d None"},{"line_number":313,"context_line":"        except (IOError, OSError) as e:"},{"line_number":314,"context_line":"            if e.errno !\u003d errno.ENOENT:"},{"line_number":315,"context_line":"                raise"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a461143_e5f14302","line":312,"in_reply_to":"3a461143_1453116c","updated":"2017-02-01 12:03:03.000000000","message":"Also the invalidation state in current read shouldn\u0027t be lost I believe. \nnit: Maybe a small comment explaining why we do this regardless of the value of \"valid\" in hashes dict?","commit_id":"a6ee421ab5c84bf2a5076ad4deec8881d7cc34c6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a211e4dd262246787d9adc65b7ccd2fe8813ea70","unresolved":false,"context_lines":[{"line_number":314,"context_line":"            if e.errno !\u003d errno.ENOENT:"},{"line_number":315,"context_line":"                raise"},{"line_number":316,"context_line":""},{"line_number":317,"context_line":"        if found_invalidation_entry:"},{"line_number":318,"context_line":"            write_hashes(partition_dir, hashes)"},{"line_number":319,"context_line":"            # Now that all the invalidations are reflected in hashes.pkl, it\u0027s"},{"line_number":320,"context_line":"            # safe to clear out the invalidations file."}],"source_content_type":"text/x-python","patch_set":11,"id":"3a461143_7aa19745","line":317,"updated":"2017-01-31 13:51:38.000000000","message":"for some reason I was expecting we would write_hashes if hashes is not valid, but I guess that is not necessary IFF we are not truncating the invalids journal.","commit_id":"a6ee421ab5c84bf2a5076ad4deec8881d7cc34c6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a211e4dd262246787d9adc65b7ccd2fe8813ea70","unresolved":false,"context_lines":[{"line_number":1038,"context_line":"    def _get_hashes(self, *args, **kwargs):"},{"line_number":1039,"context_line":"        hashed, hashes \u003d self.__get_hashes(*args, **kwargs)"},{"line_number":1040,"context_line":"        hashes.pop(\u0027updated\u0027, None)"},{"line_number":1041,"context_line":"        hashes.pop(\u0027valid\u0027, None)"},{"line_number":1042,"context_line":"        return hashed, hashes"},{"line_number":1043,"context_line":""},{"line_number":1044,"context_line":"    def __get_hashes(self, partition_path, recalculate\u003dNone, do_listdir\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a461143_0a068c51","line":1041,"updated":"2017-01-31 13:51:38.000000000","message":"ok. tests DO fail if either of lines 1040 or 1041 are removed","commit_id":"a6ee421ab5c84bf2a5076ad4deec8881d7cc34c6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a211e4dd262246787d9adc65b7ccd2fe8813ea70","unresolved":false,"context_lines":[{"line_number":1058,"context_line":"        hashed \u003d 0"},{"line_number":1059,"context_line":"        hashes_file \u003d join(partition_path, HASH_FILE)"},{"line_number":1060,"context_line":"        modified \u003d False"},{"line_number":1061,"context_line":"        orig_hashes \u003d {\u0027valid\u0027: False}"},{"line_number":1062,"context_line":""},{"line_number":1063,"context_line":"        if recalculate is None:"},{"line_number":1064,"context_line":"            recalculate \u003d []"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a461143_c51f1559","line":1061,"updated":"2017-01-31 13:51:38.000000000","message":"nit: orig_hashes \u003d None is sufficient.\n\nah, but this makes sense in context of follow on patch https://review.openstack.org/#/c/426336/4/swift/obj/diskfile.py","commit_id":"a6ee421ab5c84bf2a5076ad4deec8881d7cc34c6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a211e4dd262246787d9adc65b7ccd2fe8813ea70","unresolved":false,"context_lines":[{"line_number":1071,"context_line":""},{"line_number":1072,"context_line":"        if orig_hashes is None:"},{"line_number":1073,"context_line":"            # consolidate_hashes returns None if hashes.pkl does not exist"},{"line_number":1074,"context_line":"            orig_hashes \u003d {\u0027valid\u0027: False}"},{"line_number":1075,"context_line":"        if not orig_hashes[\u0027valid\u0027]:"},{"line_number":1076,"context_line":"            # This is the only path to a valid hashes from invalid read (e.g."},{"line_number":1077,"context_line":"            # does not exist, corrupt, etc.).  Moreover, in order to write this"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a461143_15ece428","line":1074,"updated":"2017-01-31 13:51:38.000000000","message":"ok. this condition goes away in follow on patch https://review.openstack.org/#/c/426336/4/swift/obj/diskfile.py","commit_id":"a6ee421ab5c84bf2a5076ad4deec8881d7cc34c6"},{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"6b32522e5eccb017d2f86247fdca7e6c52781ec9","unresolved":false,"context_lines":[{"line_number":1071,"context_line":""},{"line_number":1072,"context_line":"        if orig_hashes is None:"},{"line_number":1073,"context_line":"            # consolidate_hashes returns None if hashes.pkl does not exist"},{"line_number":1074,"context_line":"            orig_hashes \u003d {\u0027valid\u0027: False}"},{"line_number":1075,"context_line":"        if not orig_hashes[\u0027valid\u0027]:"},{"line_number":1076,"context_line":"            # This is the only path to a valid hashes from invalid read (e.g."},{"line_number":1077,"context_line":"            # does not exist, corrupt, etc.).  Moreover, in order to write this"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a461143_25d18b23","line":1074,"in_reply_to":"3a461143_15ece428","updated":"2017-02-01 12:03:03.000000000","message":"thanks for pointing out.","commit_id":"a6ee421ab5c84bf2a5076ad4deec8881d7cc34c6"}],"test/unit/obj/test_diskfile.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7dce70601daeafce03b199c3421e530d756e47e5","unresolved":false,"context_lines":[{"line_number":7259,"context_line":"                df, df_mgr, \u0027sda1\u0027, \u00270\u0027, \u0027a\u0027, \u0027c\u0027,"},{"line_number":7260,"context_line":"                policy\u003dpolicy, frag_index\u003d5)"},{"line_number":7261,"context_line":"            suffix2 \u003d os.path.basename(os.path.dirname(df2._datadir))"},{"line_number":7262,"context_line":"            # create a garbage invalid zero-byte file"},{"line_number":7263,"context_line":"            part_path \u003d os.path.join(df._datadir, \u0027../../\u0027)"},{"line_number":7264,"context_line":"            hashfile_path \u003d os.path.join(part_path, diskfile.HASH_FILE)"},{"line_number":7265,"context_line":"            hash_breaking_function(hashfile_path)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a3905b3_1cce4eb4","line":7262,"updated":"2017-01-20 12:55:08.000000000","message":"comment doesn\u0027t apply (only true for one of the test cases)","commit_id":"c518df1bce922cf68f2ccab1aa5a8a1b580eab88"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"be5b5f8f13fb4866c25145056f339c846ef2e34f","unresolved":false,"context_lines":[{"line_number":7259,"context_line":"                df, df_mgr, \u0027sda1\u0027, \u00270\u0027, \u0027a\u0027, \u0027c\u0027,"},{"line_number":7260,"context_line":"                policy\u003dpolicy, frag_index\u003d5)"},{"line_number":7261,"context_line":"            suffix2 \u003d os.path.basename(os.path.dirname(df2._datadir))"},{"line_number":7262,"context_line":"            # create a garbage invalid zero-byte file"},{"line_number":7263,"context_line":"            part_path \u003d os.path.join(df._datadir, \u0027../../\u0027)"},{"line_number":7264,"context_line":"            hashfile_path \u003d os.path.join(part_path, diskfile.HASH_FILE)"},{"line_number":7265,"context_line":"            hash_breaking_function(hashfile_path)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a3905b3_5c80298c","line":7262,"in_reply_to":"5a3905b3_1cce4eb4","updated":"2017-01-24 19:18:37.000000000","message":"Done","commit_id":"c518df1bce922cf68f2ccab1aa5a8a1b580eab88"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7dce70601daeafce03b199c3421e530d756e47e5","unresolved":false,"context_lines":[{"line_number":7283,"context_line":"            self.assertIn(suffix, hashes)"},{"line_number":7284,"context_line":"            self.assertIn(suffix2, hashes)"},{"line_number":7285,"context_line":""},{"line_number":7286,"context_line":"    def test_get_hashes_race_invalid_pickle(self):"},{"line_number":7287,"context_line":"        def hash_breaking_function(hashfile_path):"},{"line_number":7288,"context_line":"            # zero byte file can not unpickle"},{"line_number":7289,"context_line":"            open(hashfile_path, \u0027w\u0027).close()"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a3905b3_ab040a7e","line":7286,"updated":"2017-01-20 12:55:08.000000000","message":"interesting...run this against diskile.py from master and it fails:\n\nFailure\nTraceback (most recent call last):\n  File \"/home/anc/0dev/swift/test/unit/obj/test_diskfile.py\", line 7298, in test_get_hashes_race_invalid_pickle\n    self._test_get_hashes_race(hash_breaking_function)\n  File \"/home/anc/0dev/swift/test/unit/obj/test_diskfile.py\", line 7292, in _test_get_hashes_race\n    self.assertIn(suffix2, hashes)\nAssertionError: \u002723d\u0027 not found in {\u0027bbe\u0027: {None: \u002755f13bb9f263b4d2c43f9d8122913b5f\u0027}}\n\nbecause force_rewrite is indiscriminately forcing the first get_hashes to overwrite the better hashes.pkl that was written by the second get_hashes!","commit_id":"c518df1bce922cf68f2ccab1aa5a8a1b580eab88"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"be5b5f8f13fb4866c25145056f339c846ef2e34f","unresolved":false,"context_lines":[{"line_number":7283,"context_line":"            self.assertIn(suffix, hashes)"},{"line_number":7284,"context_line":"            self.assertIn(suffix2, hashes)"},{"line_number":7285,"context_line":""},{"line_number":7286,"context_line":"    def test_get_hashes_race_invalid_pickle(self):"},{"line_number":7287,"context_line":"        def hash_breaking_function(hashfile_path):"},{"line_number":7288,"context_line":"            # zero byte file can not unpickle"},{"line_number":7289,"context_line":"            open(hashfile_path, \u0027w\u0027).close()"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a3905b3_9c48d18c","line":7286,"in_reply_to":"5a3905b3_ab040a7e","updated":"2017-01-24 19:18:37.000000000","message":"Done","commit_id":"c518df1bce922cf68f2ccab1aa5a8a1b580eab88"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7dce70601daeafce03b199c3421e530d756e47e5","unresolved":false,"context_lines":[{"line_number":7291,"context_line":""},{"line_number":7292,"context_line":"    def test_get_hashes_race_new_partition(self):"},{"line_number":7293,"context_line":"        def hash_breaking_function(hashfile_path):"},{"line_number":7294,"context_line":"            # simulate rebalanced part doing post-rsync REPLICATE"},{"line_number":7295,"context_line":"            self.assertFalse(os.path.exists(hashfile_path))"},{"line_number":7296,"context_line":"        self._test_get_hashes_race(hash_breaking_function)"},{"line_number":7297,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"5a3905b3_2b8adabf","line":7294,"updated":"2017-01-20 12:55:08.000000000","message":"I think that once [1] lands this would not be strictly true because there would be a hashes.invalid file in the part_path due to the df.delete, whereas I think a rebalanced part would have neither hashes.pkl not hashes.invalid","commit_id":"c518df1bce922cf68f2ccab1aa5a8a1b580eab88"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"be5b5f8f13fb4866c25145056f339c846ef2e34f","unresolved":false,"context_lines":[{"line_number":7291,"context_line":""},{"line_number":7292,"context_line":"    def test_get_hashes_race_new_partition(self):"},{"line_number":7293,"context_line":"        def hash_breaking_function(hashfile_path):"},{"line_number":7294,"context_line":"            # simulate rebalanced part doing post-rsync REPLICATE"},{"line_number":7295,"context_line":"            self.assertFalse(os.path.exists(hashfile_path))"},{"line_number":7296,"context_line":"        self._test_get_hashes_race(hash_breaking_function)"},{"line_number":7297,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"5a3905b3_bc4d8d9b","line":7294,"in_reply_to":"5a3905b3_2b8adabf","updated":"2017-01-24 19:18:37.000000000","message":"Done","commit_id":"c518df1bce922cf68f2ccab1aa5a8a1b580eab88"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7dce70601daeafce03b199c3421e530d756e47e5","unresolved":false,"context_lines":[{"line_number":7294,"context_line":"            # simulate rebalanced part doing post-rsync REPLICATE"},{"line_number":7295,"context_line":"            self.assertFalse(os.path.exists(hashfile_path))"},{"line_number":7296,"context_line":"        self._test_get_hashes_race(hash_breaking_function)"},{"line_number":7297,"context_line":""},{"line_number":7298,"context_line":"    def test_get_hashes_hash_suffix_enotdir(self):"},{"line_number":7299,"context_line":"        for policy in self.iter_policies():"},{"line_number":7300,"context_line":"            df_mgr \u003d self.df_router[policy]"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a3905b3_8be10e9d","line":7297,"updated":"2017-01-20 12:55:08.000000000","message":"I played around with this to create a third test case \"test_get_hashes_race_new_partition\" which created an empty hashes.pkl in hash_breaking_function, but it\u0027s not straightforward because the hash_breaking_function is called after the df.delete and on the branch the df.delete does NOT create hashes.invalid (because there is no hashes.pkl) - it would work if rebased on https://review.openstack.org/#/c/418690/\n\nI ended up with this:\n\nhttps://gist.github.com/alistairncoles/263009d37cf58f6ff39248047fe2033d","commit_id":"c518df1bce922cf68f2ccab1aa5a8a1b580eab88"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"be5b5f8f13fb4866c25145056f339c846ef2e34f","unresolved":false,"context_lines":[{"line_number":7294,"context_line":"            # simulate rebalanced part doing post-rsync REPLICATE"},{"line_number":7295,"context_line":"            self.assertFalse(os.path.exists(hashfile_path))"},{"line_number":7296,"context_line":"        self._test_get_hashes_race(hash_breaking_function)"},{"line_number":7297,"context_line":""},{"line_number":7298,"context_line":"    def test_get_hashes_hash_suffix_enotdir(self):"},{"line_number":7299,"context_line":"        for policy in self.iter_policies():"},{"line_number":7300,"context_line":"            df_mgr \u003d self.df_router[policy]"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a3905b3_dc425968","line":7297,"in_reply_to":"5a3905b3_8be10e9d","updated":"2017-01-24 19:18:37.000000000","message":"Done","commit_id":"c518df1bce922cf68f2ccab1aa5a8a1b580eab88"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"2c2643b593d513f2be1f195f7f26bd84c7e9aad1","unresolved":false,"context_lines":[{"line_number":7451,"context_line":"            orig_hash_suffix \u003d df_mgr._hash_suffix"},{"line_number":7452,"context_line":""},{"line_number":7453,"context_line":"            # then create a suffix"},{"line_number":7454,"context_line":"            df.delete(self.ts())"},{"line_number":7455,"context_line":""},{"line_number":7456,"context_line":"            def mock_hash_suffix(*args, **kwargs):"},{"line_number":7457,"context_line":"                # capture first call to mock_hash"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a3905b3_479212a1","line":7454,"updated":"2017-01-24 19:25:29.000000000","message":"I validated that reordering the hashes.pkl tamper and the df.delete calls doesn\u0027t effect how the tests fail on master.\n\n\tubuntu@saio:~$ nosetests swift/test/unit/obj/test_diskfile.py\n\t.............................................................................................................................................................................................................................................................................................................................................................................................................F..........FF..................................\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_contains_hashes_of_existing_partition (test.unit.obj.test_diskfile.TestSuffixHashes)\n\t----------------------------------------------------------------------\n\tTraceback (most recent call last):\n\t  File \"/usr/local/lib/python2.7/dist-packages/mock/mock.py\", line 1305, in patched\n\t\treturn func(*args, **keywargs)\n\t  File \"/home/ubuntu/swift/test/unit/obj/test_diskfile.py\", line 6235, in test_contains_hashes_of_existing_partition\n\t\tself.assertIn(suffix2, hashes)\n\tAssertionError: \u00273c1\u0027 not found in {\u0027a83\u0027: \u0027a16adfc79f8a29b19d9bbbf493a89476\u0027}\n\t-------------------- \u003e\u003e begin captured stdout \u003c\u003c ---------------------\n\tsuffix-hash-test DEBUG: Run listdir on /tmp/tmpT6WPGV/node/sda1/objects/0\n\tsuffix-hash-test DEBUG: Run listdir on /tmp/tmpT6WPGV/node/sda1/objects-1/0\n\n\t--------------------- \u003e\u003e end captured stdout \u003c\u003c ----------------------\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_get_hashes_race_invalid_pickle (test.unit.obj.test_diskfile.TestSuffixHashes)\n\t----------------------------------------------------------------------\n\tTraceback (most recent call last):\n\t  File \"/home/ubuntu/swift/test/unit/obj/test_diskfile.py\", line 7477, in test_get_hashes_race_invalid_pickle\n\t\tself._test_get_hashes_race(hash_breaking_function)\n\t  File \"/home/ubuntu/swift/test/unit/obj/test_diskfile.py\", line 7471, in _test_get_hashes_race\n\t\tself.assertIn(suffix2, hashes)\n\tAssertionError: \u00273c1\u0027 not found in {\u0027a83\u0027: {None: \u0027d7c41463d67eaaa63af5e30c6af7eacb\u0027}}\n\t-------------------- \u003e\u003e begin captured stdout \u003c\u003c ---------------------\n\tsuffix-hash-test DEBUG: Run listdir on /tmp/tmp71Wzrc/node/sda1/objects/0\n\tsuffix-hash-test DEBUG: Run listdir on /tmp/tmp71Wzrc/node/sda1/objects/0\n\tsuffix-hash-test DEBUG: Run listdir on /tmp/tmp71Wzrc/node/sda1/objects/0\n\n\t--------------------- \u003e\u003e end captured stdout \u003c\u003c ----------------------\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_get_hashes_race_new_partition (test.unit.obj.test_diskfile.TestSuffixHashes)\n\t----------------------------------------------------------------------\n\tTraceback (most recent call last):\n\t  File \"/home/ubuntu/swift/test/unit/obj/test_diskfile.py\", line 7487, in test_get_hashes_race_new_partition\n\t\tself._test_get_hashes_race(hash_breaking_function)\n\t  File \"/home/ubuntu/swift/test/unit/obj/test_diskfile.py\", line 7471, in _test_get_hashes_race\n\t\tself.assertIn(suffix2, hashes)\n\tAssertionError: \u00273c1\u0027 not found in {\u0027a83\u0027: {None: \u0027d7c41463d67eaaa63af5e30c6af7eacb\u0027}}\n\t-------------------- \u003e\u003e begin captured stdout \u003c\u003c ---------------------\n\tsuffix-hash-test DEBUG: Run listdir on /tmp/tmpHpku3b/node/sda1/objects/0\n\tsuffix-hash-test DEBUG: Run listdir on /tmp/tmpHpku3b/node/sda1/objects/0\n\tsuffix-hash-test DEBUG: Run listdir on /tmp/tmpHpku3b/node/sda1/objects/0\n\n\t--------------------- \u003e\u003e end captured stdout \u003c\u003c ----------------------\n\n\t----------------------------------------------------------------------\n\tRan 444 tests in 2.674s\n\n\tFAILED (failures\u003d3)","commit_id":"63921b6a2959ac732962f39fe967016a15e4c221"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"2c2643b593d513f2be1f195f7f26bd84c7e9aad1","unresolved":false,"context_lines":[{"line_number":7483,"context_line":"            part_dir \u003d os.path.dirname(hashfile_path)"},{"line_number":7484,"context_line":"            os.unlink(os.path.join(part_dir, \u0027.lock\u0027))"},{"line_number":7485,"context_line":"            # sanity"},{"line_number":7486,"context_line":"            self.assertEqual([], os.listdir(os.path.dirname(hashfile_path)))"},{"line_number":7487,"context_line":"        self._test_get_hashes_race(hash_breaking_function)"},{"line_number":7488,"context_line":""},{"line_number":7489,"context_line":"    def test_get_hashes_race_existing_partition(self):"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a3905b3_473db2ba","line":7486,"updated":"2017-01-24 19:25:29.000000000","message":"I re-worded the sanity here slightly from what was in acoles gist\n\nhttps://gist.github.com/alistairncoles/263009d37cf58f6ff39248047fe2033d","commit_id":"63921b6a2959ac732962f39fe967016a15e4c221"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a7845dfedeaae705fc8888a71350e7c89f1ec72d","unresolved":false,"context_lines":[{"line_number":6194,"context_line":"        def mock_write_pickle_def(*args, **kwargs):"},{"line_number":6195,"context_line":"            if \u0027mtime\u0027 not in non_local:"},{"line_number":6196,"context_line":"                non_local[\u0027mtime\u0027] \u003d time()"},{"line_number":6197,"context_line":"            non_local[\u0027mtime\u0027] +\u003d 1"},{"line_number":6198,"context_line":"            write_pickle(*args, **kwargs)"},{"line_number":6199,"context_line":""},{"line_number":6200,"context_line":"        def mock_getmtime_def(filename):"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a3905b3_35ba826f","side":"PARENT","line":6197,"updated":"2017-01-25 02:23:25.000000000","message":"no!  why\u0027d i miss this!","commit_id":"63b351893d91f56d1f55bb42581268b6d83eaf24"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a7845dfedeaae705fc8888a71350e7c89f1ec72d","unresolved":false,"context_lines":[{"line_number":6233,"context_line":""},{"line_number":6234,"context_line":"            # suffix2 just sits in the invalidations file"},{"line_number":6235,"context_line":"            self.assertIn(suffix, hashes)"},{"line_number":6236,"context_line":"            self.assertNotIn(suffix2, hashes)"},{"line_number":6237,"context_line":""},{"line_number":6238,"context_line":"            # it\u0027ll show up next hash"},{"line_number":6239,"context_line":"            hashes \u003d df_mgr.get_hashes(\u0027sda1\u0027, \u00270\u0027, [], policy)"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a3905b3_d593fefe","line":6236,"updated":"2017-01-25 02:23:25.000000000","message":"because we don\u0027t double rehash ;)","commit_id":"26ff0f5edc9b604ee3f9eeadfaccb200b145e82d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4151341f3e786d8ec4c974eae6a7742991b08d98","unresolved":false,"context_lines":[{"line_number":6183,"context_line":"            self.assertIn(suffix2, hashes)"},{"line_number":6184,"context_line":""},{"line_number":6185,"context_line":"    def test_contains_hashes_of_existing_partition(self):"},{"line_number":6186,"context_line":"        # get_hashes must repeat path listing and return all hashes when"},{"line_number":6187,"context_line":"        # another concurrent process created new pkl before hashes are stored"},{"line_number":6188,"context_line":"        # by the first process"},{"line_number":6189,"context_line":"        non_local \u003d {}"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a461143_f3d8e9f1","line":6186,"updated":"2017-01-26 18:49:57.000000000","message":"this test seems to have evolved away from the original intent, this no longer tests re-entry on detection of concurrent modification","commit_id":"24276337fc1b0ac4bc91ce5838750c8a93af8420"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4f4bdd1418187ab1f05b0426bec003e37aac5354","unresolved":false,"context_lines":[{"line_number":6183,"context_line":"            self.assertIn(suffix2, hashes)"},{"line_number":6184,"context_line":""},{"line_number":6185,"context_line":"    def test_contains_hashes_of_existing_partition(self):"},{"line_number":6186,"context_line":"        # get_hashes must repeat path listing and return all hashes when"},{"line_number":6187,"context_line":"        # another concurrent process created new pkl before hashes are stored"},{"line_number":6188,"context_line":"        # by the first process"},{"line_number":6189,"context_line":"        non_local \u003d {}"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a461143_a2ba6ddf","line":6186,"in_reply_to":"3a461143_f0e9ad1b","updated":"2017-01-27 06:17:47.000000000","message":"Done","commit_id":"24276337fc1b0ac4bc91ce5838750c8a93af8420"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"36df8e4bc5810e6670ca9441bc96b80bba2d4032","unresolved":false,"context_lines":[{"line_number":6183,"context_line":"            self.assertIn(suffix2, hashes)"},{"line_number":6184,"context_line":""},{"line_number":6185,"context_line":"    def test_contains_hashes_of_existing_partition(self):"},{"line_number":6186,"context_line":"        # get_hashes must repeat path listing and return all hashes when"},{"line_number":6187,"context_line":"        # another concurrent process created new pkl before hashes are stored"},{"line_number":6188,"context_line":"        # by the first process"},{"line_number":6189,"context_line":"        non_local \u003d {}"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a461143_f0e9ad1b","line":6186,"in_reply_to":"3a461143_f3d8e9f1","updated":"2017-01-26 20:30:00.000000000","message":"well, I honestly have no idea what the name was supposed to mean originally.  Whatever was supposed to be there before - I think what\u0027s left over is demonstrating a correct and desirable behavior in a meaningful and straightforward way.\n\nThe mock_listdir invalidates a hash while hashing - at the end of the day that invalidation shouldn\u0027t get loss - I think this test demonstrates the desired behavior which is to allow the original hash to finish - but on subsequent hashes the invalidation is acknowledged.\n\n    test_invalidation_while_hashing_is_not_lost\n\n^ ???","commit_id":"24276337fc1b0ac4bc91ce5838750c8a93af8420"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4151341f3e786d8ec4c974eae6a7742991b08d98","unresolved":false,"context_lines":[{"line_number":6217,"context_line":""},{"line_number":6218,"context_line":"            with mock.patch(\u0027swift.obj.diskfile.os.listdir\u0027,"},{"line_number":6219,"context_line":"                            mock_listdir):"},{"line_number":6220,"context_line":"                # creates pkl file and repeats listing when pkl modified"},{"line_number":6221,"context_line":"                hashes \u003d df_mgr.get_hashes(\u0027sda1\u0027, \u00270\u0027, [], policy)"},{"line_number":6222,"context_line":""},{"line_number":6223,"context_line":"            # suffix2 just sits in the invalidations file"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a461143_1321f5fb","line":6220,"updated":"2017-01-26 18:49:57.000000000","message":"this is no longer true is it? the side effect of mock_listdir is now simply to delete df2 which appends to hashes.invalid but hashes.pkl is not touched so the - as noted at line 6223","commit_id":"24276337fc1b0ac4bc91ce5838750c8a93af8420"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4151341f3e786d8ec4c974eae6a7742991b08d98","unresolved":false,"context_lines":[{"line_number":7579,"context_line":"                      for i in range(non_local[\u0027suffix_count\u0027])}"},{"line_number":7580,"context_line":"                if len(calls) \u003c\u003d 3:"},{"line_number":7581,"context_line":"                    # this will make the *next* call get a slightly"},{"line_number":7582,"context_line":"                    # newer inode than the last"},{"line_number":7583,"context_line":"                    non_local[\u0027suffix_count\u0027] +\u003d 1"},{"line_number":7584,"context_line":"                # track exactly the value for every return"},{"line_number":7585,"context_line":"                calls.append(rv)"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a461143_f3f629d6","line":7582,"updated":"2017-01-26 18:49:57.000000000","message":"nit: comment needs updating","commit_id":"24276337fc1b0ac4bc91ce5838750c8a93af8420"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4f4bdd1418187ab1f05b0426bec003e37aac5354","unresolved":false,"context_lines":[{"line_number":7579,"context_line":"                      for i in range(non_local[\u0027suffix_count\u0027])}"},{"line_number":7580,"context_line":"                if len(calls) \u003c\u003d 3:"},{"line_number":7581,"context_line":"                    # this will make the *next* call get a slightly"},{"line_number":7582,"context_line":"                    # newer inode than the last"},{"line_number":7583,"context_line":"                    non_local[\u0027suffix_count\u0027] +\u003d 1"},{"line_number":7584,"context_line":"                # track exactly the value for every return"},{"line_number":7585,"context_line":"                calls.append(rv)"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a461143_428549ab","line":7582,"in_reply_to":"3a461143_50afe193","updated":"2017-01-27 06:17:47.000000000","message":"Done","commit_id":"24276337fc1b0ac4bc91ce5838750c8a93af8420"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"36df8e4bc5810e6670ca9441bc96b80bba2d4032","unresolved":false,"context_lines":[{"line_number":7579,"context_line":"                      for i in range(non_local[\u0027suffix_count\u0027])}"},{"line_number":7580,"context_line":"                if len(calls) \u003c\u003d 3:"},{"line_number":7581,"context_line":"                    # this will make the *next* call get a slightly"},{"line_number":7582,"context_line":"                    # newer inode than the last"},{"line_number":7583,"context_line":"                    non_local[\u0027suffix_count\u0027] +\u003d 1"},{"line_number":7584,"context_line":"                # track exactly the value for every return"},{"line_number":7585,"context_line":"                calls.append(rv)"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a461143_50afe193","line":7582,"in_reply_to":"3a461143_f3f629d6","updated":"2017-01-26 20:30:00.000000000","message":"this should *definitely* be fixed before merge IMHO\n\nI *really* don\u0027t like it when code lands that makes comments wrong/out-of-date - it\u0027s like *the* reason people say comments are bad (they are!)\n\nHere you spotted it?!  We have a chance to get it right!?  Why would this not be -1?  We should *definitely* fix it!  \n\n#willfix","commit_id":"24276337fc1b0ac4bc91ce5838750c8a93af8420"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"46a06230a2c3eb852a5f57105db4fce1714dc2a9","unresolved":false,"context_lines":[{"line_number":6184,"context_line":"            self.assertIn(suffix, hashes)"},{"line_number":6185,"context_line":"            self.assertIn(suffix2, hashes)"},{"line_number":6186,"context_line":""},{"line_number":6187,"context_line":"    def test_hash_invalidations_survive_racing_get_hashes_diff_suffix(self):"},{"line_number":6188,"context_line":"        # get_hashes must repeat path listing and return all hashes when"},{"line_number":6189,"context_line":"        # another concurrent process created new pkl before hashes are stored"},{"line_number":6190,"context_line":"        # by the first process"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a461143_c11cbf15","line":6187,"updated":"2017-01-27 18:43:32.000000000","message":"wait, the only substantial difference between this test and the one above is that hashes.pkl gets created here, so can\u0027t we combine them or at least give them similar names...\n\nhttps://gist.github.com/alistairncoles/c37603280696e7949ea5f277744313bc","commit_id":"767bcf1caaf7145655e9530af69cd7b0ac7d49b0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"46a06230a2c3eb852a5f57105db4fce1714dc2a9","unresolved":false,"context_lines":[{"line_number":6192,"context_line":"        for policy in self.iter_policies():"},{"line_number":6193,"context_line":"            df_mgr \u003d self.df_router[policy]"},{"line_number":6194,"context_line":"            # force hashes.pkl to exist; when it does not exist that\u0027s fine,"},{"line_number":6195,"context_line":"            # it\u0027s just a different race; in that case the invalidation file"},{"line_number":6196,"context_line":"            # gets appended, but we don\u0027t restart hashing suffixes (the"},{"line_number":6197,"context_line":"            # invalidation get\u0027s squashed in and the suffix gets rehashed on"},{"line_number":6198,"context_line":"            # the next REPLICATE call)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a461143_c4f5adcb","line":6195,"updated":"2017-01-27 18:43:32.000000000","message":"is it a different race? in both case hashes.invalid get appended (with recent change to invalidate_hash) and in this case (as well as that) the invalidation gets handled on next get_hashes call.","commit_id":"767bcf1caaf7145655e9530af69cd7b0ac7d49b0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"46a06230a2c3eb852a5f57105db4fce1714dc2a9","unresolved":false,"context_lines":[{"line_number":6208,"context_line":"            df.delete(self.ts())"},{"line_number":6209,"context_line":""},{"line_number":6210,"context_line":"            def mock_listdir(*args, **kwargs):"},{"line_number":6211,"context_line":"                # simulating hashes.pkl modification by another process while"},{"line_number":6212,"context_line":"                # get_hashes is executing"},{"line_number":6213,"context_line":"                # df2 is created to check path hashes recalculation"},{"line_number":6214,"context_line":"                result \u003d orig_listdir(*args, **kwargs)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a461143_0169678b","line":6211,"updated":"2017-01-27 18:43:32.000000000","message":":/ sorry I didn\u0027t spot this before, comment isn\u0027t correct - hashes.pkl is not modified here, just hashes.invalid\n\n#followup","commit_id":"767bcf1caaf7145655e9530af69cd7b0ac7d49b0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"46a06230a2c3eb852a5f57105db4fce1714dc2a9","unresolved":false,"context_lines":[{"line_number":6231,"context_line":"            self.assertIn(suffix, hashes)"},{"line_number":6232,"context_line":"            self.assertIn(suffix2, hashes)"},{"line_number":6233,"context_line":""},{"line_number":6234,"context_line":"    def test_hash_invalidations_survive_racing_get_hashes_same_suffix(self):"},{"line_number":6235,"context_line":"        # verify that when two processes concurrently call get_hashes, then any"},{"line_number":6236,"context_line":"        # concurrent hash invalidation will survive and be consolidated on a"},{"line_number":6237,"context_line":"        # subsequent call to get_hashes (i.e. ensure first get_hashes process"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a461143_412a8f7f","line":6234,"updated":"2017-01-27 18:43:32.000000000","message":"ok. fails with this regression:\n\n  diff --git a/swift/obj/diskfile.py b/swift/obj/diskfile.py\nindex 4d9bbb7..961beed 100644\n--- a/swift/obj/diskfile.py\n+++ b/swift/obj/diskfile.py\n@@ -268,7 +268,7 @@ def write_hashes(partition_dir, hashes):\n     The updated key is added to hashes before it is written.\n     \"\"\"\n     hashes_file \u003d join(partition_dir, HASH_FILE)\n-    hashes[\u0027updated\u0027] \u003d time.time()\n+    #hashes[\u0027updated\u0027] \u003d time.time()\n     return write_pickle(hashes, hashes_file, partition_dir, PICKLE_PROTOCOL)\n \n  \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\nFAIL: test_hash_invalidations_survive_racing_get_hashes_same_suffix (test.unit.obj.test_diskfile.TestSuffixHashes)\n----------------------------------------------------------------------\nTraceback (most recent call last):\n  File \"/home/anc/0dev/swift/test/unit/obj/test_diskfile.py\", line 6294, in test_hash_invalidations_survive_racing_get_hashes_same_suffix\n    self.assertEqual(hashes[suffix], non_local[\u0027hash\u0027])\n  File \"/home/anc/0dev/swift/test/unit/obj/test_diskfile.py\", line 5726, in assertEqual\n    unittest.TestCase.assertEqual(self, *args)\n  File \"/usr/lib/python2.7/contextlib.py\", line 35, in __exit__\n    self.gen.throw(type, value, traceback)\n  File \"/home/anc/0dev/swift/test/unit/obj/test_diskfile.py\", line 5722, in policy_in_message\n    raise AssertionError(str(err) + policy_trailer)\nAssertionError: {None: \u00278ee02fc148a6f3b001016b123461c6dd\u0027} !\u003d {None: \u002710a46b7883e121b8e976e381e54f6899\u0027}\n- {None: \u00278ee02fc148a6f3b001016b123461c6dd\u0027}\n+ {None: \u002710a46b7883e121b8e976e381e54f6899\u0027}","commit_id":"767bcf1caaf7145655e9530af69cd7b0ac7d49b0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"46a06230a2c3eb852a5f57105db4fce1714dc2a9","unresolved":false,"context_lines":[{"line_number":6243,"context_line":"            df_mgr \u003d self.df_router[policy]"},{"line_number":6244,"context_line":"            orig_hash_suffix \u003d df_mgr._hash_suffix"},{"line_number":6245,"context_line":"            # create hashes.pkl"},{"line_number":6246,"context_line":"            df_mgr.get_hashes(\u0027sda1\u0027, \u00270\u0027, [], policy)"},{"line_number":6247,"context_line":""},{"line_number":6248,"context_line":"            df \u003d df_mgr.get_diskfile(\u0027sda1\u0027, \u00270\u0027, \u0027a\u0027, \u0027c\u0027, \u0027o\u0027,"},{"line_number":6249,"context_line":"                                     policy\u003dpolicy)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a461143_c473ede5","line":6246,"updated":"2017-01-27 18:43:32.000000000","message":"OFF TOPIC: this test will fail [1] if we do not create hashes.pkl here. That\u0027s another bug - when the other process calls consolidate_hashes, it sees no hashes.pkl and so truncates hashes.invalid :/\n\n[1] https://review.openstack.org/#/c/426336","commit_id":"767bcf1caaf7145655e9530af69cd7b0ac7d49b0"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ada1a6c651d2389f47b36216474dd8a04db6caf5","unresolved":false,"context_lines":[{"line_number":6306,"context_line":"            open(hashes_file, \u0027w\u0027)"},{"line_number":6307,"context_line":"            hashes \u003d df_mgr.get_hashes(\u0027sda1\u0027, \u00270\u0027, [], policy)"},{"line_number":6308,"context_line":"            self.assertIn(suffix, hashes)"},{"line_number":6309,"context_line":"            self.assertEqual([], df_mgr.logger.get_lines_for_level(\u0027warning\u0027))"},{"line_number":6310,"context_line":""},{"line_number":6311,"context_line":"    def test_invalidate_hash_consolidation(self):"},{"line_number":6312,"context_line":"        def assert_consolidation(suffixes):"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a461143_a597b446","line":6309,"updated":"2017-01-27 23:05:40.000000000","message":"pretty sure without this warning assertion this test still passes even if you left write_hashes Type error when you pass it None (because we allow for the race to overwrite the invalidation on new suffix dirs - i.e. the other bug)","commit_id":"14c2d5713bb6a73c6b199e1a121b051d47308c9e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a211e4dd262246787d9adc65b7ccd2fe8813ea70","unresolved":false,"context_lines":[{"line_number":6185,"context_line":"            self.assertIn(suffix, hashes)"},{"line_number":6186,"context_line":"            self.assertIn(suffix2, hashes)"},{"line_number":6187,"context_line":""},{"line_number":6188,"context_line":"    def test_hash_invalidations_survive_racing_get_hashes_diff_suffix(self):"},{"line_number":6189,"context_line":"        # get_hashes must repeat path listing and return all hashes when"},{"line_number":6190,"context_line":"        # another concurrent process created new pkl before hashes are stored"},{"line_number":6191,"context_line":"        # by the first process"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a461143_a98ca3da","line":6188,"updated":"2017-01-31 13:51:38.000000000","message":"nit: followup, I had some comments on this test in an earlier patchset\n\nhttps://review.openstack.org/#/c/419787/9/test/unit/obj/test_diskfile.py@6187\n\nDone in follow up here https://review.openstack.org/#/c/427205/","commit_id":"a6ee421ab5c84bf2a5076ad4deec8881d7cc34c6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a211e4dd262246787d9adc65b7ccd2fe8813ea70","unresolved":false,"context_lines":[{"line_number":6293,"context_line":"            with open(invalidations_file, \u0027r\u0027) as f:"},{"line_number":6294,"context_line":"                self.assertEqual(\u0027\u0027, f.read())"},{"line_number":6295,"context_line":"            # so hashes should have the latest suffix hash..."},{"line_number":6296,"context_line":"            self.assertEqual(hashes[suffix], non_local[\u0027hash\u0027])"},{"line_number":6297,"context_line":""},{"line_number":6298,"context_line":"    def _check_unpickle_error_and_get_hashes_failure(self, existing):"},{"line_number":6299,"context_line":"        for policy in self.iter_policies():"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a461143_095db714","line":6296,"updated":"2017-01-31 13:51:38.000000000","message":"ok. this test will fails here on master (with the assertion at line 6279 removed) because second process consolidate_hashes doesn\u0027t write the hashes.pkl (invalidation for same suffix -\u003e hashes entry is unchanged -\u003e no write_pickle)","commit_id":"a6ee421ab5c84bf2a5076ad4deec8881d7cc34c6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a211e4dd262246787d9adc65b7ccd2fe8813ea70","unresolved":false,"context_lines":[{"line_number":6301,"context_line":"            df \u003d df_mgr.get_diskfile(\u0027sda1\u0027, \u00270\u0027, \u0027a\u0027, \u0027c\u0027, \u0027o\u0027,"},{"line_number":6302,"context_line":"                                     policy\u003dpolicy)"},{"line_number":6303,"context_line":"            suffix \u003d os.path.basename(os.path.dirname(df._datadir))"},{"line_number":6304,"context_line":"            if existing:"},{"line_number":6305,"context_line":"                df.delete(self.ts())"},{"line_number":6306,"context_line":"                hashes \u003d df_mgr.get_hashes(\u0027sda1\u0027, \u00270\u0027, [], policy)"},{"line_number":6307,"context_line":"            df.delete(self.ts())"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a461143_e57091a6","line":6304,"updated":"2017-01-31 13:51:38.000000000","message":"+1 nice improvement","commit_id":"a6ee421ab5c84bf2a5076ad4deec8881d7cc34c6"},{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"6b32522e5eccb017d2f86247fdca7e6c52781ec9","unresolved":false,"context_lines":[{"line_number":6309,"context_line":"                                     diskfile.get_data_dir(policy), \u00270\u0027)"},{"line_number":6310,"context_line":"            hashes_file \u003d os.path.join(part_path, diskfile.HASH_FILE)"},{"line_number":6311,"context_line":"            # write a corrupt hashes.pkl"},{"line_number":6312,"context_line":"            open(hashes_file, \u0027w\u0027)"},{"line_number":6313,"context_line":"            # simulate first call to get_hashes failing after attempting to"},{"line_number":6314,"context_line":"            # consolidate hashes"},{"line_number":6315,"context_line":"            with mock.patch(\u0027swift.obj.diskfile.os.listdir\u0027,"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a461143_25a6ebad","line":6312,"updated":"2017-02-01 12:03:03.000000000","message":"nit: maybe\n\nwith open(hashes_file, \u0027w\u0027):\n    pass\n\nIt\u0027s okay for this test though.","commit_id":"a6ee421ab5c84bf2a5076ad4deec8881d7cc34c6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a211e4dd262246787d9adc65b7ccd2fe8813ea70","unresolved":false,"context_lines":[{"line_number":6316,"context_line":"                            side_effect\u003dException()):"},{"line_number":6317,"context_line":"                self.assertRaises("},{"line_number":6318,"context_line":"                    Exception, df_mgr.get_hashes, \u0027sda1\u0027, \u00270\u0027, [], policy)"},{"line_number":6319,"context_line":"            # sanity on-disk state is invalid"},{"line_number":6320,"context_line":"            with open(hashes_file) as f:"},{"line_number":6321,"context_line":"                found_hashes \u003d pickle.load(f)"},{"line_number":6322,"context_line":"                found_hashes.pop(\u0027updated\u0027)"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a461143_8911a706","line":6319,"updated":"2017-01-31 13:51:38.000000000","message":"ok these sanity checks fail on master","commit_id":"a6ee421ab5c84bf2a5076ad4deec8881d7cc34c6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a211e4dd262246787d9adc65b7ccd2fe8813ea70","unresolved":false,"context_lines":[{"line_number":6324,"context_line":"            # verify subsequent call to get_hashes reaches correct outcome"},{"line_number":6325,"context_line":"            hashes \u003d df_mgr.get_hashes(\u0027sda1\u0027, \u00270\u0027, [], policy)"},{"line_number":6326,"context_line":"            self.assertIn(suffix, hashes)"},{"line_number":6327,"context_line":"            self.assertEqual([], df_mgr.logger.get_lines_for_level(\u0027warning\u0027))"},{"line_number":6328,"context_line":""},{"line_number":6329,"context_line":"    def test_unpickle_error_and_get_hashes_failure_new_part(self):"},{"line_number":6330,"context_line":"        self._check_unpickle_error_and_get_hashes_failure(False)"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a461143_e9022bb3","line":6327,"updated":"2017-01-31 13:51:38.000000000","message":"apart from on disk state sanity checks this passes on master","commit_id":"a6ee421ab5c84bf2a5076ad4deec8881d7cc34c6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a211e4dd262246787d9adc65b7ccd2fe8813ea70","unresolved":false,"context_lines":[{"line_number":6449,"context_line":"                mock_hash_suffix.reset_mock()"},{"line_number":6450,"context_line":"                # updates pkl file"},{"line_number":6451,"context_line":"                df_mgr.get_hashes(\u0027sda1\u0027, \u00270\u0027, [], policy)"},{"line_number":6452,"context_line":"                mock_consolidate_hashes.assert_called_once()"},{"line_number":6453,"context_line":"                self.assertEqual([mock.call(suffix_dir2)],"},{"line_number":6454,"context_line":"                                 mock_hash_suffix.call_args_list)"},{"line_number":6455,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"3a461143_899b6765","line":6452,"updated":"2017-01-31 13:51:38.000000000","message":"ok, fails on master here - consolidate_hashes gets called twice","commit_id":"a6ee421ab5c84bf2a5076ad4deec8881d7cc34c6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a211e4dd262246787d9adc65b7ccd2fe8813ea70","unresolved":false,"context_lines":[{"line_number":7621,"context_line":"        def hash_breaking_function(hashfile_path):"},{"line_number":7622,"context_line":"            # create a garbage invalid zero-byte file which can not unpickle"},{"line_number":7623,"context_line":"            open(hashfile_path, \u0027w\u0027).close()"},{"line_number":7624,"context_line":"        self._test_get_hashes_race(hash_breaking_function)"},{"line_number":7625,"context_line":""},{"line_number":7626,"context_line":"    def test_get_hashes_race_new_partition(self):"},{"line_number":7627,"context_line":"        def hash_breaking_function(hashfile_path):"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a461143_a9fbc377","line":7624,"updated":"2017-01-31 13:51:38.000000000","message":"ok fails on master at line 7618","commit_id":"a6ee421ab5c84bf2a5076ad4deec8881d7cc34c6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a211e4dd262246787d9adc65b7ccd2fe8813ea70","unresolved":false,"context_lines":[{"line_number":7631,"context_line":"            os.unlink(os.path.join(part_dir, \u0027.lock\u0027))"},{"line_number":7632,"context_line":"            # sanity"},{"line_number":7633,"context_line":"            self.assertEqual([], os.listdir(os.path.dirname(hashfile_path)))"},{"line_number":7634,"context_line":"        self._test_get_hashes_race(hash_breaking_function)"},{"line_number":7635,"context_line":""},{"line_number":7636,"context_line":"    def test_get_hashes_race_existing_partition(self):"},{"line_number":7637,"context_line":"        def hash_breaking_function(hashfile_path):"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a461143_e9f14b96","line":7634,"updated":"2017-01-31 13:51:38.000000000","message":"ok fails on master at line 7618","commit_id":"a6ee421ab5c84bf2a5076ad4deec8881d7cc34c6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a211e4dd262246787d9adc65b7ccd2fe8813ea70","unresolved":false,"context_lines":[{"line_number":7637,"context_line":"        def hash_breaking_function(hashfile_path):"},{"line_number":7638,"context_line":"            # no-op - simulate ok existing partition"},{"line_number":7639,"context_line":"            self.assertTrue(os.path.exists(hashfile_path))"},{"line_number":7640,"context_line":"        self._test_get_hashes_race(hash_breaking_function)"},{"line_number":7641,"context_line":""},{"line_number":7642,"context_line":"    def test_get_hashes_hash_suffix_enotdir(self):"},{"line_number":7643,"context_line":"        for policy in self.iter_policies():"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a461143_8937e74b","line":7640,"updated":"2017-01-31 13:51:38.000000000","message":"passes on master","commit_id":"a6ee421ab5c84bf2a5076ad4deec8881d7cc34c6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a211e4dd262246787d9adc65b7ccd2fe8813ea70","unresolved":false,"context_lines":[{"line_number":7773,"context_line":"        }"},{"line_number":7774,"context_line":"        self.assertEqual(expected, data)"},{"line_number":7775,"context_line":""},{"line_number":7776,"context_line":"    def test_write_hashes_safe_default(self):"},{"line_number":7777,"context_line":"        hashes \u003d {}"},{"line_number":7778,"context_line":"        now \u003d time()"},{"line_number":7779,"context_line":"        with mock.patch(\u0027swift.obj.diskfile.time.time\u0027, return_value\u003dnow):"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a461143_25cd3985","line":7776,"updated":"2017-01-31 13:51:38.000000000","message":"ok fails if I remove the setdefault(\u0027valid\u0027, False) in write_hashes, or make the default True","commit_id":"a6ee421ab5c84bf2a5076ad4deec8881d7cc34c6"}]}