)]}'
{"swift/common/storage_policy.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e9ab8c62e46342520dee6a422a0faa35f31d8cdc","unresolved":false,"context_lines":[{"line_number":639,"context_line":"            swift_dir, ring_name\u003dself.ring_name,"},{"line_number":640,"context_line":"            validation_hook\u003dvalidate_ring_data)"},{"line_number":641,"context_line":""},{"line_number":642,"context_line":"    def get_backend_frag_index(self, node_index):"},{"line_number":643,"context_line":"        \"\"\""},{"line_number":644,"context_line":"        Fragment index for the erasure coded object"},{"line_number":645,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"5f201791_8f5364e2","line":642,"updated":"2017-06-27 15:47:50.000000000","message":"+1, the method name was confusing","commit_id":"85dfb3a93703345074170e02760e890991309e45"}],"swift/proxy/controllers/obj.py":[{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"b0a71cab7485c209444899350c3a3d3d95ca2cbf","unresolved":false,"context_lines":[{"line_number":1447,"context_line":"        self.path \u003d path"},{"line_number":1448,"context_line":"        self.connect_duration \u003d connect_duration"},{"line_number":1449,"context_line":"        # for handoff nodes frag_index is None"},{"line_number":1450,"context_line":"        self.frag_index \u003d node.get(\u0027frag_index\u0027)"},{"line_number":1451,"context_line":""},{"line_number":1452,"context_line":"        self.failed \u003d False"},{"line_number":1453,"context_line":"        self.queue \u003d None"}],"source_content_type":"text/x-python","patch_set":3,"id":"ffe62b97_a1cfa3ae","line":1450,"updated":"2017-03-27 19:48:31.000000000","message":"This is the generic Putter class that applies to replication as well as EC.  In some version of this file, we did have an ECPutter where this frag_index would have been very appropriate for.  Since this class is used for both, it may be good to also mention that frag_index is None for replication as well (it already has a comment for handoff nodes).\n\nAnd for handoffs, I don\u0027t see a \u0027None\u0027 value for handoff nodes in the unittests.  The only EC unittest that gets None for frag_index is TestECObjController.test_PUT_old_obj_server, and I see several places that speak of testing handoffs.\n\nSo, maybe this comment should just change to say \n# frag_index is None for non-EC","commit_id":"61baaed401a9b565f315bee67dd8fb3643f0058a"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b4c64c4330a98191fd0e1a6588e633f15a4d4479","unresolved":false,"context_lines":[{"line_number":1447,"context_line":"        self.path \u003d path"},{"line_number":1448,"context_line":"        self.connect_duration \u003d connect_duration"},{"line_number":1449,"context_line":"        # for handoff nodes frag_index is None"},{"line_number":1450,"context_line":"        self.frag_index \u003d node.get(\u0027frag_index\u0027)"},{"line_number":1451,"context_line":""},{"line_number":1452,"context_line":"        self.failed \u003d False"},{"line_number":1453,"context_line":"        self.queue \u003d None"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff0f0b1f_3d944544","line":1450,"in_reply_to":"ffe62b97_a1cfa3ae","updated":"2017-05-17 23:29:07.000000000","message":"+1 on calling out that frag_index is None for all replicated Putters.\n\nJanie, IIRC we *instantiate* handoff Putters with None then assign them a frag (without setting .frag_index!) in _determine_chunk_destinations -- that\u0027s why we see the funny \"handoff\" manipulations starting around https://github.com/openstack/swift/blob/2.14.0/test/unit/proxy/controllers/test_obj.py#L1460\n\nDo we ever start trying to connect to handoffs before some attempts to primaries have failed, though? I guess we still don\u0027t want to start making assignments until we actually know what connections we have available, though; otherwise we may lose durability when we have duplicated frags and not enough connections.","commit_id":"61baaed401a9b565f315bee67dd8fb3643f0058a"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"5fde215d16da103044dbb60c59d668e5f20946eb","unresolved":false,"context_lines":[{"line_number":1447,"context_line":"        self.path \u003d path"},{"line_number":1448,"context_line":"        self.connect_duration \u003d connect_duration"},{"line_number":1449,"context_line":"        # for handoff nodes frag_index is None"},{"line_number":1450,"context_line":"        self.frag_index \u003d node.get(\u0027frag_index\u0027)"},{"line_number":1451,"context_line":""},{"line_number":1452,"context_line":"        self.failed \u003d False"},{"line_number":1453,"context_line":"        self.queue \u003d None"}],"source_content_type":"text/x-python","patch_set":3,"id":"7f231b9d_94460bc0","line":1450,"in_reply_to":"ffe62b97_a1cfa3ae","updated":"2017-06-15 08:30:23.000000000","message":"Done","commit_id":"61baaed401a9b565f315bee67dd8fb3643f0058a"},{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"b0a71cab7485c209444899350c3a3d3d95ca2cbf","unresolved":false,"context_lines":[{"line_number":2658,"context_line":"        def frag_index_updated(node):"},{"line_number":2659,"context_line":"            node.update("},{"line_number":2660,"context_line":"                {\u0027frag_index\u0027:"},{"line_number":2661,"context_line":"                 policy.get_backend_frag_index(node.get(\u0027index\u0027))})"},{"line_number":2662,"context_line":"            return node"},{"line_number":2663,"context_line":""},{"line_number":2664,"context_line":"        obj_ring \u003d policy.object_ring"}],"source_content_type":"text/x-python","patch_set":3,"id":"ffe62b97_9049878c","line":2661,"updated":"2017-03-27 19:48:31.000000000","message":"Let me see if I understand.  This change adds storing the frag_index on the node.  That is why the putter no longer needs node_index, but does now store frag_index.  And now the calculation from node_index to frag_index is not required inside determine_chunk_destinations.  \n\nIt seems like this is just moving things and renaming.  I am missing the motivation behind the change.  How is it better?  Did something change with EC duplication that would help me to understand why you thought to make this change?  Is this setting up for some other change later where node[\u0027frag_index\u0027] is used?","commit_id":"61baaed401a9b565f315bee67dd8fb3643f0058a"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"5fde215d16da103044dbb60c59d668e5f20946eb","unresolved":false,"context_lines":[{"line_number":2658,"context_line":"        def frag_index_updated(node):"},{"line_number":2659,"context_line":"            node.update("},{"line_number":2660,"context_line":"                {\u0027frag_index\u0027:"},{"line_number":2661,"context_line":"                 policy.get_backend_frag_index(node.get(\u0027index\u0027))})"},{"line_number":2662,"context_line":"            return node"},{"line_number":2663,"context_line":""},{"line_number":2664,"context_line":"        obj_ring \u003d policy.object_ring"}],"source_content_type":"text/x-python","patch_set":3,"id":"7f231b9d_d47803c5","line":2661,"in_reply_to":"ffe62b97_9049878c","updated":"2017-06-15 08:30:23.000000000","message":"The main motivation is for this change is that I\u0027d like to keep *correct attribute name* in the Putter class.\n\nAs you understand if it looks just renaming but the value is different if swift uses ec duplication. (i.e. frag_index could be duplicate but node_index should be unique)\n\nAnd then, in the old code, the node_index can be used only with policy information because the necessary information is *frag_index* in fact.\n\nSo moving the mapping to here, frag_index is from policy based method with node_index. it looks to be obvious. And then determine_chunk_destination will be able to concentrate only for filling the lack frag_index holes without extra node_index/frag_index calculation, won\u0027t it?\n\nEr - perhaps the meaning of this patch has been faded down while working on ec duplication landed. Before ec duplication, I was feeling we have annoying complexity between the word of \"node_index\" and \"frag_index\". (e.g. somewhere using node_index that means frag_index because boefore ec duplication, that was same.)\n\nSince ec duplication landed, the node_index is not equal to frag_index so keeping correct information in the instance will be win for code maintainability. However, exactly, the diff results in just renaming... Hmm...\n\nThough it might be another topic, if possible, I would have liked to encapsulate around here into ring but the mapping requires policy info so we cannot do that and one more reason we can not change all *node_index* to *frag_index* is *reconstructor* needs node_index to determine the partner nodes.","commit_id":"61baaed401a9b565f315bee67dd8fb3643f0058a"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b4c64c4330a98191fd0e1a6588e633f15a4d4479","unresolved":false,"context_lines":[{"line_number":2664,"context_line":"        obj_ring \u003d policy.object_ring"},{"line_number":2665,"context_line":"        node_iter \u003d GreenthreadSafeIterator("},{"line_number":2666,"context_line":"            six.moves.map(lambda node: frag_index_updated(node),"},{"line_number":2667,"context_line":"                          self.iter_nodes_local_first(obj_ring, partition)))"},{"line_number":2668,"context_line":""},{"line_number":2669,"context_line":"        min_conns \u003d policy.quorum"},{"line_number":2670,"context_line":"        putters \u003d self._get_put_connections("}],"source_content_type":"text/x-python","patch_set":3,"id":"ff0f0b1f_fdbecdfd","line":2667,"updated":"2017-05-17 23:29:07.000000000","message":"Better as just\n\n node_iter \u003d GreenthreadSafeIterator(\n     frag_index_updated(node)\n     for node in self.iter_nodes_local_first(obj_ring, partition))\n\n? At the very least we can drop the lambda and just say\n\n map(frag_index_updated, ...)","commit_id":"61baaed401a9b565f315bee67dd8fb3643f0058a"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"5fde215d16da103044dbb60c59d668e5f20946eb","unresolved":false,"context_lines":[{"line_number":2664,"context_line":"        obj_ring \u003d policy.object_ring"},{"line_number":2665,"context_line":"        node_iter \u003d GreenthreadSafeIterator("},{"line_number":2666,"context_line":"            six.moves.map(lambda node: frag_index_updated(node),"},{"line_number":2667,"context_line":"                          self.iter_nodes_local_first(obj_ring, partition)))"},{"line_number":2668,"context_line":""},{"line_number":2669,"context_line":"        min_conns \u003d policy.quorum"},{"line_number":2670,"context_line":"        putters \u003d self._get_put_connections("}],"source_content_type":"text/x-python","patch_set":3,"id":"7f231b9d_741c8fc0","line":2667,"in_reply_to":"ff0f0b1f_fdbecdfd","updated":"2017-06-15 08:30:23.000000000","message":"Done","commit_id":"61baaed401a9b565f315bee67dd8fb3643f0058a"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"776e9b9f5304cf33eb0d662812a75739ce298c2f","unresolved":false,"context_lines":[{"line_number":165,"context_line":"            (node for node in all_nodes if node not in first_n_local_nodes))"},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"        return self.app.iter_nodes("},{"line_number":168,"context_line":"            ring, partition, node_iter\u003dlocal_first_node_iter, policy\u003dpolicy)"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"    def GETorHEAD(self, req):"},{"line_number":171,"context_line":"        \"\"\"Handle HTTP GET or HEAD requests.\"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"7f231b9d_b3645d23","line":168,"updated":"2017-06-16 04:34:12.000000000","message":"It seems like the iter_nodes re-order the nodes on PUT and it triggers invalid test statement on the failure test at gerrit CI.\n\nHowever, if I understand correctly, this will happen even it\u0027s on the master branch so that I need dig more state here to correct this patch.","commit_id":"64585802c9bc186f325244e763928a2c70b55e1c"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e9ab8c62e46342520dee6a422a0faa35f31d8cdc","unresolved":false,"context_lines":[{"line_number":1451,"context_line":"        self.path \u003d path"},{"line_number":1452,"context_line":"        self.connect_duration \u003d connect_duration"},{"line_number":1453,"context_line":"        # for handoff nodes node_index is None"},{"line_number":1454,"context_line":"        self.node_index \u003d node.get(\u0027index\u0027)"},{"line_number":1455,"context_line":""},{"line_number":1456,"context_line":"        self.failed \u003d False"},{"line_number":1457,"context_line":"        self.queue \u003d None"}],"source_content_type":"text/x-python","patch_set":6,"id":"5f201791_af950089","side":"PARENT","line":1454,"updated":"2017-06-27 15:47:50.000000000","message":"why do we even need this variable to be set (but only when node happens to be an EC primary), when node is already a var? We can just extract node_index/frag_index when (and only when) it is needed in _determine_chunk_destinations","commit_id":"dfa361261cb87d8863a69b2f1fb88ed6cd503410"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e9ab8c62e46342520dee6a422a0faa35f31d8cdc","unresolved":false,"context_lines":[{"line_number":2359,"context_line":"        handoff_conns \u003d []"},{"line_number":2360,"context_line":"        putter_to_frag_index \u003d {}"},{"line_number":2361,"context_line":"        for p in putters:"},{"line_number":2362,"context_line":"            if p.node_index is not None:"},{"line_number":2363,"context_line":"                putter_to_frag_index[p] \u003d policy.get_backend_index("},{"line_number":2364,"context_line":"                    p.node_index)"},{"line_number":2365,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":6,"id":"5f201791_2fd8d078","side":"PARENT","line":2362,"updated":"2017-06-27 15:47:50.000000000","message":"this could be:\n\n  if p.node.get(\u0027index\u0027) is not None:\n\nthen we examine node_index/frag_index in EC case only\n\na couple of tests fail with this change because FakePutter doesn\u0027t have a node","commit_id":"dfa361261cb87d8863a69b2f1fb88ed6cd503410"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e9ab8c62e46342520dee6a422a0faa35f31d8cdc","unresolved":false,"context_lines":[{"line_number":2361,"context_line":"        for p in putters:"},{"line_number":2362,"context_line":"            if p.node_index is not None:"},{"line_number":2363,"context_line":"                putter_to_frag_index[p] \u003d policy.get_backend_index("},{"line_number":2364,"context_line":"                    p.node_index)"},{"line_number":2365,"context_line":"            else:"},{"line_number":2366,"context_line":"                handoff_conns.append(p)"},{"line_number":2367,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"5f201791_8f9da467","side":"PARENT","line":2364,"updated":"2017-06-27 15:47:50.000000000","message":"I think I prefer having this call made here because it is then adjacent to where the result has significance","commit_id":"dfa361261cb87d8863a69b2f1fb88ed6cd503410"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"ab46eba3285eb9d15d146d1f699062f3a47242c7","unresolved":false,"context_lines":[{"line_number":2361,"context_line":"        for p in putters:"},{"line_number":2362,"context_line":"            if p.node_index is not None:"},{"line_number":2363,"context_line":"                putter_to_frag_index[p] \u003d policy.get_backend_index("},{"line_number":2364,"context_line":"                    p.node_index)"},{"line_number":2365,"context_line":"            else:"},{"line_number":2366,"context_line":"                handoff_conns.append(p)"},{"line_number":2367,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3f1d235d_b8072a19","side":"PARENT","line":2364,"in_reply_to":"5f201791_8f9da467","updated":"2017-07-10 09:46:06.000000000","message":"My primary motivation of this patch was to enable determine_chunk_destinatinons as more obvious/intuitive.\n\nThe node index info is only used for calculating backend frag index and that is used both, too fill the non-indexed Putters (i.e. assigning frag index to handoff putters), and to push the frag-index as footer for the PUT.\n\nHence, pulling the node_index/frag_index translation could be easy to understand.\n\nHowever, in the recent reviews with you and Janie, it could make the code worse perhaps to understand?\n\nPerhaps, putter_to_frag_index could be already obvious to know the mapping from putter to fragment index for PUT sequence...","commit_id":"dfa361261cb87d8863a69b2f1fb88ed6cd503410"}]}