tarantool / vshard

The new generation of sharding based on virtual buckets
Other
98 stars 29 forks source link

storage: do bucket transitions checks on replace #379

Closed Serpentian closed 1 year ago

Serpentian commented 1 year ago

After https://github.com/tarantool/vshard/issues/173 buckets will be protected from GC if replicas have refs, and they can't change to a bad status if there are running requests. For example, a bucket can't be deleted if it has any refs.

However it seems like a couple of replicas might get same tuple in _bucket in different states:

This is of course a result of multiple sequential bad reconfiguration steps. But this particular case could be detected.

As a follow up to https://github.com/tarantool/vshard/issues/173 it might be feasible to validate bucket state transitions in _bucket:on_replace() trigger. This transition must be one of these:

* none -> RECEIVING
* RECEIVING -> GARBAGE
* RECEIVING -> ACTIVE
* PINNED <-> ACTIVE <-> SENDING -> SENT -> GARBAGE -> none

Buckets now are protected with on_replace trigger, which also prohibits the creation of the ACTIVE buckets from nowhere, as this is done only for bootstrap and not during the normal work of vshard. We should either allow transition from none to ACTIVE or create them like that: none -> RECEIVING-> ACTIVE. We cannot disable protection and create ACTIVE buckets as we don't know how much bootstrap will take on the other instances, where protection must be disabled too.

Going through the RECEIVING status during bootstrap seems to be more preferable way of solving the problem, rather then allowing creating ACTIVE buckets. We should reduce the chance of creating unwanted buckets as e.g. if at least one extra bucket is be created in cluster, rebalancing process stops.

Closes https://github.com/tarantool/vshard/issues/377

NO_DOC=internal change

Serpentian commented 1 year ago

The test fails due to https://github.com/tarantool/tarantool/issues/7487. Its failing isn't related to the changes of the patch as it fails on the current master too (we use release, not debug, version in CI, so it doesn't fail on assertion in this PR's CI checks).