<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>