)]}'
{"swift/obj/reconstructor.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e72da75ac322239514e2b62bce0acc1f43718cf6","unresolved":false,"context_lines":[{"line_number":302,"context_line":"                all_devices_reporting \u003d False"},{"line_number":303,"context_line":"                break"},{"line_number":304,"context_line":"            first_start \u003d min(first_start, start_time)"},{"line_number":305,"context_line":"            last_finish \u003d max(last_finish, finish_time)"},{"line_number":306,"context_line":"        if all_devices_reporting and last_finish \u003e 0:"},{"line_number":307,"context_line":"            duration \u003d last_finish - first_start"},{"line_number":308,"context_line":"            recon_update \u003d {"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f436f4f_88fcd9b5","line":305,"updated":"2017-08-21 15:14:35.000000000","message":"alternative fix might be:\n\n  if not self.all_local_devices:\n      last_finish \u003d first_start()","commit_id":"f6534555acd2c81882c4a0199f29ab2105b7b7f1"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e72da75ac322239514e2b62bce0acc1f43718cf6","unresolved":false,"context_lines":[{"line_number":1238,"context_line":"        }"},{"line_number":1239,"context_line":""},{"line_number":1240,"context_line":"        devices \u003d override_devices or self.all_local_devices"},{"line_number":1241,"context_line":"        if self.reconstructor_workers \u003e 0 and devices:"},{"line_number":1242,"context_line":"            recon_update[\u0027pid\u0027] \u003d os.getpid()"},{"line_number":1243,"context_line":"            recon_update \u003d {\u0027object_reconstruction_per_disk\u0027: {"},{"line_number":1244,"context_line":"                d: recon_update for d in devices}}"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f436f4f_c8bf81fa","line":1241,"range":{"start_line":1241,"start_character":46,"end_line":1241,"end_character":53},"updated":"2017-08-21 15:14:35.000000000","message":"note to self: \n\nAt first glance one might think that self.all_local_devices would be a sufficient condition, but including override_devices in the condition for doing a per-disk recon_update IS significant. We could have no local devices but still have override_devices when --devices is used on CLI with once mode. In that case we do NOT want to write total stats!","commit_id":"f6534555acd2c81882c4a0199f29ab2105b7b7f1"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"e17ea226a0d24514b3b45fa46ab573905b9f46dc","unresolved":false,"context_lines":[{"line_number":1241,"context_line":"            devices \u003d override_devices or self.all_local_devices"},{"line_number":1242,"context_line":"            recon_update[\u0027pid\u0027] \u003d os.getpid()"},{"line_number":1243,"context_line":"            recon_update \u003d {\u0027object_reconstruction_per_disk\u0027: {"},{"line_number":1244,"context_line":"                d: recon_update for d in devices}}"},{"line_number":1245,"context_line":"        else:"},{"line_number":1246,"context_line":"            # if not running in worker mode, kill any per_disk stats"},{"line_number":1247,"context_line":"            recon_update[\u0027object_reconstruction_per_disk\u0027] \u003d {}"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f287b81_d4909451","side":"PARENT","line":1244,"updated":"2017-08-22 09:27:11.000000000","message":"To Pete. I suppose if devices is an empty list this for-loop doesn\u0027t work and the direct assignment to recon_unpdate will loose the reconstruction_time and last. But... IDK why not doing just recon_update.update({\u0027object_reconstruction_per_disk\u0027: d: recon_update for d in devices}) to keep recunstruction_time/last info in the root node in the recon_update dict.","commit_id":"1f751a797997e241d5cc9a32b9307e3b9f0df4c3"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"e17ea226a0d24514b3b45fa46ab573905b9f46dc","unresolved":false,"context_lines":[{"line_number":1239,"context_line":""},{"line_number":1240,"context_line":"        devices \u003d override_devices or self.all_local_devices"},{"line_number":1241,"context_line":"        if self.reconstructor_workers \u003e 0 and devices:"},{"line_number":1242,"context_line":"            recon_update[\u0027pid\u0027] \u003d os.getpid()"},{"line_number":1243,"context_line":"            recon_update \u003d {\u0027object_reconstruction_per_disk\u0027: {"},{"line_number":1244,"context_line":"                d: recon_update for d in devices}}"},{"line_number":1245,"context_line":"        else:"},{"line_number":1246,"context_line":"            # if not running in worker mode, kill any per_disk stats"},{"line_number":1247,"context_line":"            recon_update[\u0027object_reconstruction_per_disk\u0027] \u003d {}"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f436f4f_f7d334d4","line":1244,"range":{"start_line":1242,"start_character":0,"end_line":1244,"end_character":50},"updated":"2017-08-22 09:27:11.000000000","message":"At the first look, I was thinking the *pid* info seems lost because recon_update is overwritten by per_device stat but it\u0027s wrong. the *recon_update* dict is kept in the per_disk dict.\n\nHmm... but I\u0027m still not get yet the difference of object_reconstruction_time/last between per_disk and root node in the recon_update dict.\n\nAt the first look, I\u0027m feeling why we don\u0027t do update the recon_update with per_disk stat, instead of directly assignment, like:\n\n recon_update[\u0027object_reconstruction_per_disk\u0027] \u003d {\n   d: recon_update for d in devices}\n\nor\n\n recon_update.update({\u0027object_reconstruction_per_disk\u0027: {\n   d: recon_update for d in devices}})\n\n??\n\nOtherwise, the recon_cache will have different format A) with no device object_reconstruction_time/last is in root node in the dict but B) with device(s), the root  node is as object_reconstruction_per_disk and then, each child node contains object_reconstruction_time/last info.\n\ni.e.\n\nA):\n {\u0027object_reconstruction_time\u0027: ts1, \u0027object_reconstruction_last\u0027: ts2}\n\nB):\n {\u0027object_reconstruction_per_disk\u0027:\n   {d1: {\u0027object_reconstruction_time\u0027: ts1,\n         \u0027object_reconstruction_time\u0027: ts2}\n   }\n }\n\nSince the recon_cache is kept when we don\u0027t update the key, object_reconstruction_time is not updated from old timestamp at dumping without devices info. like https://review.openstack.org/#/c/495805/4/test/unit/obj/test_reconstructor.py@1668\n\n\nHowever I\u0027m not sure if this is by design (because it\u0027s tested)\nPerhaps, it seems like it\u0027s better keeping reconstruction_time/last in the root node like as\n\nExpected(?)):\n {\u0027object_reconstruction_time\u0027: ts1,\n  \u0027object_reconstruction_last\u0027: ts2,\n  \u0027object_reconstruction_per_disk\u0027:\n   {d1: {\u0027object_reconstruction_time\u0027: ts1,\n         \u0027object_reconstruction_time\u0027: ts2}\n   }\n }\n\nwhen some of per disk stats is updated?\n\nI saw also the recon aggregation in other place, https://github.com/openstack/swift/blob/master/swift/obj/reconstructor.py#L306-L310 and it seems, at least, object_reconstruction_last is the most recent finish time so max of object_reconstruction_last for each device.\n\nAm I missing something?","commit_id":"63ca3a74ef409c416c49fe7ceeba2792787082e1"}]}