Re: [tarantool-patches] Re: [PATCH 2/2] replication: fix garbage collection logic

[Vladimir Davydov <vdavydov.dev@gmail.com>]

On Thu, Apr 11, 2019 at 10:32:55AM +0300, Konstantin Osipov wrote:
> * Vladimir Davydov <vdavydov.dev@gmail.com> [19/04/09 20:09]:
> > Currently, the garbage collector works with vclock signatures and
> > doesn't take into account vclock components. This works as long as
> > the caller (i.e. relay) makes sure that it doesn't advance a consumer
> > associated with a replica unless its acknowledged vclock is greater
> > than or equal to the vclock of a WAL file fed to it. The bug is that
> > it does not - it only compares vclock signatures. As a result, if
> > a replica has some local changes or changes pulled from other members
> > of the cluster, which render its signature greater, the master may
> > remove files that are still needed by the replica, permanently breaking
> > replication and requiring rebootstrap.
> > 
> > I guess the proper fix would be teaching the garbage collector
> > operate on vclock components rather than signatures, but it's rather
> > difficult to implement. This patch is a quick fix, which simply
> > replaces vclock signature comparison in relay with vclock_compare.
> 
> This patch is OK to push. I still think we need a special compare
> function, which ignores one specified dimension, and we should
> change vclock_compare in recover_remaining_wals and this
> vclock_compare to use it.
> 
> This dimension is the server id of replica we're feeding wals to.
> The logic is that we should not bother with feeding replica its
> own changes, and depend on having these changes. This will make
> vclocks comparable even if replica has local changes, and master
> has local changes, and some of the xlogs which predate these
> changes are already missing.

The problem is the replica may have changes from other members of
the clusters, which haven't been pulled by the master yet. In other
words, it isn't as simple as ignoring just one vclock component.
In case of GC we should maintain a vclock that consists of minimal
components among all replicas' vclocks, e.g. if we have three replicas
with vclocks {1, 100, 200, 10}, {10, 50, 150, 30}, {1, 100, 100, 50},
then the gc vclock on the master (i.e. the vclock of the newest row
we can collect) must equal {1, 50, 100, 10}. This is kinda difficult to
implement - it's much easier to compare vclocks in relay upon feeding a
WAL file, which I did. However, I guess we'll have to figure out how to
do that for sync replication anyway.