<HTML><BODY><p style="font-family: Arial, Tahoma, Verdana, sans-serif;" data-mce-style="font-family: Arial, Tahoma, Verdana, sans-serif;">Hi!<br><br>Thanks for your review.</p><p style="font-family: Arial, Tahoma, Verdana, sans-serif;" data-mce-style="font-family: Arial, Tahoma, Verdana, sans-serif;">In v5 I will add new patch to this patchset to wrap <span style="font-family: Arial, Tahoma, Verdana, sans-serif;" data-mce-style="font-family: Arial, Tahoma, Verdana, sans-serif;">std-originated </span>exceptions.</p><p style="font-family: Arial, Tahoma, Verdana, sans-serif;" data-mce-style="font-family: Arial, Tahoma, Verdana, sans-serif;"><br><span style="color: #222222;font-family: Rubik, Arial, sans-serif;font-size: 17px;" data-mce-style="color: #222222; font-family: Rubik, Arial, sans-serif; font-size: 17px;">Sincerely,<br></span>Ilya Kosarev</p><br><br><br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;">
        Вторник,  8 октября 2019, 22:53 +03:00 от Sergey Ostanevich <sergos@tarantool.org>:<br>
        <br>
        <div id="">






<div class="js-helper js-readmsg-msg">
        <style type="text/css"></style>
        <div>
                
                
            <div id="style_15705644310118142131_BODY">Hi Ilya!<br>
<br>
Thanks for the patch.<br>
Please, put into the follow-up list fixes for std:: sources of<br>
exceptions, such as std::bad_alloc that can appear in<br>
on_replace_dd_space() at alter.cc:2135 and in functions called from it.<br>
The same is applicalbe to all functions you changed.<br>
<br>
With the above taken into account - LGTM.<br>
<br>
Sergos<br>
<br>
On 23 Sep 18:56, Ilya Kosarev wrote:<br>
> Some functions called from triggers won't be cleared from<br>
> exceptions within this patchset. They are wrapped in try..catch<br>
> blocks.<br>
> <br>
> Part of #4247<br>
> ---<br>
>  src/box/alter.cc       | 58 ++++++++++++++++++++++++++++++++++--------<br>
>  src/box/replication.cc | 12 ++++++---<br>
>  2 files changed, 55 insertions(+), 15 deletions(-)<br>
> <br>
> diff --git a/src/box/alter.cc b/src/box/alter.cc<br>
> index e21dce5bf..2c1c31023 100644<br>
> --- a/src/box/alter.cc<br>
> +++ b/src/box/alter.cc<br>
> @@ -878,8 +878,12 @@ alter_space_commit(struct trigger *trigger, void *event)<br>
>     * indexes into their new places.<br>
>     */<br>
>    class AlterSpaceOp *op;<br>
> -  rlist_foreach_entry(op, &alter->ops, link)<br>
> -          op->commit(alter, signature);<br>
> +  try {<br>
> +          rlist_foreach_entry(op, &alter->ops, link)<br>
> +                  op->commit(alter, signature);<br>
> +  } catch (Exception *e) {<br>
> +          return -1;<br>
> +  }<br>
>  <br>
>    alter->new_space = NULL; /* for alter_space_delete(). */<br>
>    /*<br>
> @@ -906,8 +910,12 @@ alter_space_rollback(struct trigger *trigger, void * /* event */)<br>
>    struct alter_space *alter = (struct alter_space *) trigger->data;<br>
>    /* Rollback alter ops */<br>
>    class AlterSpaceOp *op;<br>
> -  rlist_foreach_entry(op, &alter->ops, link) {<br>
> -          op->rollback(alter);<br>
> +  try {<br>
> +          rlist_foreach_entry(op, &alter->ops, link) {<br>
> +                  op->rollback(alter);<br>
> +          }<br>
> +  } catch (Exception *e) {<br>
> +          return -1;<br>
>    }<br>
>    /* Rebuild index maps once for all indexes. */<br>
>    space_fill_index_map(alter->old_space);<br>
> @@ -2132,7 +2140,11 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)<br>
>            alter_space_move_indexes(alter, 0, old_space->index_id_max + 1);<br>
>            /* Remember to update schema_version. */<br>
>            (void) new UpdateSchemaVersion(alter);<br>
> -          alter_space_do(stmt, alter);<br>
> +          try {<br>
> +                  alter_space_do(stmt, alter);<br>
> +          } catch (Exception *e) {<br>
> +                  return -1;<br>
> +          }<br>
>            alter_guard.is_active = false;<br>
>    }<br>
>    return 0;<br>
> @@ -2371,7 +2383,11 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event)<br>
>    (void) new MoveCkConstraints(alter);<br>
>    /* Add an op to update schema_version on commit. */<br>
>    (void) new UpdateSchemaVersion(alter);<br>
> -  alter_space_do(stmt, alter);<br>
> +  try {<br>
> +          alter_space_do(stmt, alter);<br>
> +  } catch (Exception *e) {<br>
> +          return -1;<br>
> +  }<br>
>    scoped_guard.is_active = false;<br>
>    return 0;<br>
>  }<br>
> @@ -2448,7 +2464,11 @@ on_replace_dd_truncate(struct trigger * /* trigger */, void *event)<br>
>    }<br>
>  <br>
>    (void) new MoveCkConstraints(alter);<br>
> -  alter_space_do(stmt, alter);<br>
> +  try {<br>
> +          alter_space_do(stmt, alter);<br>
> +  } catch (Exception *e) {<br>
> +          return -1;<br>
> +  }<br>
>    scoped_guard.is_active = false;<br>
>    return 0;<br>
>  }<br>
> @@ -2611,7 +2631,11 @@ user_cache_alter_user(struct trigger *trigger, void * /* event */)<br>
>    struct user_def *user = user_def_new_from_tuple(tuple);<br>
>    auto def_guard = make_scoped_guard([=] { free(user); });<br>
>    /* Can throw if, e.g. too many users. */<br>
> -  user_cache_replace(user);<br>
> +  try {<br>
> +          user_cache_replace(user);<br>
> +  } catch (Exception *e) {<br>
> +          return -1;<br>
> +  }<br>
>    def_guard.is_active = false;<br>
>    return 0;<br>
>  }<br>
> @@ -2635,7 +2659,11 @@ on_replace_dd_user(struct trigger * /* trigger */, void *event)<br>
>            access_check_ddl(user->name, user->uid, user->owner, user->type,<br>
>                             PRIV_C);<br>
>            auto def_guard = make_scoped_guard([=] { free(user); });<br>
> -          (void) user_cache_replace(user);<br>
> +          try {<br>
> +                  (void) user_cache_replace(user);<br>
> +          } catch (Exception *e) {<br>
> +                  return -1;<br>
> +          }<br>
>            def_guard.is_active = false;<br>
>            struct trigger *on_rollback =<br>
>                    txn_alter_trigger_new(user_cache_remove_user, new_tuple);<br>
> @@ -2673,7 +2701,11 @@ on_replace_dd_user(struct trigger * /* trigger */, void *event)<br>
>            access_check_ddl(user->name, user->uid, user->uid,<br>
>                             old_user->def->type, PRIV_A);<br>
>            auto def_guard = make_scoped_guard([=] { free(user); });<br>
> -          user_cache_replace(user);<br>
> +          try {<br>
> +                  user_cache_replace(user);<br>
> +          } catch (Exception *e) {<br>
> +                  return -1;<br>
> +          }<br>
>            def_guard.is_active = false;<br>
>            struct trigger *on_rollback =<br>
>                    txn_alter_trigger_new(user_cache_alter_user, old_tuple);<br>
> @@ -4872,7 +4904,11 @@ on_replace_dd_func_index(struct trigger *trigger, void *event)<br>
>                             space->index_id_max + 1);<br>
>    (void) new MoveCkConstraints(alter);<br>
>    (void) new UpdateSchemaVersion(alter);<br>
> -  alter_space_do(stmt, alter);<br>
> +  try {<br>
> +          alter_space_do(stmt, alter);<br>
> +  } catch (Exception *e) {<br>
> +          return -1;<br>
> +  }<br>
>  <br>
>    scoped_guard.is_active = false;<br>
>    return 0;<br>
> diff --git a/src/box/replication.cc b/src/box/replication.cc<br>
> index 030221457..82bf496c8 100644<br>
> --- a/src/box/replication.cc<br>
> +++ b/src/box/replication.cc<br>
> @@ -415,10 +415,14 @@ replica_on_applier_state_f(struct trigger *trigger, void *event)<br>
>            replicaset.is_joining = false;<br>
>            break;<br>
>    case APPLIER_CONNECTED:<br>
> -          if (tt_uuid_is_nil(&replica->uuid))<br>
> -                  replica_on_applier_connect(replica);<br>
> -          else<br>
> -                  replica_on_applier_reconnect(replica);<br>
> +          try {<br>
> +                  if (tt_uuid_is_nil(&replica->uuid))<br>
> +                          replica_on_applier_connect(replica);<br>
> +                  else<br>
> +                          replica_on_applier_reconnect(replica);<br>
> +          } catch (Exception *e) {<br>
> +                  return -1;<br>
> +          }<br>
>            break;<br>
>    case APPLIER_LOADING:<br>
>    case APPLIER_DISCONNECTED:<br>
> -- <br>
> 2.17.1<br>
> <br>
> <br>
<br>
</div>
            
        
                
        </div>

        
</div>


</div>
</blockquote>
<br>
<br>-- <br>Ilya Kosarev<br></BODY></HTML>