From: Serge Petrenko <sergepetrenko@tarantool.org> To: tarantool-patches@freelists.org Cc: Vladimir Davydov <vdavydov.dev@gmail.com> Subject: Re: [tarantool-patches] [PATCH 1/2] memtx: add yields during space format check Date: Mon, 3 Jun 2019 18:54:04 +0300 [thread overview] Message-ID: <E547D37C-E53D-452D-AA11-9B99A1A47AB6@tarantool.org> (raw) In-Reply-To: <c1d8dd242b2f5da84de40ffee1c8cc4a938703f4.1559576874.git.sergepetrenko@tarantool.org> > 3 июня 2019 г., в 18:51, Serge Petrenko <sergepetrenko@tarantool.org> написал(а): > > Just like index build, space check in memtx stalls event loop for the > whole check time. Add occasional yields, and an on_replace trigger, > which checks format for tuples inserted while space format check is in > progress. > > Follow-up #3976 > > @TarantoolBol document «TarantoolBol» -> «TarantoolBot». Sorry, fixed. > Title: memtx now checks space format in background > > There is no event loop stall when memtx engine checks space format > anymore. You may insert tuples into a space, while its new format is > being checked. If the tuples don't match the new format, format change > will be aborted. > --- > src/box/memtx_space.c | 110 ++++++++++++++++++++++++++++++++---------- > 1 file changed, 84 insertions(+), 26 deletions(-) > > diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c > index 1d209033c..a370c038a 100644 > --- a/src/box/memtx_space.c > +++ b/src/box/memtx_space.c > @@ -43,6 +43,16 @@ > #include "column_mask.h" > #include "sequence.h" > > +/* > + * Yield every 1K tuples. > + * In debug mode yield more often for testing purposes. > + */ > +#ifdef NDEBUG > +enum { MEMTX_YIELD_LOOPS = 1000 }; > +#else > +enum { MEMTX_YIELD_LOOPS = 10 }; > +#endif > + > static void > memtx_space_destroy(struct space *space) > { > @@ -814,6 +824,52 @@ memtx_space_add_primary_key(struct space *space) > return 0; > } > > +/* > + * Ongoing index build or format check state used by > + * corrseponding on_replace triggers. > + */ > +struct memtx_op_state { > + /* The index being built. */ > + struct index *index; > + /* New format to be enforced. */ > + struct tuple_format *format; > + /* Operation cursor. Marks the last processed tuple to date */ > + struct tuple *cursor; > + /* Primary key key_def to compare new tuples with cursor. */ > + struct key_def *cmp_def; > + struct diag diag; > + int rc; > +}; > + > +static void > +memtx_check_on_replace(struct trigger *trigger, void *event) > +{ > + struct txn *txn = event; > + struct memtx_op_state *state = trigger->data; > + struct txn_stmt *stmt = txn_current_stmt(txn); > + > + /* Nothing to check on DELETE. */ > + if (stmt->new_tuple == NULL) > + return; > + > + /* We have already failed. */ > + if (state->rc != 0) > + return; > + > + /* > + * Only check format for already processed part of the space, > + * all the tuples inserted below cursor will be checked by the > + * main routine later. > + */ > + if (tuple_compare(state->cursor, HINT_NONE, stmt->new_tuple, HINT_NONE, > + state->cmp_def) < 0) > + return; > + > + state->rc = tuple_validate(state->format, stmt->new_tuple); > + if (state->rc != 0) > + diag_move(diag_get(), &state->diag); > +} > + > static int > memtx_space_check_format(struct space *space, struct tuple_format *format) > { > @@ -827,8 +883,19 @@ memtx_space_check_format(struct space *space, struct tuple_format *format) > if (it == NULL) > return -1; > > + struct memtx_op_state state; > + state.format = format; > + state.cmp_def = pk->def->key_def; > + state.rc = 0; > + diag_create(&state.diag); > + > + struct trigger on_replace; > + trigger_create(&on_replace, memtx_check_on_replace, &state, NULL); > + trigger_add(&space->on_replace, &on_replace); > + > int rc; > struct tuple *tuple; > + size_t count = 0; > while ((rc = iterator_next(it, &tuple)) == 0 && tuple != NULL) { > /* > * Check that the tuple is OK according to the > @@ -837,8 +904,22 @@ memtx_space_check_format(struct space *space, struct tuple_format *format) > rc = tuple_validate(format, tuple); > if (rc != 0) > break; > + if (++count % MEMTX_YIELD_LOOPS == 0) { > + state.cursor = tuple; > + tuple_ref(state.cursor); > + fiber_sleep(0); > + tuple_unref(state.cursor); > + > + if (state.rc != 0) { > + rc = -1; > + diag_move(&state.diag, diag_get()); > + break; > + } > + } > } > iterator_delete(it); > + diag_destroy(&state.diag); > + trigger_clear(&on_replace); > return rc; > } > > @@ -870,25 +951,11 @@ memtx_init_ephemeral_space(struct space *space) > memtx_space_add_primary_key(space); > } > > -/* Ongoing index build state used by memtx_build_on_replace triggers. */ > -struct memtx_build_state { > - /* The index being built. */ > - struct index *index; > - /* New index format to be enforced. */ > - struct tuple_format *format; > - /* Index build cursor. Marks the last tuple inserted to date */ > - struct tuple *cursor; > - /* Primary key key_def to compare inserted tuples with cursor. */ > - struct key_def *cmp_def; > - struct diag diag; > - int rc; > -}; > - > static void > memtx_build_on_replace(struct trigger *trigger, void *event) > { > struct txn *txn = event; > - struct memtx_build_state *state = trigger->data; > + struct memtx_op_state *state = trigger->data; > struct txn_stmt *stmt = txn_current_stmt(txn); > > struct tuple *cmp_tuple = stmt->new_tuple != NULL ? stmt->new_tuple : > @@ -925,15 +992,6 @@ static int > memtx_space_build_index(struct space *src_space, struct index *new_index, > struct tuple_format *new_format) > { > - /* > - * Yield every 1K tuples. > - * In debug mode yield more often for testing purposes. > - */ > -#ifdef NDEBUG > - enum { YIELD_LOOPS = 1000 }; > -#else > - enum { YIELD_LOOPS = 10 }; > -#endif > /** > * If it's a secondary key, and we're not building them > * yet (i.e. it's snapshot recovery for memtx), do nothing. > @@ -959,7 +1017,7 @@ memtx_space_build_index(struct space *src_space, struct index *new_index, > if (it == NULL) > return -1; > > - struct memtx_build_state state; > + struct memtx_op_state state; > state.index = new_index; > state.format = new_format; > state.cmp_def = pk->def->key_def; > @@ -1005,7 +1063,7 @@ memtx_space_build_index(struct space *src_space, struct index *new_index, > */ > if (new_index->def->iid == 0) > tuple_ref(tuple); > - if (++count % YIELD_LOOPS == 0) { > + if (++count % MEMTX_YIELD_LOOPS == 0) { > /* > * Remember the latest inserted tuple to > * avoid processing yet to be added tuples > -- > 2.20.1 (Apple Git-117) > >
next prev parent reply other threads:[~2019-06-03 15:54 UTC|newest] Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-06-03 15:51 [PATCH 0/2] memtx: make space format check yield occasionally Serge Petrenko 2019-06-03 15:51 ` [PATCH 1/2] memtx: add yields during space format check Serge Petrenko 2019-06-03 15:54 ` Serge Petrenko [this message] 2019-06-04 15:17 ` Vladimir Davydov 2019-06-03 15:51 ` [PATCH 2/2] test: move vinyl space format test case to engine suite Serge Petrenko
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=E547D37C-E53D-452D-AA11-9B99A1A47AB6@tarantool.org \ --to=sergepetrenko@tarantool.org \ --cc=tarantool-patches@freelists.org \ --cc=vdavydov.dev@gmail.com \ --subject='Re: [tarantool-patches] [PATCH 1/2] memtx: add yields during space format check' \ /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