Tarantool development patches archive
 help / color / mirror / Atom feed
From: Konstantin Osipov <kostja@tarantool.org>
To: tarantool-patches@freelists.org
Subject: [tarantool-patches] Re: [PATCH v5 2/2] replication: force gc to clean xdir on ENOSPC err
Date: Fri, 6 Jul 2018 20:00:23 +0300	[thread overview]
Message-ID: <20180706170023.GA29935@chai> (raw)
In-Reply-To: <31d0178970ffb16aa02d245585d6fb9e4b4ffea2.1530815780.git.k.belyavskiy@tarantool.org>

* Konstantin Belyavskiy <k.belyavskiy@tarantool.org> [18/07/05 22:56]:
> Garbage collector do not delete xlog unless replica do not notify
> master with newer vclock. This can lead to running out of disk
> space error and this is not right behaviour since it will stop the
> master.
> Fix it by forcing gc to clean xlogs for replica with highest lag.
> Add an error injection and a test.

Please rebase this patch to the latest 1.10

Please use relay_stop() as a callback to unregister the consumer.

> +void
> +gc_xdir_clean_notify()
> +{
> +	/*
> +	 * Compare the current time with the time of the last run.
> +	 * This is needed in case of multiple failures to prevent
> +	 * from deleting all replicas.


> +	 */
> +	static double prev_time = 0.;
> +	double cur_time = ev_monotonic_time();
> +	if (cur_time - prev_time < 1.)
> +		return;

This throttles gc, which is good. But we would still get a lot of messages
from WAL thread. Maybe we should move the throttling to the WAL
side? This would spare us from creating the message as well.
Ideally we should use a single statically allocated message from
the WAL for this purpose (but still throttle it as well).

Plus, eventually you're going to reach a state when kicking off
replicas doesn't help with space. In this case you're going to
have a lot of messages, and they are going to be all useless.
This also suggests that throttling should be done on the WAL side.

> +	prev_time = cur_time;
> +	struct gc_consumer *leftmost =
> +	    gc_tree_first(&gc.consumers);
> +	/*
> +	 * Exit if no consumers left or if this consumer is
> +	 * not associated with replica (backup for example).
> +	 */

> +	if (leftmost == NULL || leftmost->replica == NULL)
> +		return;


> +	/*
> +	 * We have to maintain @checkpoint_count oldest snapshots,
> +	 * plus we can't remove snapshots that are still in use.
> +	 * So if leftmost replica has signature greater or equel
> +	 * then the oldest checkpoint that must be preserved,
> +	 * nothing to do.
> +	 */

This comment is useful, but the search in checkpoint array is not.
What about possible other types of consumers which are not
dispensable with anyway, e.g. backups? What if they are holding a
reference as well? 

Apparently this check is taking care of the problem:

> +	if (leftmost == NULL || leftmost->replica == NULL)
> +		return;

Could you write a test with two 
"abandoned" replicas, each holding an xlog file?


-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov

  reply	other threads:[~2018-07-06 17:00 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-05 18:39 [tarantool-patches] [PATCH v5 0/2] force gc on running out of disk space Konstantin Belyavskiy
2018-07-05 18:39 ` [tarantool-patches] [PATCH v5 1/2] replication: rename thread from tx to tx_prio Konstantin Belyavskiy
2018-07-05 18:39 ` [tarantool-patches] [PATCH v5 2/2] replication: force gc to clean xdir on ENOSPC err Konstantin Belyavskiy
2018-07-06 17:00   ` Konstantin Osipov [this message]
2018-07-10 15:10     ` [tarantool-patches] Re: [tarantool-patches] " Konstantin Belyavskiy
2018-07-10 18:37       ` Konstantin Osipov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180706170023.GA29935@chai \
    --to=kostja@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='[tarantool-patches] Re: [PATCH v5 2/2] replication: force gc to clean xdir on ENOSPC err' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox