[tarantool-patches] Re: [PATCH v2 1/6] xlog: store prev vclock in xlog header

  • From: Konstantin Osipov <kostja@xxxxxxxxxxxxx>
  • To: Vladimir Davydov <vdavydov.dev@xxxxxxxxx>
  • Date: Thu, 5 Jul 2018 14:22:28 +0300

* Vladimir Davydov <vdavydov.dev@xxxxxxxxx> [18/07/05 14:01]:

On Thu, Jul 05, 2018 at 09:52:41AM +0300, Konstantin Osipov wrote:
* Vladimir Davydov <vdavydov.dev@xxxxxxxxx> [18/06/29 19:49]:
This patch adds a new key to xlog header, PrevVclock, which contains the
vclock of the previous xlog file in the xlog directory. It is set by
xdir_create_xlog() to the last vclock in xdir::index. The new key is
only present in XLOG files (it doesn't make sense for SNAP or VYLOG
anyway). It will be used to make the check for xlog gaps more thorough.

What about the following diff:

---
diff --git a/src/box/xlog.c b/src/box/xlog.c
index 59459b25d..a60f2ffa8 100644
--- a/src/box/xlog.c
+++ b/src/box/xlog.c
@@ -126,12 +126,10 @@ xlog_meta_format(const struct xlog_meta *meta, char 
*buf, int size)
            VCLOCK_KEY ": %s\n",
            meta->filetype, v13, PACKAGE_VERSION, instance_uuid, vstr);
    free(vstr);
-   if (meta->has_prev_vclock) {
-           vstr = vclock_to_string(&meta->prev_vclock);
-           SNPRINT(total, snprintf, buf, size,
-                   PREV_VCLOCK_KEY ": %s\n", vstr);
-           free(vstr);
-   }
+   vstr = vclock_to_string(&meta->prev_vclock);
+   SNPRINT(total, snprintf, buf, size,
+           PREV_VCLOCK_KEY ": %s\n", vstr);
+   free(vstr);
    SNPRINT(total, snprintf, buf, size, "\n");
    assert(total > 0);
    return total;
@@ -263,7 +261,6 @@ xlog_meta_parse(struct xlog_meta *meta, const char 
**data,
                     */
                    if (parse_vclock(val, val_end, &meta->prev_vclock) != 0)
                            return -1;
-                   meta->has_prev_vclock = true;

We need to know if PrevVclock is available so that we can skip the check
when recovering from an xlog generated by a previous version.

In theory we could use Version key for this.


            } else if (memcmp(key, VERSION_KEY, key_end - key) == 0) {
                    /* Ignore Version: for now */
            } else {
@@ -926,12 +923,10 @@ xdir_create_xlog(struct xdir *dir, struct xlog *xlog,
     * For WAL dir: store vclock of the previous xlog file
     * to check for gaps on recovery.
     */
-   if (dir->type == XLOG && !vclockset_empty(&dir->index)) {
+   if (!vclockset_empty(&dir->index)) {
            vclock_copy(&meta.prev_vclock, vclockset_last(&dir->index));
-           meta.has_prev_vclock = true;
    } else {
            vclock_create(&meta.prev_vclock);
-           meta.has_prev_vclock = false;
    }

Storing vclock in snap and vylog files doesn't make any sense IMHO.
I'd prefer to check dir->type explicitly.

If you dislike the extra variable, we can probably introduce a helper to
check if vclock is set, something like

      static inline bool
      vclock_is_set(const struct vclock *vclock)
      {
              return vclock->signature >= 0;
      }

This is better IMHO.


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

Other related posts: