* [tarantool-patches] [PATCH v2 1/6] Add space address to index_replace
[not found] <cover.1531742578.git.imeevma@gmail.com>
@ 2018-07-17 9:23 ` imeevma
2018-07-17 10:18 ` [tarantool-patches] " Vladislav Shpilevoy
0 siblings, 1 reply; 2+ messages in thread
From: imeevma @ 2018-07-17 9:23 UTC (permalink / raw)
To: tarantool-patches
On error in some cases index_replace
looking for space using space_id that
is saved in index. It is wrong as
function that calls index_replace
already have that space. Also in case
of ephemeral spaces space_cache_find
doesn't work right as all ephemeral
space have id == 0.
Part of #3375.
---
Branch: https://github.com/tarantool/tarantool/compare/imeevma/gh-3375-lua-expose-ephemeral-spaces
Issue: https://github.com/tarantool/tarantool/issues/3375
src/box/index.cc | 16 +++++++++-------
src/box/index.h | 30 +++++++++++++++++-------------
src/box/memtx_bitset.c | 7 ++++---
src/box/memtx_engine.c | 6 +++---
src/box/memtx_hash.c | 12 +++++-------
src/box/memtx_rtree.c | 7 ++++---
src/box/memtx_space.c | 13 +++++++------
src/box/memtx_tree.c | 16 ++++++++--------
8 files changed, 57 insertions(+), 50 deletions(-)
diff --git a/src/box/index.cc b/src/box/index.cc
index 188995e..94dcf4b 100644
--- a/src/box/index.cc
+++ b/src/box/index.cc
@@ -510,7 +510,7 @@ index_delete(struct index *index)
}
int
-index_build(struct index *index, struct index *pk)
+index_build(struct index *index, struct space *space, struct index *pk)
{
ssize_t n_tuples = index_size(pk);
if (n_tuples < 0)
@@ -539,7 +539,7 @@ index_build(struct index *index, struct index *pk)
break;
if (tuple == NULL)
break;
- rc = index_build_next(index, tuple);
+ rc = index_build_next(index, space, tuple);
if (rc != 0)
break;
}
@@ -658,10 +658,11 @@ generic_index_get(struct index *index, const char *key,
}
int
-generic_index_replace(struct index *index, struct tuple *old_tuple,
- struct tuple *new_tuple, enum dup_replace_mode mode,
- struct tuple **result)
+generic_index_replace(struct index *index, struct space *space,
+ struct tuple *old_tuple, struct tuple *new_tuple,
+ enum dup_replace_mode mode, struct tuple **result)
{
+ (void)space;
(void)old_tuple;
(void)new_tuple;
(void)mode;
@@ -709,10 +710,11 @@ generic_index_reserve(struct index *, uint32_t)
}
int
-generic_index_build_next(struct index *index, struct tuple *tuple)
+generic_index_build_next(struct index *index, struct space *space,
+ struct tuple *tuple)
{
struct tuple *unused;
- return index_replace(index, NULL, tuple, DUP_INSERT, &unused);
+ return index_replace(index, space, NULL, tuple, DUP_INSERT, &unused);
}
void
diff --git a/src/box/index.h b/src/box/index.h
index 686e7a1..2b5357a 100644
--- a/src/box/index.h
+++ b/src/box/index.h
@@ -41,6 +41,7 @@ extern "C" {
struct tuple;
struct engine;
+struct space;
struct index;
struct index_def;
struct key_def;
@@ -411,9 +412,9 @@ struct index_vtab {
const char *key, uint32_t part_count);
int (*get)(struct index *index, const char *key,
uint32_t part_count, struct tuple **result);
- int (*replace)(struct index *index, struct tuple *old_tuple,
- struct tuple *new_tuple, enum dup_replace_mode mode,
- struct tuple **result);
+ int (*replace)(struct index *index, struct space *space,
+ struct tuple *old_tuple, struct tuple *new_tuple,
+ enum dup_replace_mode mode, struct tuple **result);
/** Create an index iterator. */
struct iterator *(*create_iterator)(struct index *index,
enum iterator_type type,
@@ -443,7 +444,8 @@ struct index_vtab {
* begin_build().
*/
int (*reserve)(struct index *index, uint32_t size_hint);
- int (*build_next)(struct index *index, struct tuple *tuple);
+ int (*build_next)(struct index *index, struct space *space,
+ struct tuple *tuple);
void (*end_build)(struct index *index);
};
@@ -504,7 +506,7 @@ index_delete(struct index *index);
/** Build this index based on the contents of another index. */
int
-index_build(struct index *index, struct index *pk);
+index_build(struct index *index, struct space *space, struct index *pk);
static inline void
index_commit_create(struct index *index, int64_t signature)
@@ -596,11 +598,12 @@ index_get(struct index *index, const char *key,
}
static inline int
-index_replace(struct index *index, struct tuple *old_tuple,
- struct tuple *new_tuple, enum dup_replace_mode mode,
- struct tuple **result)
+index_replace(struct index *index, struct space *space,
+ struct tuple *old_tuple, struct tuple *new_tuple,
+ enum dup_replace_mode mode, struct tuple **result)
{
- return index->vtab->replace(index, old_tuple, new_tuple, mode, result);
+ return index->vtab->replace(index, space, old_tuple, new_tuple, mode,
+ result);
}
static inline struct iterator *
@@ -647,9 +650,9 @@ index_reserve(struct index *index, uint32_t size_hint)
}
static inline int
-index_build_next(struct index *index, struct tuple *tuple)
+index_build_next(struct index *index, struct space *space, struct tuple *tuple)
{
- return index->vtab->build_next(index, tuple);
+ return index->vtab->build_next(index, space, tuple);
}
static inline void
@@ -674,7 +677,8 @@ int generic_index_random(struct index *, uint32_t, struct tuple **);
ssize_t generic_index_count(struct index *, enum iterator_type,
const char *, uint32_t);
int generic_index_get(struct index *, const char *, uint32_t, struct tuple **);
-int generic_index_replace(struct index *, struct tuple *, struct tuple *,
+int generic_index_replace(struct index *, struct space *,
+ struct tuple *, struct tuple *,
enum dup_replace_mode, struct tuple **);
struct snapshot_iterator *generic_index_create_snapshot_iterator(struct index *);
void generic_index_stat(struct index *, struct info_handler *);
@@ -682,7 +686,7 @@ void generic_index_compact(struct index *);
void generic_index_reset_stat(struct index *);
void generic_index_begin_build(struct index *);
int generic_index_reserve(struct index *, uint32_t);
-int generic_index_build_next(struct index *, struct tuple *);
+int generic_index_build_next(struct index *, struct space *, struct tuple *);
void generic_index_end_build(struct index *);
#if defined(__cplusplus)
diff --git a/src/box/memtx_bitset.c b/src/box/memtx_bitset.c
index a665f1a..ada6fa8 100644
--- a/src/box/memtx_bitset.c
+++ b/src/box/memtx_bitset.c
@@ -252,10 +252,11 @@ make_key(const char *field, uint32_t *key_len)
}
static int
-memtx_bitset_index_replace(struct index *base, struct tuple *old_tuple,
- struct tuple *new_tuple, enum dup_replace_mode mode,
- struct tuple **result)
+memtx_bitset_index_replace(struct index *base, struct space *space,
+ struct tuple *old_tuple, struct tuple *new_tuple,
+ enum dup_replace_mode mode, struct tuple **result)
{
+ (void)space;
struct memtx_bitset_index *index = (struct memtx_bitset_index *)base;
assert(!base->def->opts.is_unique);
diff --git a/src/box/memtx_engine.c b/src/box/memtx_engine.c
index c210afb..addc477 100644
--- a/src/box/memtx_engine.c
+++ b/src/box/memtx_engine.c
@@ -160,7 +160,7 @@ memtx_build_secondary_keys(struct space *space, void *param)
}
for (uint32_t j = 1; j < space->index_count; j++) {
- if (index_build(space->index[j], pk) < 0)
+ if (index_build(space->index[j], space, pk) < 0)
return -1;
}
@@ -448,8 +448,8 @@ memtx_engine_rollback_statement(struct engine *engine, struct txn *txn,
struct tuple *unused;
struct index *index = space->index[i];
/* Rollback must not fail. */
- if (index_replace(index, stmt->new_tuple, stmt->old_tuple,
- DUP_INSERT, &unused) != 0) {
+ if (index_replace(index, space, stmt->new_tuple,
+ stmt->old_tuple, DUP_INSERT, &unused) != 0) {
diag_log();
unreachable();
panic("failed to rollback change");
diff --git a/src/box/memtx_hash.c b/src/box/memtx_hash.c
index eae07e8..76fe35f 100644
--- a/src/box/memtx_hash.c
+++ b/src/box/memtx_hash.c
@@ -244,9 +244,9 @@ memtx_hash_index_get(struct index *base, const char *key,
}
static int
-memtx_hash_index_replace(struct index *base, struct tuple *old_tuple,
- struct tuple *new_tuple, enum dup_replace_mode mode,
- struct tuple **result)
+memtx_hash_index_replace(struct index *base, struct space *space,
+ struct tuple *old_tuple, struct tuple *new_tuple,
+ enum dup_replace_mode mode, struct tuple **result)
{
struct memtx_hash_index *index = (struct memtx_hash_index *)base;
struct light_index_core *hash_table = &index->hash_table;
@@ -281,10 +281,8 @@ memtx_hash_index_replace(struct index *base, struct tuple *old_tuple,
"recover of int hash_table");
}
}
- struct space *sp = space_cache_find(base->def->space_id);
- if (sp != NULL)
- diag_set(ClientError, errcode, base->def->name,
- space_name(sp));
+ diag_set(ClientError, errcode, base->def->name,
+ space_name(space));
return -1;
}
diff --git a/src/box/memtx_rtree.c b/src/box/memtx_rtree.c
index 0b12cda..333d191 100644
--- a/src/box/memtx_rtree.c
+++ b/src/box/memtx_rtree.c
@@ -210,10 +210,11 @@ memtx_rtree_index_get(struct index *base, const char *key,
}
static int
-memtx_rtree_index_replace(struct index *base, struct tuple *old_tuple,
- struct tuple *new_tuple, enum dup_replace_mode mode,
- struct tuple **result)
+memtx_rtree_index_replace(struct index *base, struct space *space,
+ struct tuple *old_tuple, struct tuple *new_tuple,
+ enum dup_replace_mode mode, struct tuple **result)
{
+ (void)space;
(void)mode;
struct memtx_rtree_index *index = (struct memtx_rtree_index *)base;
struct rtree_rect rect;
diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c
index b94c4ab..185be92 100644
--- a/src/box/memtx_space.c
+++ b/src/box/memtx_space.c
@@ -124,7 +124,7 @@ memtx_space_replace_build_next(struct space *space, struct tuple *old_tuple,
panic("Failed to commit transaction when loading "
"from snapshot");
}
- if (index_build_next(space->index[0], new_tuple) != 0)
+ if (index_build_next(space->index[0], space, new_tuple) != 0)
return -1;
memtx_space_update_bsize(space, NULL, new_tuple);
return 0;
@@ -140,7 +140,7 @@ memtx_space_replace_primary_key(struct space *space, struct tuple *old_tuple,
enum dup_replace_mode mode,
struct tuple **result)
{
- if (index_replace(space->index[0], old_tuple,
+ if (index_replace(space->index[0], space, old_tuple,
new_tuple, mode, &old_tuple) != 0)
return -1;
memtx_space_update_bsize(space, old_tuple, new_tuple);
@@ -259,7 +259,8 @@ memtx_space_replace_all_keys(struct space *space, struct tuple *old_tuple,
* If old_tuple is not NULL, the index has to
* find and delete it, or return an error.
*/
- if (index_replace(pk, old_tuple, new_tuple, mode, &old_tuple) != 0)
+ if (index_replace(pk, space, old_tuple,
+ new_tuple, mode, &old_tuple) != 0)
return -1;
assert(old_tuple || new_tuple);
@@ -267,7 +268,7 @@ memtx_space_replace_all_keys(struct space *space, struct tuple *old_tuple,
for (i++; i < space->index_count; i++) {
struct tuple *unused;
struct index *index = space->index[i];
- if (index_replace(index, old_tuple, new_tuple,
+ if (index_replace(index, space, old_tuple, new_tuple,
DUP_INSERT, &unused) != 0)
goto rollback;
}
@@ -281,7 +282,7 @@ rollback:
struct tuple *unused;
struct index *index = space->index[i - 1];
/* Rollback must not fail. */
- if (index_replace(index, new_tuple, old_tuple,
+ if (index_replace(index, space, new_tuple, old_tuple,
DUP_INSERT, &unused) != 0) {
diag_log();
unreachable();
@@ -898,7 +899,7 @@ memtx_space_build_index(struct space *src_space, struct index *new_index,
* @todo: better message if there is a duplicate.
*/
struct tuple *old_tuple;
- rc = index_replace(new_index, NULL, tuple,
+ rc = index_replace(new_index, src_space, NULL, tuple,
DUP_INSERT, &old_tuple);
if (rc != 0)
break;
diff --git a/src/box/memtx_tree.c b/src/box/memtx_tree.c
index f851fb8..3da52b3 100644
--- a/src/box/memtx_tree.c
+++ b/src/box/memtx_tree.c
@@ -438,9 +438,9 @@ memtx_tree_index_get(struct index *base, const char *key,
}
static int
-memtx_tree_index_replace(struct index *base, struct tuple *old_tuple,
- struct tuple *new_tuple, enum dup_replace_mode mode,
- struct tuple **result)
+memtx_tree_index_replace(struct index *base, struct space *space,
+ struct tuple *old_tuple, struct tuple *new_tuple,
+ enum dup_replace_mode mode, struct tuple **result)
{
struct memtx_tree_index *index = (struct memtx_tree_index *)base;
if (new_tuple) {
@@ -461,10 +461,8 @@ memtx_tree_index_replace(struct index *base, struct tuple *old_tuple,
memtx_tree_delete(&index->tree, new_tuple);
if (dup_tuple)
memtx_tree_insert(&index->tree, dup_tuple, 0);
- struct space *sp = space_cache_find(base->def->space_id);
- if (sp != NULL)
- diag_set(ClientError, errcode, base->def->name,
- space_name(sp));
+ diag_set(ClientError, errcode, base->def->name,
+ space_name(space));
return -1;
}
if (dup_tuple) {
@@ -549,8 +547,10 @@ memtx_tree_index_reserve(struct index *base, uint32_t size_hint)
}
static int
-memtx_tree_index_build_next(struct index *base, struct tuple *tuple)
+memtx_tree_index_build_next(struct index *base, struct space *space,
+ struct tuple *tuple)
{
+ (void)space;
struct memtx_tree_index *index = (struct memtx_tree_index *)base;
if (index->build_array == NULL) {
index->build_array = (struct tuple **)malloc(MEMTX_EXTENT_SIZE);
--
2.7.4
^ permalink raw reply [flat|nested] 2+ messages in thread
* [tarantool-patches] Re: [PATCH v2 1/6] Add space address to index_replace
2018-07-17 9:23 ` [tarantool-patches] [PATCH v2 1/6] Add space address to index_replace imeevma
@ 2018-07-17 10:18 ` Vladislav Shpilevoy
0 siblings, 0 replies; 2+ messages in thread
From: Vladislav Shpilevoy @ 2018-07-17 10:18 UTC (permalink / raw)
To: tarantool-patches, imeevma
Hello! Thanks for the patch! See 3 comments below.
1. Please, on the commit title specify a subsystem name which
the patch affected. Here it is 'bos: ' I think.
On 17/07/2018 12:23, imeevma@tarantool.org wrote:
> On error in some cases index_replace
> looking for space using space_id that
> is saved in index. It is wrong as
> function that calls index_replace
> already have that space. Also in case
> of ephemeral spaces space_cache_find
> doesn't work right as all ephemeral
> space have id == 0.
>
> Part of #3375.
> ---
> Branch: https://github.com/tarantool/tarantool/compare/imeevma/gh-3375-lua-expose-ephemeral-spaces
> Issue: https://github.com/tarantool/tarantool/issues/3375
>
> src/box/index.cc | 16 +++++++++-------
> src/box/index.h | 30 +++++++++++++++++-------------
> src/box/memtx_bitset.c | 7 ++++---
> src/box/memtx_engine.c | 6 +++---
> src/box/memtx_hash.c | 12 +++++-------
> src/box/memtx_rtree.c | 7 ++++---
> src/box/memtx_space.c | 13 +++++++------
> src/box/memtx_tree.c | 16 ++++++++--------
> 8 files changed, 57 insertions(+), 50 deletions(-)
>
> diff --git a/src/box/memtx_hash.c b/src/box/memtx_hash.c
> index eae07e8..76fe35f 100644
> --- a/src/box/memtx_hash.c
> +++ b/src/box/memtx_hash.c
> @@ -244,9 +244,9 @@ memtx_hash_index_get(struct index *base, const char *key,
> }
>
> static int
> -memtx_hash_index_replace(struct index *base, struct tuple *old_tuple,
> - struct tuple *new_tuple, enum dup_replace_mode mode,
> - struct tuple **result)
> +memtx_hash_index_replace(struct index *base, struct space *space,
> + struct tuple *old_tuple, struct tuple *new_tuple,
> + enum dup_replace_mode mode, struct tuple **result)
> {
> struct memtx_hash_index *index = (struct memtx_hash_index *)base;
> struct light_index_core *hash_table = &index->hash_table;
> @@ -281,10 +281,8 @@ memtx_hash_index_replace(struct index *base, struct tuple *old_tuple,
> "recover of int hash_table");
> }
> }
> - struct space *sp = space_cache_find(base->def->space_id);
2. Now you can remove schema.h dependency.
> - if (sp != NULL)
> - diag_set(ClientError, errcode, base->def->name,
> - space_name(sp));
> + diag_set(ClientError, errcode, base->def->name,
> + space_name(space));
> return -1;
> }
>
> diff --git a/src/box/memtx_tree.c b/src/box/memtx_tree.c
> index f851fb8..3da52b3 100644
> --- a/src/box/memtx_tree.c
> +++ b/src/box/memtx_tree.c
3. Same.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2018-07-17 10:19 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <cover.1531742578.git.imeevma@gmail.com>
2018-07-17 9:23 ` [tarantool-patches] [PATCH v2 1/6] Add space address to index_replace imeevma
2018-07-17 10:18 ` [tarantool-patches] " Vladislav Shpilevoy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox