From: Vladimir Davydov <vdavydov.dev@gmail.com> To: kostja@tarantool.org Cc: tarantool-patches@freelists.org Subject: Re: [PATCH 12/12] vinyl: allow to modify format of non-empty spaces Date: Mon, 9 Apr 2018 11:24:36 +0300 [thread overview] Message-ID: <20180409082436.uhl7j2lvcxfbkuzk@esperanza> (raw) In-Reply-To: <d274c9290186f6431572729a2ca03c2286deb561.1523105106.git.vdavydov.dev@gmail.com> On Sat, Apr 07, 2018 at 04:38:09PM +0300, Vladimir Davydov wrote: > static int > vinyl_space_check_format(struct space *space, struct tuple_format *format) > { > - (void)format; > struct vy_env *env = vy_env(space->engine); > - if (space->index_count == 0) > + > + /* > + * If this is local recovery, the space was checked before > + * restart so there's nothing we need to do. > + */ > + if (env->status == VINYL_INITIAL_RECOVERY_LOCAL || > + env->status == VINYL_FINAL_RECOVERY_LOCAL) > return 0; > + > + if (space->index_count == 0) > + return 0; /* space is empty, nothing to do */ > + > + /* > + * Iterate over all tuples stored in the given space and > + * check each of them for conformity to the new format. > + * Since read iterator may yield, we install an on_replace > + * trigger to check tuples inserted after we started the > + * iteration. > + */ > struct vy_lsm *pk = vy_lsm(space->index[0]); > - if (env->status != VINYL_ONLINE) > - return 0; > - if (pk->stat.disk.count.rows == 0 && pk->stat.memory.count.rows == 0) > - return 0; > - diag_set(ClientError, ER_UNSUPPORTED, "Vinyl", > - "changing format of a non-empty space"); > - return -1; > + > + struct tuple *key = vy_stmt_new_select(pk->env->key_format, NULL, 0); > + if (key == NULL) > + return -1; > + > + struct trigger on_replace; > + struct vy_check_format_ctx ctx; > + ctx.format = format; > + ctx.is_failed = false; > + diag_create(&ctx.diag); > + trigger_create(&on_replace, vy_check_format_on_replace, &ctx, NULL); > + trigger_add(&space->on_replace, &on_replace); > + > + struct vy_read_iterator itr; > + vy_read_iterator_open(&itr, pk, NULL, ITER_ALL, key, > + &env->xm->p_global_read_view); > + int rc; > + struct tuple *tuple; > + while ((rc = vy_read_iterator_next(&itr, &tuple)) == 0) { > + if (tuple == NULL) > + break; > + if (ctx.is_failed) { > + diag_move(&ctx.diag, diag_get()); > + rc = -1; > + break; > + } This check should be before the EOF check (tuple == NULL), because even in case read iterator returned NULL it may have yield, which might have opened a time window for a concurrent fiber to insert a new tuple that doesn't match the new format. Fixed on the branch. The incremental diff is in the end of this email. > + rc = tuple_validate(format, tuple); > + if (rc != 0) > + break; > + } > + vy_read_iterator_close(&itr); > + diag_destroy(&ctx.diag); > + trigger_clear(&on_replace); > + tuple_unref(key); > + return rc; > } diff --git a/src/box/vinyl.c b/src/box/vinyl.c index c2769e6d..6bed09da 100644 --- a/src/box/vinyl.c +++ b/src/box/vinyl.c @@ -1104,13 +1104,13 @@ vinyl_space_check_format(struct space *space, struct tuple_format *format) int rc; struct tuple *tuple; while ((rc = vy_read_iterator_next(&itr, &tuple)) == 0) { - if (tuple == NULL) - break; if (ctx.is_failed) { diag_move(&ctx.diag, diag_get()); rc = -1; break; } + if (tuple == NULL) + break; rc = tuple_validate(format, tuple); if (rc != 0) break;
next prev parent reply other threads:[~2018-04-09 8:24 UTC|newest] Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-04-07 13:37 [PATCH 00/12] " Vladimir Davydov 2018-04-07 13:37 ` [PATCH 01/12] alter: introduce CheckSpaceFormat AlterSpaceOp for validating format Vladimir Davydov 2018-04-09 20:25 ` Konstantin Osipov 2018-04-07 13:37 ` [PATCH 02/12] alter: fold ModifySpaceFormat into ModifySpace Vladimir Davydov 2018-04-09 20:26 ` Konstantin Osipov 2018-04-07 13:38 ` [PATCH 03/12] alter: move dictionary update from ModifySpace::alter_def to alter Vladimir Davydov 2018-04-09 20:32 ` Konstantin Osipov 2018-04-10 7:53 ` Vladimir Davydov 2018-04-10 11:45 ` Vladimir Davydov 2018-04-07 13:38 ` [PATCH 04/12] alter: use space_index instead of index_find where appropriate Vladimir Davydov 2018-04-09 20:34 ` Konstantin Osipov 2018-04-07 13:38 ` [PATCH 05/12] alter: allocate triggers before the point of no return Vladimir Davydov 2018-04-09 20:36 ` Konstantin Osipov 2018-04-10 7:57 ` Vladimir Davydov 2018-04-10 11:54 ` Vladimir Davydov 2018-04-07 13:38 ` [PATCH 06/12] space: space_vtab::build_secondary_key => build_index Vladimir Davydov 2018-04-09 20:39 ` Konstantin Osipov 2018-04-10 8:05 ` Vladimir Davydov 2018-04-10 12:14 ` Vladimir Davydov 2018-04-07 13:38 ` [PATCH 07/12] space: pass new format instead of new space to space_vtab::check_format Vladimir Davydov 2018-04-09 20:40 ` Konstantin Osipov 2018-04-07 13:38 ` [PATCH 08/12] alter: introduce preparation phase Vladimir Davydov 2018-04-09 20:46 ` [tarantool-patches] " Konstantin Osipov 2018-04-10 8:31 ` Vladimir Davydov 2018-04-10 8:46 ` Konstantin Osipov 2018-04-07 13:38 ` [PATCH 09/12] alter: zap space_def_check_compatibility Vladimir Davydov 2018-04-09 20:49 ` Konstantin Osipov 2018-04-07 13:38 ` [PATCH 10/12] vinyl: remove superfluous ddl checks Vladimir Davydov 2018-04-09 20:49 ` Konstantin Osipov 2018-04-07 13:38 ` [PATCH 11/12] vinyl: force index rebuild if indexed field type is narrowed Vladimir Davydov 2018-04-09 20:51 ` Konstantin Osipov 2018-04-07 13:38 ` [PATCH 12/12] vinyl: allow to modify format of non-empty spaces Vladimir Davydov 2018-04-09 8:24 ` Vladimir Davydov [this message] 2018-04-09 20:55 ` 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=20180409082436.uhl7j2lvcxfbkuzk@esperanza \ --to=vdavydov.dev@gmail.com \ --cc=kostja@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='Re: [PATCH 12/12] vinyl: allow to modify format of non-empty spaces' \ /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