)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"8c1d3add3e50f2ffb5fbea040a1f3afcc8c24143","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Don\u0027t lose objects when db is rsync replicated during sharding"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Before, if an rsync replication updated a hash.db that was partially"},{"line_number":10,"context_line":"cleaved and objects were added in the already cleaved namespace, the"},{"line_number":11,"context_line":"hash.db would be deleted when cleaving completed and the new object(s)"},{"line_number":12,"context_line":"would be lost."},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"5f93b717_89370ae4","line":10,"range":{"start_line":9,"start_character":14,"end_line":10,"end_character":7},"updated":"2018-01-29 20:16:41.000000000","message":"Wasn\u0027t this the point of the _can_push check around https://github.com/openstack/swift/blob/feature/deep/swift/container/replicator.py#L280-L283 ? To switch everything over to usync while sharding?","commit_id":"5213ba46fc847f471e9362e61593962c9cdb50ee"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c7dda9711477d788f8efd3f50bec86999e9a0903","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Don\u0027t lose objects when db is rsync replicated during sharding"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Before, if an rsync replication updated a hash.db that was partially"},{"line_number":10,"context_line":"cleaved and objects were added in the already cleaved namespace, the"},{"line_number":11,"context_line":"hash.db would be deleted when cleaving completed and the new object(s)"},{"line_number":12,"context_line":"would be lost."},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"5f93b717_a9a8941f","line":10,"range":{"start_line":9,"start_character":14,"end_line":10,"end_character":7},"in_reply_to":"5f93b717_89370ae4","updated":"2018-01-31 12:04:02.000000000","message":"hmmm, so I confess I had missed the significance of _can_push when theorising about possible replication scenarios. [1]\n\nPerhaps it is a contrived scenario that this patch fixes, but _can_push() doesn\u0027t help if the db file is missing, I think? then an rsync will happen (in fact, the probe test proves that) and we could (before this patch) lose objects.\n\nBut, are we sure the _can_push() veto is a good idea? i.e. do we really want to always force usync once in sharding state? yes, it keeps the old db read only, but if the unsharded peer has a large number of rows to replicate then the new \u0027small and responsive\u0027 hash_shard.db will get filled and even after cleaving will have a bunch of deleted items in its db for reclaim_age.\n\nAlso, compare with what happens when an unsharded node replicates to a sharded node. I *think* its REPLICATE request will get a 404 because the hash.db is missing and it will do an rsync [2] - so we end up needing to cleave the rsync\u0027d hash.db in that scenario.\n\nWith this patch, we could drop the _can_push() check and IF an unsharded peer has 2x row count then accept that it will rsync and merge to the \u0027read only\u0027 hash.db, and repeat the cleaving. Perhaps better, restart the cleaving.\n\n[1] https://etherpad.openstack.org/p/deep-containers-replication\n[2] that is what I observed while working on https://review.openstack.org/#/c/536520 where the replication source was the final shard shrinking back to the root","commit_id":"5213ba46fc847f471e9362e61593962c9cdb50ee"}],"swift/container/sharder.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c1ccfdcb6f7ad0eff53167eaabb8248aeaa3de56","unresolved":false,"context_lines":[{"line_number":542,"context_line":"                if broker.is_root_container():"},{"line_number":543,"context_line":"                    if is_leader:"},{"line_number":544,"context_line":"                        self._find_shrinks(broker, node, part)"},{"line_number":545,"context_line":"                else:"},{"line_number":546,"context_line":"                    # let the root know about this shard\u0027s sharded shard ranges"},{"line_number":547,"context_line":"                    # TODO: these shard ranges may have existed for some time"},{"line_number":548,"context_line":"                    # and already be updating the root with their usage e.g."},{"line_number":549,"context_line":"                    # multiple cycles before we finished cleaving, or process"},{"line_number":550,"context_line":"                    # failed right here. If so then the updates we send here"},{"line_number":551,"context_line":"                    # probably have an out of date view of the shards\u0027 usage,"},{"line_number":552,"context_line":"                    # but I think it is still ok to send these updates because"},{"line_number":553,"context_line":"                    # the meta_timestamp should prevent these updates undoing"},{"line_number":554,"context_line":"                    # any newer updates at the root from the actual shards."},{"line_number":555,"context_line":"                    # *It would be good to have a test to verify that.*"},{"line_number":556,"context_line":"                    own_shard_range \u003d broker.get_own_shard_range()"},{"line_number":557,"context_line":"                    own_shard_range \u003d own_shard_range.copy("},{"line_number":558,"context_line":"                        timestamp\u003dTimestamp.now())"},{"line_number":559,"context_line":"                    # TODO: maybe differentiate SHARDED vs SHRUNK?"},{"line_number":560,"context_line":"                    own_shard_range.state \u003d ShardRange.SHARDED"},{"line_number":561,"context_line":"                    own_shard_range.deleted \u003d 1"},{"line_number":562,"context_line":"                    self._update_shard_ranges("},{"line_number":563,"context_line":"                        broker.root_account, broker.root_container,"},{"line_number":564,"context_line":"                        [own_shard_range] + broker.get_shard_ranges())"},{"line_number":565,"context_line":"            else:"},{"line_number":566,"context_line":"                if not broker.is_root_container():"},{"line_number":567,"context_line":"                    # update the root container with this shard\u0027s usage stats"}],"source_content_type":"text/x-python","patch_set":6,"id":"3fa0c359_7414054f","side":"PARENT","line":564,"range":{"start_line":545,"start_character":16,"end_line":564,"end_character":70},"updated":"2018-02-07 09:50:24.000000000","message":"hmmm, I\u0027m having second thoughts about this change...\n\nbefore, we\u0027d repeatedly inform the root about shard/sub-shard status once in sharded state, now we get one chance when transitioning from sharding to sharded...and if that fails there is no guard to prevent moving to sharded, where we now never update the root.\n\nseems like the original code which always sent updates while in sharded state was safer","commit_id":"fbbde5c70c3f392627b0709170ad5f001f75e4f3"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"9ca0f8847980d31a88590baaba9cfdd0ce350262","unresolved":false,"context_lines":[{"line_number":60,"context_line":"         (value, timestamp.internal))"},{"line_number":61,"context_line":"        for key, value in info.items()"},{"line_number":62,"context_line":"    )"},{"line_number":63,"context_line":"    broker.update_metadata(metadata, validate_metadata\u003dTrue)"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"# TODO: needs unit test"}],"source_content_type":"text/x-python","patch_set":6,"id":"3fa0c359_adf814c4","line":63,"range":{"start_line":63,"start_character":37,"end_line":63,"end_character":59},"updated":"2018-02-06 21:43:41.000000000","message":"So what are we going to do when this raises HTTPBadRequest?\n\nWe\u0027re never actually in a context where we really want a swob.Response getting raised, are we?","commit_id":"cf1bfe37c63279f30bcb53572e95188229990d99"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c1ccfdcb6f7ad0eff53167eaabb8248aeaa3de56","unresolved":false,"context_lines":[{"line_number":60,"context_line":"         (value, timestamp.internal))"},{"line_number":61,"context_line":"        for key, value in info.items()"},{"line_number":62,"context_line":"    )"},{"line_number":63,"context_line":"    broker.update_metadata(metadata, validate_metadata\u003dTrue)"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"# TODO: needs unit test"}],"source_content_type":"text/x-python","patch_set":6,"id":"3fa0c359_316d2f32","line":63,"range":{"start_line":63,"start_character":37,"end_line":63,"end_character":59},"in_reply_to":"3fa0c359_adf814c4","updated":"2018-02-07 09:50:24.000000000","message":"eek, that\u0027s not good, my bad. I dropped in the validate_metadata check to flush out places we had booleans going into metadata but it probably doesn\u0027t need to be there since there are no user generated values being set here","commit_id":"cf1bfe37c63279f30bcb53572e95188229990d99"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"9ca0f8847980d31a88590baaba9cfdd0ce350262","unresolved":false,"context_lines":[{"line_number":538,"context_line":"                    if not broker.is_root_container():"},{"line_number":539,"context_line":"                        # let the root know about this shard\u0027s sharded shard"},{"line_number":540,"context_line":"                        # ranges; this is the point at which the root container"},{"line_number":541,"context_line":"                        # will see the new shards will move to ACTIVE state and"},{"line_number":542,"context_line":"                        # the sharded shard simultaneously become deleted. Do"},{"line_number":543,"context_line":"                        # this now because the shards are active, even though"},{"line_number":544,"context_line":"                        # there may be repeated cleaving if the hash.db"}],"source_content_type":"text/x-python","patch_set":6,"id":"3fa0c359_f2d037cc","line":541,"range":{"start_line":541,"start_character":26,"end_line":541,"end_character":59},"updated":"2018-02-06 21:43:41.000000000","message":"\u003e will see the new shards move\n\nyeah?","commit_id":"cf1bfe37c63279f30bcb53572e95188229990d99"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"9ca0f8847980d31a88590baaba9cfdd0ce350262","unresolved":false,"context_lines":[{"line_number":551,"context_line":"                        # view of the shards\u0027 usage, but I think it is still ok"},{"line_number":552,"context_line":"                        # to send these updates because the meta_timestamp"},{"line_number":553,"context_line":"                        # should prevent these updates undoing any newer"},{"line_number":554,"context_line":"                        # updates at the root from the actual shards. *It would"},{"line_number":555,"context_line":"                        # be good to have a test to verify that.*"},{"line_number":556,"context_line":"                        own_shard_range \u003d broker.get_own_shard_range()"},{"line_number":557,"context_line":"                        own_shard_range \u003d own_shard_range.copy("},{"line_number":558,"context_line":"                            timestamp\u003dTimestamp.now())"}],"source_content_type":"text/x-python","patch_set":6,"id":"3fa0c359_0d27c8b5","line":555,"range":{"start_line":554,"start_character":70,"end_line":555,"end_character":65},"updated":"2018-02-06 21:43:41.000000000","message":"+100 :-)","commit_id":"cf1bfe37c63279f30bcb53572e95188229990d99"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"9ca0f8847980d31a88590baaba9cfdd0ce350262","unresolved":false,"context_lines":[{"line_number":561,"context_line":"                        own_shard_range.deleted \u003d 1"},{"line_number":562,"context_line":"                        self._update_shard_ranges("},{"line_number":563,"context_line":"                            broker.root_account, broker.root_container,"},{"line_number":564,"context_line":"                            [own_shard_range] + broker.get_shard_ranges())"},{"line_number":565,"context_line":""},{"line_number":566,"context_line":"                    context \u003d get_sharding_info(broker, \u0027Context\u0027, node)"},{"line_number":567,"context_line":"                    if broker.set_sharded_state(context):"}],"source_content_type":"text/x-python","patch_set":6,"id":"3fa0c359_eda4dc0a","line":564,"range":{"start_line":564,"start_character":48,"end_line":564,"end_character":73},"updated":"2018-02-06 21:43:41.000000000","message":"Just to refresh my memory: am I right to think that shards only ever record sub-shards of their own namespace?","commit_id":"cf1bfe37c63279f30bcb53572e95188229990d99"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"7fdbf51e2307e61e5c7eaeccd241a342627aa8dc","unresolved":false,"context_lines":[{"line_number":561,"context_line":"                        own_shard_range.deleted \u003d 1"},{"line_number":562,"context_line":"                        self._update_shard_ranges("},{"line_number":563,"context_line":"                            broker.root_account, broker.root_container,"},{"line_number":564,"context_line":"                            [own_shard_range] + broker.get_shard_ranges())"},{"line_number":565,"context_line":""},{"line_number":566,"context_line":"                    context \u003d get_sharding_info(broker, \u0027Context\u0027, node)"},{"line_number":567,"context_line":"                    if broker.set_sharded_state(context):"}],"source_content_type":"text/x-python","patch_set":6,"id":"3fa0c359_e8f3639d","line":564,"updated":"2018-02-07 04:38:23.000000000","message":"Yup, unless there shrinking, but then we wont be in this state.","commit_id":"cf1bfe37c63279f30bcb53572e95188229990d99"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c1ccfdcb6f7ad0eff53167eaabb8248aeaa3de56","unresolved":false,"context_lines":[{"line_number":561,"context_line":"                        own_shard_range.deleted \u003d 1"},{"line_number":562,"context_line":"                        self._update_shard_ranges("},{"line_number":563,"context_line":"                            broker.root_account, broker.root_container,"},{"line_number":564,"context_line":"                            [own_shard_range] + broker.get_shard_ranges())"},{"line_number":565,"context_line":""},{"line_number":566,"context_line":"                    context \u003d get_sharding_info(broker, \u0027Context\u0027, node)"},{"line_number":567,"context_line":"                    if broker.set_sharded_state(context):"}],"source_content_type":"text/x-python","patch_set":6,"id":"3fa0c359_d4871961","line":564,"in_reply_to":"3fa0c359_e8f3639d","updated":"2018-02-07 09:50:24.000000000","message":"1. yes, shards only record sub shards, except when given an acceptor shard during shrinking\n2. during shrinking we do get execute this code - this is the update that causes own range is deleted in the root and acceptor to be expanded in the root\n3. but it should be safe because the shard does not modify the acceptor stats (see line 1075...) so it is just updating the root with same version of acceptor that root sent to it.","commit_id":"cf1bfe37c63279f30bcb53572e95188229990d99"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"9ca0f8847980d31a88590baaba9cfdd0ce350262","unresolved":false,"context_lines":[{"line_number":568,"context_line":"                        state \u003d DB_STATE_SHARDED"},{"line_number":569,"context_line":"                        self.logger.increment(\u0027sharding_complete\u0027)"},{"line_number":570,"context_line":"                    else:"},{"line_number":571,"context_line":"                        self.logger.debug(\u0027Remaining in sharding state %s/%s\u0027,"},{"line_number":572,"context_line":"                                          broker.account, broker.container)"},{"line_number":573,"context_line":""},{"line_number":574,"context_line":"            if state \u003d\u003d DB_STATE_SHARDED:"}],"source_content_type":"text/x-python","patch_set":6,"id":"3fa0c359_72766709","line":571,"range":{"start_line":571,"start_character":56,"end_line":571,"end_character":64},"updated":"2018-02-06 21:43:41.000000000","message":"Another possible way for set_sharded_state to fail is if we aren\u0027t *actually* in the \"sharding\" state...","commit_id":"cf1bfe37c63279f30bcb53572e95188229990d99"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"9ca0f8847980d31a88590baaba9cfdd0ce350262","unresolved":false,"context_lines":[{"line_number":573,"context_line":""},{"line_number":574,"context_line":"            if state \u003d\u003d DB_STATE_SHARDED:"},{"line_number":575,"context_line":"                if broker.is_root_container():"},{"line_number":576,"context_line":"                    if is_leader:"},{"line_number":577,"context_line":"                        self._find_shrinks(broker, node, part)"},{"line_number":578,"context_line":"            else:"},{"line_number":579,"context_line":"                if not broker.is_root_container():"}],"source_content_type":"text/x-python","patch_set":6,"id":"3fa0c359_cd94808e","line":576,"updated":"2018-02-06 21:43:41.000000000","message":"nit: These nested ifs are getting a little silly... I think we can spare an \u0027and\u0027.","commit_id":"cf1bfe37c63279f30bcb53572e95188229990d99"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"9ca0f8847980d31a88590baaba9cfdd0ce350262","unresolved":false,"context_lines":[{"line_number":986,"context_line":"                             broker.account, broker.container)"},{"line_number":987,"context_line":"        else:"},{"line_number":988,"context_line":"            # last_upper might be \u0027\u0027 if there has been no previous cleaving"},{"line_number":989,"context_line":"            # or if cleaving previously completed, i.e. the"},{"line_number":990,"context_line":"            # entire namespace was cleaved. In either case, while in state"},{"line_number":991,"context_line":"            # sharding, start cleaving from first shard range."},{"line_number":992,"context_line":"            context \u003d broker.get_sharding_context()"}],"source_content_type":"text/x-python","patch_set":6,"id":"3fa0c359_4d04f015","line":989,"range":{"start_line":989,"start_character":17,"end_line":989,"end_character":49},"updated":"2018-02-06 21:43:41.000000000","message":"In this case, we cleave the *whole thing*, all over again, yeah? Since we\u0027re already recording the current max_row when we start the first round of cleaving, could we just do the rows since then? They won\u0027t come out in a nice order, but we expect it to be a *much* shorter list, right?","commit_id":"cf1bfe37c63279f30bcb53572e95188229990d99"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c1ccfdcb6f7ad0eff53167eaabb8248aeaa3de56","unresolved":false,"context_lines":[{"line_number":986,"context_line":"                             broker.account, broker.container)"},{"line_number":987,"context_line":"        else:"},{"line_number":988,"context_line":"            # last_upper might be \u0027\u0027 if there has been no previous cleaving"},{"line_number":989,"context_line":"            # or if cleaving previously completed, i.e. the"},{"line_number":990,"context_line":"            # entire namespace was cleaved. In either case, while in state"},{"line_number":991,"context_line":"            # sharding, start cleaving from first shard range."},{"line_number":992,"context_line":"            context \u003d broker.get_sharding_context()"}],"source_content_type":"text/x-python","patch_set":6,"id":"3fa0c359_ef0842da","line":989,"range":{"start_line":989,"start_character":17,"end_line":989,"end_character":49},"in_reply_to":"3fa0c359_4d04f015","updated":"2018-02-07 09:50:24.000000000","message":"There\u0027s likely other optimisations too, for example check the context after every shard range has cleaved and reset if context has changed, rather than waiting until all ranges are cleaved.\n\nBut I don\u0027t expect this to be a common case, at least not currently.","commit_id":"cf1bfe37c63279f30bcb53572e95188229990d99"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"9ca0f8847980d31a88590baaba9cfdd0ce350262","unresolved":false,"context_lines":[{"line_number":1064,"context_line":"                # Do not do the same when sharding a shard because we\u0027re not"},{"line_number":1065,"context_line":"                # ready to handover donor namespace until all shards are"},{"line_number":1066,"context_line":"                # cleaved."},{"line_number":1067,"context_line":"            if (shard_range.state !\u003d ShardRange.ACTIVE and"},{"line_number":1068,"context_line":"                    (broker.is_root_container() or"},{"line_number":1069,"context_line":"                     own_shard_range.includes(shard_range))):"},{"line_number":1070,"context_line":"                # TODO: unit test scenario when this condition is not met"}],"source_content_type":"text/x-python","patch_set":6,"id":"3fa0c359_8dbc3806","line":1067,"range":{"start_line":1067,"start_character":16,"end_line":1067,"end_character":54},"updated":"2018-02-06 21:43:41.000000000","message":"Why\u0027d we pick up this added condition?","commit_id":"cf1bfe37c63279f30bcb53572e95188229990d99"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c1ccfdcb6f7ad0eff53167eaabb8248aeaa3de56","unresolved":false,"context_lines":[{"line_number":1064,"context_line":"                # Do not do the same when sharding a shard because we\u0027re not"},{"line_number":1065,"context_line":"                # ready to handover donor namespace until all shards are"},{"line_number":1066,"context_line":"                # cleaved."},{"line_number":1067,"context_line":"            if (shard_range.state !\u003d ShardRange.ACTIVE and"},{"line_number":1068,"context_line":"                    (broker.is_root_container() or"},{"line_number":1069,"context_line":"                     own_shard_range.includes(shard_range))):"},{"line_number":1070,"context_line":"                # TODO: unit test scenario when this condition is not met"}],"source_content_type":"text/x-python","patch_set":6,"id":"3fa0c359_f159075a","line":1067,"range":{"start_line":1067,"start_character":16,"end_line":1067,"end_character":54},"in_reply_to":"3fa0c359_8dbc3806","updated":"2018-02-07 09:50:24.000000000","message":"See comment \u0027only do this the first time each shard range is cleaved\u0027\n\nfirst time the shard range is cleaved we have a pretty good view of the shard stats, but once it has been cleaved the actual shard container becomes the authority (it is accepting new updates). If the shard range is re-cleaved due to db change then it is no longer reasonable to assume that the stats reflect reality.","commit_id":"cf1bfe37c63279f30bcb53572e95188229990d99"}]}