Tarantool development patches archive
 help / color / mirror / Atom feed
From: Konstantin Osipov <kostja@tarantool.org>
To: Vladimir Davydov <vdavydov.dev@gmail.com>
Cc: tarantool-patches@freelists.org
Subject: Re: [PATCH 3/6] Don't take schema lock for checkpointing
Date: Wed, 3 Jul 2019 22:21:53 +0300	[thread overview]
Message-ID: <20190703192153.GF17318@atlas> (raw)
In-Reply-To: <2b12ec00f416bbb2ab214c5a0ddfc373375ed038.1561922496.git.vdavydov.dev@gmail.com>

* Vladimir Davydov <vdavydov.dev@gmail.com> [19/07/01 10:04]:
> Memtx checkpointing proceeds as follows: first we open iterators over
> primary indexes of all spaces and save them to a list, then we start
> a thread that uses the iterators to dump space contents to a snap file.
> To avoid accessing a freed tuple, we put the small allocator to the
> delayed free mode. However, this doesn't prevent an index from being
> dropped so we also take the schema lock to lock out any DDL operation
> that can potentially destroy a space or an index. Note, vinyl doesn't
> need this lock, because it implements index reference counting under
> the hood.
> 
> Actually, we don't really need to take a lock - instead we can simply
> postpone index destruction until checkpointing is complete, similarly
> to how we postpone destruction of individual tuples. We even have all
> the infrastructure for this - it's delayed garbage collection. So this
> patch tweaks it a bit to delay the actual index destruction to be done
> after checkpointing is complete.
> 
> This is a step forward towards removal of the schema lock, which stands
> in the way of transactional DDL.

Looks like you do it because I said once I hate reference
counting.

First, I don't mind having reference counting for memtx index objects now
that we've approached transactional ddl frontier.

But even reference counting would be a bit cumbersome for this.
Please take a look at how bps does it - it links all pages into a
fifo-like list, and a checkpoint simply sets a savepoint on that
list. Committing a checkpoint releases the savepoint and garbage
collects all objects that have been freed after the savepoint. 

SQL will need multiple concurrent snapshots - so it will need
multiple versions. So please consider turning the algorithm you've
just used a general-purpose one - so that any database object
could add itself for delayed destruction, the delayed destruction
would take place immediately if there are no savepoints, or
immediately after the savepoint up until the next savepoint.

Then we can move all subsystems to this.

If you have a better general-purpose object garbage collection
idea, please share/implement it too.


-- 
Konstantin Osipov, Moscow, Russia

  reply	other threads:[~2019-07-03 19:21 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-30 19:40 [PATCH 0/6] Get rid of the schema lock Vladimir Davydov
2019-06-30 19:40 ` [PATCH 1/6] Add ERROR_INJECT_YIELD and ERROR_INJECT_SLEEP helpers Vladimir Davydov
2019-07-03 19:12   ` Konstantin Osipov
2019-07-04 15:50     ` Vladimir Davydov
2019-06-30 19:40 ` [PATCH 2/6] Replace ERRINJ_SNAP_WRITE_ROW_TIMEOUT with ERRINJ_SNAP_WRITE_DELAY Vladimir Davydov
2019-07-03 19:13   ` Konstantin Osipov
2019-07-04 15:51     ` Vladimir Davydov
2019-06-30 19:40 ` [PATCH 3/6] Don't take schema lock for checkpointing Vladimir Davydov
2019-07-03 19:21   ` Konstantin Osipov [this message]
2019-07-03 20:05     ` Vladimir Davydov
2019-06-30 19:40 ` [PATCH 4/6] test: make vinyl/replica_rejoin more stable Vladimir Davydov
2019-07-03 19:23   ` Konstantin Osipov
2019-07-04 15:51     ` Vladimir Davydov
2019-06-30 19:40 ` [PATCH 5/6] vinyl: don't yield while logging index creation Vladimir Davydov
2019-06-30 19:40 ` [PATCH 6/6] Replace schema lock with fine-grained locking Vladimir Davydov
2019-07-03 19:35   ` Konstantin Osipov
2019-07-03 19:56     ` Vladimir Davydov
2019-07-04  8:09       ` Konstantin Osipov
2019-07-04 17:06         ` Vladimir Davydov
2019-07-08  7:40           ` Konstantin Osipov
2019-07-08  8:41             ` Vladimir Davydov
2019-07-05  8:53 ` [PATCH 0/6] Get rid of the schema lock Vladimir Davydov

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=20190703192153.GF17318@atlas \
    --to=kostja@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --cc=vdavydov.dev@gmail.com \
    --subject='Re: [PATCH 3/6] Don'\''t take schema lock for checkpointing' \
    /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