From 73733e51331027424ddce117c09402b7d61db490 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov <vdavydov@tarantool.org> Date: Mon, 16 May 2022 15:46:50 +0300 Subject: [PATCH] alter: don't swap tuple dictionaries for online space upgrade We don't rebuild the tuples stored in the space when a space format is updated, just check that they are compatible with the new format. For the old tuple fields to be accessible by the new format field names, we point the old format dictionary to the new format dictionary (a field dictionary is a reference counted object so the same dictionary can be used by multiple formats). This isn't necessary for online space upgrade, because in this case we upgrade each tuple stored in the space to the new format before returning it to the user. Moreover, using the new dictionary for the old tuples would even be harmful in this case, because the old tuples may be incompatible with the new format, while the space upgrae function may use the old field names. Needed for https://github.com/tarantool/tarantool-ee/issues/112 NO_DOC=ee NO_TEST=ee NO_CHANGELOG=ee --- src/box/alter.cc | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/box/alter.cc b/src/box/alter.cc index 728cad6169..8c99017016 100644 --- a/src/box/alter.cc +++ b/src/box/alter.cc @@ -1080,15 +1080,23 @@ void ModifySpace::alter_def(struct alter_space *alter) { /* - * Use the old dictionary for the new space, because - * it is already referenced by existing tuple formats. + * Unless it's online space upgrade, Use the old dictionary for the new + * space, because it's already referenced by existing tuple formats. * We will update it in place in ModifySpace::alter. + * + * For online space upgrade, all tuples fetched from the space will be + * upgraded to the new format before returning to the user so it isn't + * necessary. Moreover, the old tuples may be incompatible with the new + * format so using the new dictionary for them would be wrong and could + * result in error accessing fields by name from the space upgrade + * function. */ - new_dict = new_def->dict; - new_def->dict = alter->old_space->def->dict; - tuple_dictionary_ref(new_def->dict); + if (new_def->opts.upgrade_def == NULL) { + new_dict = new_def->dict; + new_def->dict = alter->old_space->def->dict; + tuple_dictionary_ref(new_def->dict); + } new_def->view_ref_count = alter->old_space->def->view_ref_count; - space_def_delete(alter->space_def); alter->space_def = new_def; /* Now alter owns the def. */ @@ -1103,13 +1111,15 @@ ModifySpace::alter(struct alter_space *alter) * referenced by existing tuple formats. New dictionary * object is deleted later, in destructor. */ - tuple_dictionary_swap(alter->new_space->def->dict, new_dict); + if (new_dict != NULL) + tuple_dictionary_swap(alter->new_space->def->dict, new_dict); } void ModifySpace::rollback(struct alter_space *alter) { - tuple_dictionary_swap(alter->new_space->def->dict, new_dict); + if (new_dict != NULL) + tuple_dictionary_swap(alter->new_space->def->dict, new_dict); } ModifySpace::~ModifySpace() -- GitLab