[tarantool-patches] Re: [tarantool-patches] Re: [PATCH v4 02/20] refactoring: wrap incorrigibly throwing calls in try..catch block
Ilya Kosarev
i.kosarev at tarantool.org
Mon Oct 21 14:32:42 MSK 2019
Hi!
Thanks for your review.
In v5 I will add new patch to this patchset to wrap std-originated exceptions.
Sincerely,
Ilya Kosarev
>Вторник, 8 октября 2019, 22:53 +03:00 от Sergey Ostanevich <sergos at tarantool.org>:
>
>Hi Ilya!
>
>Thanks for the patch.
>Please, put into the follow-up list fixes for std:: sources of
>exceptions, such as std::bad_alloc that can appear in
>on_replace_dd_space() at alter.cc:2135 and in functions called from it.
>The same is applicalbe to all functions you changed.
>
>With the above taken into account - LGTM.
>
>Sergos
>
>On 23 Sep 18:56, Ilya Kosarev wrote:
>> Some functions called from triggers won't be cleared from
>> exceptions within this patchset. They are wrapped in try..catch
>> blocks.
>>
>> Part of #4247
>> ---
>> src/box/alter.cc | 58 ++++++++++++++++++++++++++++++++++--------
>> src/box/replication.cc | 12 ++++++---
>> 2 files changed, 55 insertions(+), 15 deletions(-)
>>
>> diff --git a/src/box/alter.cc b/src/box/alter.cc
>> index e21dce5bf..2c1c31023 100644
>> --- a/src/box/alter.cc
>> +++ b/src/box/alter.cc
>> @@ -878,8 +878,12 @@ alter_space_commit(struct trigger *trigger, void *event)
>> * indexes into their new places.
>> */
>> class AlterSpaceOp *op;
>> - rlist_foreach_entry(op, &alter->ops, link)
>> - op->commit(alter, signature);
>> + try {
>> + rlist_foreach_entry(op, &alter->ops, link)
>> + op->commit(alter, signature);
>> + } catch (Exception *e) {
>> + return -1;
>> + }
>>
>> alter->new_space = NULL; /* for alter_space_delete(). */
>> /*
>> @@ -906,8 +910,12 @@ alter_space_rollback(struct trigger *trigger, void * /* event */)
>> struct alter_space *alter = (struct alter_space *) trigger->data;
>> /* Rollback alter ops */
>> class AlterSpaceOp *op;
>> - rlist_foreach_entry(op, &alter->ops, link) {
>> - op->rollback(alter);
>> + try {
>> + rlist_foreach_entry(op, &alter->ops, link) {
>> + op->rollback(alter);
>> + }
>> + } catch (Exception *e) {
>> + return -1;
>> }
>> /* Rebuild index maps once for all indexes. */
>> space_fill_index_map(alter->old_space);
>> @@ -2132,7 +2140,11 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)
>> alter_space_move_indexes(alter, 0, old_space->index_id_max + 1);
>> /* Remember to update schema_version. */
>> (void) new UpdateSchemaVersion(alter);
>> - alter_space_do(stmt, alter);
>> + try {
>> + alter_space_do(stmt, alter);
>> + } catch (Exception *e) {
>> + return -1;
>> + }
>> alter_guard.is_active = false;
>> }
>> return 0;
>> @@ -2371,7 +2383,11 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event)
>> (void) new MoveCkConstraints(alter);
>> /* Add an op to update schema_version on commit. */
>> (void) new UpdateSchemaVersion(alter);
>> - alter_space_do(stmt, alter);
>> + try {
>> + alter_space_do(stmt, alter);
>> + } catch (Exception *e) {
>> + return -1;
>> + }
>> scoped_guard.is_active = false;
>> return 0;
>> }
>> @@ -2448,7 +2464,11 @@ on_replace_dd_truncate(struct trigger * /* trigger */, void *event)
>> }
>>
>> (void) new MoveCkConstraints(alter);
>> - alter_space_do(stmt, alter);
>> + try {
>> + alter_space_do(stmt, alter);
>> + } catch (Exception *e) {
>> + return -1;
>> + }
>> scoped_guard.is_active = false;
>> return 0;
>> }
>> @@ -2611,7 +2631,11 @@ user_cache_alter_user(struct trigger *trigger, void * /* event */)
>> struct user_def *user = user_def_new_from_tuple(tuple);
>> auto def_guard = make_scoped_guard([=] { free(user); });
>> /* Can throw if, e.g. too many users. */
>> - user_cache_replace(user);
>> + try {
>> + user_cache_replace(user);
>> + } catch (Exception *e) {
>> + return -1;
>> + }
>> def_guard.is_active = false;
>> return 0;
>> }
>> @@ -2635,7 +2659,11 @@ on_replace_dd_user(struct trigger * /* trigger */, void *event)
>> access_check_ddl(user->name, user->uid, user->owner, user->type,
>> PRIV_C);
>> auto def_guard = make_scoped_guard([=] { free(user); });
>> - (void) user_cache_replace(user);
>> + try {
>> + (void) user_cache_replace(user);
>> + } catch (Exception *e) {
>> + return -1;
>> + }
>> def_guard.is_active = false;
>> struct trigger *on_rollback =
>> txn_alter_trigger_new(user_cache_remove_user, new_tuple);
>> @@ -2673,7 +2701,11 @@ on_replace_dd_user(struct trigger * /* trigger */, void *event)
>> access_check_ddl(user->name, user->uid, user->uid,
>> old_user->def->type, PRIV_A);
>> auto def_guard = make_scoped_guard([=] { free(user); });
>> - user_cache_replace(user);
>> + try {
>> + user_cache_replace(user);
>> + } catch (Exception *e) {
>> + return -1;
>> + }
>> def_guard.is_active = false;
>> struct trigger *on_rollback =
>> txn_alter_trigger_new(user_cache_alter_user, old_tuple);
>> @@ -4872,7 +4904,11 @@ on_replace_dd_func_index(struct trigger *trigger, void *event)
>> space->index_id_max + 1);
>> (void) new MoveCkConstraints(alter);
>> (void) new UpdateSchemaVersion(alter);
>> - alter_space_do(stmt, alter);
>> + try {
>> + alter_space_do(stmt, alter);
>> + } catch (Exception *e) {
>> + return -1;
>> + }
>>
>> scoped_guard.is_active = false;
>> return 0;
>> diff --git a/src/box/replication.cc b/src/box/replication.cc
>> index 030221457..82bf496c8 100644
>> --- a/src/box/replication.cc
>> +++ b/src/box/replication.cc
>> @@ -415,10 +415,14 @@ replica_on_applier_state_f(struct trigger *trigger, void *event)
>> replicaset.is_joining = false;
>> break;
>> case APPLIER_CONNECTED:
>> - if (tt_uuid_is_nil(&replica->uuid))
>> - replica_on_applier_connect(replica);
>> - else
>> - replica_on_applier_reconnect(replica);
>> + try {
>> + if (tt_uuid_is_nil(&replica->uuid))
>> + replica_on_applier_connect(replica);
>> + else
>> + replica_on_applier_reconnect(replica);
>> + } catch (Exception *e) {
>> + return -1;
>> + }
>> break;
>> case APPLIER_LOADING:
>> case APPLIER_DISCONNECTED:
>> --
>> 2.17.1
>>
>>
>
--
Ilya Kosarev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20191021/48463f20/attachment.html>
More information about the Tarantool-patches
mailing list