From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: tarantool-patches@freelists.org, imeevma@tarantool.org,
Kirill Shcherbatov <kshcherbatov@tarantool.org>
Subject: [tarantool-patches] Re: [PATCH v2 3/7] box: create new methods for ephemeral spaces
Date: Wed, 18 Jul 2018 11:06:29 +0300 [thread overview]
Message-ID: <55802bb0-8ab5-7531-14bd-e55283c43603@tarantool.org> (raw)
In-Reply-To: <d6efda7558f2df66ef79e44627e8bcfb9e4b3a7e.1531837331.git.imeevma@gmail.com>
Kirill, please, do the second review.
On 17/07/2018 17:55, imeevma@tarantool.org wrote:
> Up to this patch any space had two additional methods
> that were methods of ephemeral spaces. In this patch
> these methods deleted from vtab and from now on
> ephemeral spaces are spaces with special vtab.
>
> 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/box.cc | 155 +++++++++++++++++-----
> src/box/box.h | 94 +++++++++++++
> src/box/memtx_space.c | 333 ++++++++++++++++++++++++++++++-----------------
> src/box/space.h | 17 ---
> src/box/sql.c | 33 ++++-
> src/box/sysview_engine.c | 22 ----
> src/box/vinyl.c | 22 ----
> 7 files changed, 459 insertions(+), 217 deletions(-)
>
> diff --git a/src/box/box.cc b/src/box/box.cc
> index 1dcbfbf..9d9f6f0 100644
> --- a/src/box/box.cc
> +++ b/src/box/box.cc
> @@ -872,6 +872,52 @@ box_set_net_msg_max(void)
> /* }}} configuration bindings */
>
> /**
> + * Fill up request according to given
> + * operation type. For all cases but
> + * UPSERT argument name match names of
> + * fields of struct request. For UPSERT
> + * arguments key and key_end are fields
> + * ops and ops_end.
> + */
> +static inline void
> +request_fill(struct request *request, uint32_t type, uint32_t space_id,
> + uint32_t index_id, const char *key, const char *key_end,
> + const char *tuple, const char *tuple_end, int index_base)
> +{
> + memset(request, 0, sizeof(*request));
> + request->type = type;
> + request->space_id = space_id;
> + request->index_id = index_id;
> + switch (type) {
> + case IPROTO_INSERT:
> + case IPROTO_REPLACE:
> + request->tuple = tuple;
> + request->tuple_end = tuple_end;
> + break;
> + case IPROTO_DELETE:
> + request->key = key;
> + request->key_end = key_end;
> + break;
> + case IPROTO_UPDATE:
> + request->key = key;
> + request->key_end = key_end;
> + request->tuple = tuple;
> + request->tuple_end = tuple_end;
> + request->index_base = index_base;
> + break;
> + case IPROTO_UPSERT:
> + request->ops = key;
> + request->ops_end = key_end;
> + request->tuple = tuple;
> + request->tuple_end = tuple_end;
> + request->index_base = index_base;
> + break;
> + default:
> + unreachable();
> + }
> +}
> +
> +/**
> * Execute a request against a given space id with
> * a variable-argument tuple described in format.
> *
> @@ -1125,11 +1171,8 @@ box_insert(uint32_t space_id, const char *tuple, const char *tuple_end,
> if (space == NULL || space_check_writable(space) != 0)
> return -1;
> struct request request;
> - memset(&request, 0, sizeof(request));
> - request.type = IPROTO_INSERT;
> - request.space_id = space_id;
> - request.tuple = tuple;
> - request.tuple_end = tuple_end;
> + request_fill(&request, IPROTO_INSERT, space_id, 0, NULL, NULL, tuple,
> + tuple_end, 0);
> return box_process_rw(&request, space, result);
> }
>
> @@ -1142,11 +1185,8 @@ box_replace(uint32_t space_id, const char *tuple, const char *tuple_end,
> if (space == NULL || space_check_writable(space) != 0)
> return -1;
> struct request request;
> - memset(&request, 0, sizeof(request));
> - request.type = IPROTO_REPLACE;
> - request.space_id = space_id;
> - request.tuple = tuple;
> - request.tuple_end = tuple_end;
> + request_fill(&request, IPROTO_REPLACE, space_id, 0, NULL, NULL, tuple,
> + tuple_end, 0);
> return box_process_rw(&request, space, result);
> }
>
> @@ -1160,12 +1200,8 @@ box_delete(uint32_t space_id, uint32_t index_id, const char *key,
> key_check_findable(space, index_id, key) != 0)
> return -1;
> struct request request;
> - memset(&request, 0, sizeof(request));
> - request.type = IPROTO_DELETE;
> - request.space_id = space_id;
> - request.index_id = index_id;
> - request.key = key;
> - request.key_end = key_end;
> + request_fill(&request, IPROTO_DELETE, space_id, index_id, key, key_end,
> + NULL, NULL, 0);
> return box_process_rw(&request, space, result);
> }
>
> @@ -1181,17 +1217,8 @@ box_update(uint32_t space_id, uint32_t index_id, const char *key,
> key_check_findable(space, index_id, key) != 0)
> return -1;
> struct request request;
> - memset(&request, 0, sizeof(request));
> - request.type = IPROTO_UPDATE;
> - request.space_id = space_id;
> - request.index_id = index_id;
> - request.key = key;
> - request.key_end = key_end;
> - request.index_base = index_base;
> - /** Legacy: in case of update, ops are passed in in request tuple */
> - request.tuple = ops;
> - request.tuple_end = ops_end;
> -
> + request_fill(&request, IPROTO_UPDATE, space_id, index_id, key, key_end,
> + ops, ops_end, index_base);
> return box_process_rw(&request, space, result);
> }
>
> @@ -1206,18 +1233,74 @@ box_upsert(uint32_t space_id, uint32_t index_id, const char *tuple,
> if (space == NULL || space_check_writable(space) != 0)
> return -1;
> struct request request;
> - memset(&request, 0, sizeof(request));
> - request.type = IPROTO_UPSERT;
> - request.space_id = space_id;
> - request.index_id = index_id;
> - request.ops = ops;
> - request.ops_end = ops_end;
> - request.tuple = tuple;
> - request.tuple_end = tuple_end;
> - request.index_base = index_base;
> + request_fill(&request, IPROTO_UPSERT, space_id, index_id, ops, ops_end,
> + tuple, tuple_end, index_base);
> return box_process_rw(&request, space, result);
> }
>
> +int
> +box_ephemeral_insert(struct space *space, const char *tuple,
> + const char *tuple_end, box_tuple_t **result)
> +{
> + mp_tuple_assert(tuple, tuple_end);
> + struct request request;
> + request_fill(&request, IPROTO_INSERT, 0, 0, NULL, NULL, tuple,
> + tuple_end, 0);
> + return space_execute_dml(space, NULL, &request, result);
> +}
> +
> +int
> +box_ephemeral_replace(struct space *space, const char *tuple,
> + const char *tuple_end, box_tuple_t **result)
> +{
> + mp_tuple_assert(tuple, tuple_end);
> + struct request request;
> + request_fill(&request, IPROTO_REPLACE, 0, 0, NULL, NULL, tuple,
> + tuple_end, 0);
> + return space_execute_dml(space, NULL, &request, result);
> +}
> +
> +int
> +box_ephemeral_delete(struct space *space, uint32_t index_id, const char *key,
> + const char *key_end, box_tuple_t **result)
> +{
> + mp_tuple_assert(key, key_end);
> + if (key_check_findable(space, index_id, key) != 0)
> + return -1;
> + struct request request;
> + request_fill(&request, IPROTO_DELETE, 0, index_id, key, key_end,
> + NULL, NULL, 0);
> + return space_execute_dml(space, NULL, &request, result);
> +}
> +
> +int
> +box_ephemeral_update(struct space *space, uint32_t index_id, const char *key,
> + const char *key_end, const char *ops, const char *ops_end,
> + int index_base, box_tuple_t **result)
> +{
> + mp_tuple_assert(key, key_end);
> + mp_tuple_assert(ops, ops_end);
> + if (key_check_findable(space, index_id, key) != 0)
> + return -1;
> + struct request request;
> + request_fill(&request, IPROTO_UPDATE, 0, index_id, key, key_end,
> + ops, ops_end, index_base);
> + return space_execute_dml(space, NULL, &request, result);
> +}
> +
> +int
> +box_ephemeral_upsert(struct space *space, uint32_t index_id, const char *tuple,
> + const char *tuple_end, const char *ops,
> + const char *ops_end, int index_base, box_tuple_t **result)
> +{
> + mp_tuple_assert(ops, ops_end);
> + mp_tuple_assert(tuple, tuple_end);
> + struct request request;
> + request_fill(&request, IPROTO_UPSERT, 0, index_id, ops, ops_end,
> + tuple, tuple_end, index_base);
> + return space_execute_dml(space, NULL, &request, result);
> +}
> +
> /**
> * Trigger space truncation by bumping a counter
> * in _truncate space.
> diff --git a/src/box/box.h b/src/box/box.h
> index 422f62f..3601823 100644
> --- a/src/box/box.h
> +++ b/src/box/box.h
> @@ -401,6 +401,100 @@ int
> box_process1(struct request *request, box_tuple_t **result);
>
> /**
> + * Execute an INSERT request for ephemeral spaces.
> + *
> + * \param space ephemeral space.
> + * \param tuple encoded tuple in
> + * MsgPack Array format.
> + * \param tuple_end end of @a tuple.
> + * \param[out] result a new tuple.
> + * \retval -1 on error.
> + * \retval 0 on success.
> + */
> +int
> +box_ephemeral_insert(struct space *space, const char *tuple,
> + const char *tuple_end, box_tuple_t **result);
> +
> +/**
> + * Execute an REPLACE request for ephemeral spaces.
> + *
> + * \param space ephemeral space.
> + * \param tuple encoded tuple in
> + * MsgPack Array format.
> + * \param tuple_end end of @a tuple.
> + * \param[out] result a new tuple.
> + * \retval -1 on error.
> + * \retval 0 on success.
> + */
> +int
> +box_ephemeral_replace(struct space *space, const char *tuple,
> + const char *tuple_end, box_tuple_t **result);
> +
> +/**
> + * Execute an DELETE request for ephemeral spaces.
> + *
> + * \param space ephemeral space.
> + * \param index_id index identifier
> + * \param key encoded key in
> + * MsgPack Array format.
> + * \param key_end the end of encoded \a key.
> + * \param[out] result an old tuple.
> + * \retval -1 on error.
> + * \retval 0 on success.
> + */
> +int
> +box_ephemeral_delete(struct space *space, uint32_t index_id, const char *key,
> + const char *key_end, box_tuple_t **result);
> +
> +/**
> + * Execute an UPDATE request for ephemeral spaces.
> + *
> + * \param space ephemeral space.
> + * \param index_id index identifier
> + * \param key encoded key in
> + * MsgPack Array format.
> + * \param key_end the end of encoded \a key.
> + * \param ops encoded operations in
> + * MsgPack Arrat format.
> + * \param ops_end the end of encoded \a ops.
> + * \param index_base 0 if fieldnos in
> + * update operations are zero-based
> + * indexed (like C) or 1 if for one-based
> + * indexed field ids (like Lua).
> + * \param[out] result a new tuple.
> + * \retval -1 on error.
> + * \retval 0 on success.
> + */
> +int
> +box_ephemeral_update(struct space *space, uint32_t index_id, const char *key,
> + const char *key_end, const char *ops, const char *ops_end,
> + int index_base, box_tuple_t **result);
> +
> +/**
> + * Execute an UPSERT request for ephemeral spaces.
> + *
> + * \param space ephemeral space.
> + * \param index_id index identifier
> + * \param ops encoded operations in
> + * MsgPack Arrat format.
> + * \param ops_end the end of encoded \a ops.
> + * \param tuple encoded tuple in
> + * MsgPack Array format.
> + * \param tuple_end end of @a tuple.
> + * \param index_base 0 if fieldnos in update
> + * operations are zero-based
> + * indexed (like C) or 1 if for one-based
> + * indexed field ids (like Lua).
> + * \param[out] result a new tuple.
> + * \retval -1 on error.
> + * \retval 0 on success.
> + */
> +int
> +box_ephemeral_upsert(struct space *space, uint32_t index_id, const char *tuple,
> + const char *tuple_end, const char *ops,
> + const char *ops_end, int index_base, box_tuple_t **result);
> +
> +/**
> * Execute request on given space.
> *
> * \param request Request to be executed
> diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c
> index f440951..ce169be 100644
> --- a/src/box/memtx_space.c
> +++ b/src/box/memtx_space.c
> @@ -327,77 +327,76 @@ rollback:
> return -1;
> }
>
> -static int
> -memtx_space_execute_replace(struct space *space, struct txn *txn,
> - struct request *request, struct tuple **result)
> +static inline int
> +memtx_space_replace_impl(struct space *space, struct request *request,
> + struct tuple **new_tuple, struct tuple **old_tuple,
> + struct tuple **result)
> {
> struct memtx_space *memtx_space = (struct memtx_space *)space;
> - struct txn_stmt *stmt = txn_current_stmt(txn);
> enum dup_replace_mode mode = dup_replace_mode(request->type);
> - stmt->new_tuple = memtx_tuple_new(space->format, request->tuple,
> - request->tuple_end);
> - if (stmt->new_tuple == NULL)
> + *new_tuple = memtx_tuple_new(space->format, request->tuple,
> + request->tuple_end);
> + if (*new_tuple == NULL)
> return -1;
> - tuple_ref(stmt->new_tuple);
> - struct tuple *old_tuple;
> - if (memtx_space->replace(space, stmt->old_tuple, stmt->new_tuple,
> - mode, &old_tuple) != 0)
> + tuple_ref(*new_tuple);
> + struct tuple *tmp_tuple;
> + if (memtx_space->replace(space, *old_tuple, *new_tuple, mode,
> + &tmp_tuple) != 0)
> return -1;
> - stmt->old_tuple = old_tuple;
> - stmt->engine_savepoint = stmt;
> + *old_tuple = tmp_tuple;
> /** The new tuple is referenced by the primary key. */
> - *result = stmt->new_tuple;
> + *result = *new_tuple;
> return 0;
> +
> }
>
> -static int
> -memtx_space_execute_delete(struct space *space, struct txn *txn,
> - struct request *request, struct tuple **result)
> +static inline int
> +memtx_space_delete_impl(struct space *space, struct request *request,
> + struct tuple **new_tuple, struct tuple **old_tuple,
> + struct tuple **result)
> {
> struct memtx_space *memtx_space = (struct memtx_space *)space;
> - struct txn_stmt *stmt = txn_current_stmt(txn);
> /* Try to find the tuple by unique key. */
> struct index *pk = index_find_unique(space, request->index_id);
> if (pk == NULL)
> return -1;
> const char *key = request->key;
> uint32_t part_count = mp_decode_array(&key);
> - if (index_get(pk, key, part_count, &stmt->old_tuple) != 0)
> + if (index_get(pk, key, part_count, old_tuple) != 0)
> return -1;
> - struct tuple *old_tuple = NULL;
> - if (stmt->old_tuple != NULL &&
> - memtx_space->replace(space, stmt->old_tuple, stmt->new_tuple,
> - DUP_REPLACE_OR_INSERT, &old_tuple) != 0)
> + struct tuple *tmp_tuple = NULL;
> + if (*old_tuple != NULL &&
> + memtx_space->replace(space, *old_tuple, *new_tuple,
> + DUP_REPLACE_OR_INSERT, &tmp_tuple) != 0)
> return -1;
> - stmt->old_tuple = old_tuple;
> - stmt->engine_savepoint = stmt;
> - *result = stmt->old_tuple;
> + *old_tuple = tmp_tuple;
> + *result = *old_tuple;
> return 0;
> }
>
> static int
> -memtx_space_execute_update(struct space *space, struct txn *txn,
> - struct request *request, struct tuple **result)
> +memtx_space_update_impl(struct space *space, struct request *request,
> + struct tuple **new_tuple, struct tuple **old_tuple,
> + struct tuple **result)
> {
> struct memtx_space *memtx_space = (struct memtx_space *)space;
> - struct txn_stmt *stmt = txn_current_stmt(txn);
> /* Try to find the tuple by unique key. */
> struct index *pk = index_find_unique(space, request->index_id);
> if (pk == NULL)
> return -1;
> const char *key = request->key;
> uint32_t part_count = mp_decode_array(&key);
> - if (index_get(pk, key, part_count, &stmt->old_tuple) != 0)
> + if (index_get(pk, key, part_count, old_tuple) != 0)
> return -1;
>
> - if (stmt->old_tuple == NULL) {
> + if (*old_tuple == NULL) {
> *result = NULL;
> return 0;
> }
>
> /* Update the tuple; legacy, request ops are in request->tuple */
> uint32_t new_size = 0, bsize;
> - const char *old_data = tuple_data_range(stmt->old_tuple, &bsize);
> + const char *old_data = tuple_data_range(*old_tuple, &bsize);
> const char *new_data =
> tuple_update_execute(region_aligned_alloc_cb, &fiber()->gc,
> request->tuple, request->tuple_end,
> @@ -406,28 +405,26 @@ memtx_space_execute_update(struct space *space, struct txn *txn,
> if (new_data == NULL)
> return -1;
>
> - stmt->new_tuple = memtx_tuple_new(space->format, new_data,
> - new_data + new_size);
> - if (stmt->new_tuple == NULL)
> + *new_tuple = memtx_tuple_new(space->format, new_data,
> + new_data + new_size);
> + if (*new_tuple == NULL)
> return -1;
> - tuple_ref(stmt->new_tuple);
> - struct tuple *old_tuple = NULL;
> - if (stmt->old_tuple != NULL &&
> - memtx_space->replace(space, stmt->old_tuple, stmt->new_tuple,
> - DUP_REPLACE, &old_tuple) != 0)
> + tuple_ref(*new_tuple);
> + struct tuple *tmp_tuple = NULL;
> + if (*old_tuple != NULL &&
> + memtx_space->replace(space, *old_tuple, *new_tuple,
> + DUP_REPLACE, &tmp_tuple) != 0)
> return -1;
> - stmt->old_tuple = old_tuple;
> - stmt->engine_savepoint = stmt;
> - *result = stmt->new_tuple;
> + *old_tuple = tmp_tuple;
> + *result = *new_tuple;
> return 0;
> }
>
> static int
> -memtx_space_execute_upsert(struct space *space, struct txn *txn,
> - struct request *request)
> +memtx_space_upsert_impl(struct space *space, struct request *request,
> + struct tuple **new_tuple, struct tuple **old_tuple)
> {
> struct memtx_space *memtx_space = (struct memtx_space *)space;
> - struct txn_stmt *stmt = txn_current_stmt(txn);
> /*
> * Check all tuple fields: we should produce an error on
> * malformed tuple even if upsert turns into an update.
> @@ -450,10 +447,10 @@ memtx_space_execute_upsert(struct space *space, struct txn *txn,
> mp_decode_array(&key);
>
> /* Try to find the tuple by primary key. */
> - if (index_get(index, key, part_count, &stmt->old_tuple) != 0)
> + if (index_get(index, key, part_count, old_tuple) != 0)
> return -1;
>
> - if (stmt->old_tuple == NULL) {
> + if (*old_tuple == NULL) {
> /**
> * Old tuple was not found. A write optimized
> * engine may only know this after commit, so
> @@ -470,21 +467,19 @@ memtx_space_execute_upsert(struct space *space, struct txn *txn,
> * we get it here, it's also OK to throw it
> * @sa https://github.com/tarantool/tarantool/issues/1156
> */
> - if (tuple_update_check_ops(region_aligned_alloc_cb, &fiber()->gc,
> - request->ops, request->ops_end,
> - request->index_base)) {
> + if (tuple_update_check_ops(region_aligned_alloc_cb,
> + &fiber()->gc, request->ops,
> + request->ops_end,
> + request->index_base))
> return -1;
> - }
> - stmt->new_tuple = memtx_tuple_new(space->format,
> - request->tuple,
> - request->tuple_end);
> - if (stmt->new_tuple == NULL)
> + *new_tuple = memtx_tuple_new(space->format, request->tuple,
> + request->tuple_end);
> + if (*new_tuple == NULL)
> return -1;
> - tuple_ref(stmt->new_tuple);
> + tuple_ref(*new_tuple);
> } else {
> uint32_t new_size = 0, bsize;
> - const char *old_data = tuple_data_range(stmt->old_tuple,
> - &bsize);
> + const char *old_data = tuple_data_range(*old_tuple, &bsize);
> /*
> * Update the tuple.
> * tuple_upsert_execute() fails on totally wrong
> @@ -502,24 +497,24 @@ memtx_space_execute_upsert(struct space *space, struct txn *txn,
> if (new_data == NULL)
> return -1;
>
> - stmt->new_tuple = memtx_tuple_new(space->format, new_data,
> - new_data + new_size);
> - if (stmt->new_tuple == NULL)
> + *new_tuple = memtx_tuple_new(space->format, new_data,
> + new_data + new_size);
> + if (*new_tuple == NULL)
> return -1;
> - tuple_ref(stmt->new_tuple);
> + tuple_ref(*new_tuple);
>
> struct index *pk = space->index[0];
> if (!key_update_can_be_skipped(pk->def->key_def->column_mask,
> column_mask) &&
> - tuple_compare(stmt->old_tuple, stmt->new_tuple,
> + tuple_compare(*old_tuple, *new_tuple,
> pk->def->key_def) != 0) {
> /* Primary key is changed: log error and do nothing. */
> diag_set(ClientError, ER_CANT_UPDATE_PRIMARY_KEY,
> pk->def->name, space_name(space));
> diag_log();
> - tuple_unref(stmt->new_tuple);
> - stmt->old_tuple = NULL;
> - stmt->new_tuple = NULL;
> + tuple_unref(*new_tuple);
> + *old_tuple = NULL;
> + *new_tuple = NULL;
> }
> }
> /*
> @@ -528,77 +523,153 @@ memtx_space_execute_upsert(struct space *space, struct txn *txn,
> * we checked this case explicitly and skipped the upsert
> * above.
> */
> - struct tuple *old_tuple = NULL;
> - if (stmt->new_tuple != NULL &&
> - memtx_space->replace(space, stmt->old_tuple, stmt->new_tuple,
> - DUP_REPLACE_OR_INSERT, &old_tuple) != 0)
> + struct tuple *tmp_tuple = NULL;
> + if (*new_tuple != NULL &&
> + memtx_space->replace(space, *old_tuple, *new_tuple,
> + DUP_REPLACE_OR_INSERT, &tmp_tuple) != 0)
> + return -1;
> + *old_tuple = tmp_tuple;
> + return 0;
> +}
> +
> +static int
> +memtx_space_execute_replace(struct space *space, struct txn *txn,
> + struct request *request, struct tuple **result)
> +{
> + struct txn_stmt *stmt = txn_current_stmt(txn);
> + if (memtx_space_replace_impl(space, request, &stmt->new_tuple,
> + &stmt->old_tuple, result) != 0)
> + return -1;
> + stmt->engine_savepoint = stmt;
> + return 0;
> +}
> +
> +static int
> +memtx_space_execute_delete(struct space *space, struct txn *txn,
> + struct request *request, struct tuple **result)
> +{
> + struct txn_stmt *stmt = txn_current_stmt(txn);
> + if (memtx_space_delete_impl(space, request, &stmt->new_tuple,
> + &stmt->old_tuple, result) != 0)
> + return -1;
> + stmt->engine_savepoint = stmt;
> + return 0;
> +}
> +
> +static int
> +memtx_space_execute_update(struct space *space, struct txn *txn,
> + struct request *request, struct tuple **result)
> +{
> + struct txn_stmt *stmt = txn_current_stmt(txn);
> + if (memtx_space_update_impl(space, request, &stmt->new_tuple,
> + &stmt->old_tuple, result) != 0)
> + return -1;
> + stmt->engine_savepoint = stmt;
> + return 0;
> +}
> +
> +static int
> +memtx_space_execute_upsert(struct space *space, struct txn *txn,
> + struct request *request)
> +{
> + struct txn_stmt *stmt = txn_current_stmt(txn);
> + if (memtx_space_upsert_impl(space, request, &stmt->new_tuple,
> + &stmt->old_tuple) != 0)
> return -1;
> - stmt->old_tuple = old_tuple;
> stmt->engine_savepoint = stmt;
> /* Return nothing: UPSERT does not return data. */
> return 0;
> }
>
> /**
> - * This function simply creates new memtx tuple, refs it and calls space's
> - * replace function. In constrast to original memtx_space_execute_replace(), it
> - * doesn't handle any transaction routine.
> - * Ephemeral spaces shouldn't be involved in transaction routine, since
> - * they are used only for internal purposes. Moreover, ephemeral spaces
> - * can be created and destroyed within one transaction and rollback of already
> - * destroyed space may lead to undefined behaviour. For this reason it
> - * doesn't take txn as an argument.
> + * Executes INSERT and REPLACE
> + * operations for ephemeral spaces.
> + *
> + * This function isn't involved in
> + * any transaction routine.
> */
> static int
> -memtx_space_ephemeral_replace(struct space *space, const char *tuple,
> - const char *tuple_end)
> +memtx_space_ephemeral_replace(struct space *space, struct txn *txn,
> + struct request *request, struct tuple **result)
> {
> - struct memtx_space *memtx_space = (struct memtx_space *)space;
> - struct tuple *new_tuple = memtx_tuple_new(space->format, tuple,
> - tuple_end);
> - if (new_tuple == NULL)
> - return -1;
> - tuple_ref(new_tuple);
> + assert(txn == NULL);
> + (void)txn;
> + struct tuple *new_tuple = NULL;
> struct tuple *old_tuple = NULL;
> - if (memtx_space->replace(space, old_tuple, new_tuple,
> - DUP_REPLACE_OR_INSERT, &old_tuple) != 0) {
> + int rc = memtx_space_replace_impl(space, request, &new_tuple,
> + &old_tuple, result);
> + if (rc && new_tuple != NULL)
> tuple_unref(new_tuple);
> + if (rc != 0)
> return -1;
> - }
> if (old_tuple != NULL)
> tuple_unref(old_tuple);
> return 0;
> }
>
> /**
> - * Delete tuple with given key from primary index. Tuple checking is omitted
> - * due to the ability of ephemeral spaces to hold nulls in primary key.
> - * Generally speaking, it is not correct behaviour owing to ambiguity when
> - * fetching/deleting tuple from space with several tuples containing
> - * nulls in PK. On the other hand, ephemeral spaces are used only for internal
> - * needs, so if it is guaranteed that no such situation occur
> - * (when several tuples with nulls in PK exist), it is OK to allow
> - * insertion nulls in PK.
> + * Executes DELETE operation
> + * for ephemeral space.
> *
> - * Similarly to ephemeral replace function,
> - * it isn't involved in any transaction routine.
> + * This function isn't involved in
> + * any transaction routine.
> */
> static int
> -memtx_space_ephemeral_delete(struct space *space, const char *key)
> +memtx_space_ephemeral_delete(struct space *space, struct txn *txn,
> + struct request *request, struct tuple **result)
> {
> - struct memtx_space *memtx_space = (struct memtx_space *)space;
> - struct index *primary_index = space_index(space, 0 /* primary index*/);
> - if (primary_index == NULL)
> + assert(txn == NULL);
> + (void)txn;
> + struct tuple *new_tuple = NULL;
> + struct tuple *old_tuple = NULL;
> + if (memtx_space_delete_impl(space, request, &new_tuple, &old_tuple,
> + result) != 0)
> return -1;
> - uint32_t part_count = mp_decode_array(&key);
> - struct tuple *old_tuple;
> - if (index_get(primary_index, key, part_count, &old_tuple) != 0)
> + return 0;
> +}
> +
> +/**
> + * Executes UPDATE operation
> + * for ephemeral space.
> + *
> + * This function isn't involved in
> + * any transaction routine.
> + */
> +static int
> +memtx_space_ephemeral_update(struct space *space, struct txn *txn,
> + struct request *request, struct tuple **result)
> +{
> + assert(txn == NULL);
> + (void)txn;
> + struct tuple *new_tuple = NULL;
> + struct tuple *old_tuple = NULL;
> + int rc = memtx_space_update_impl(space, request, &new_tuple,
> + &old_tuple, result);
> + if (rc != 0 && new_tuple != NULL)
> + tuple_unref(new_tuple);
> + if (rc != 0)
> return -1;
> - if (old_tuple != NULL &&
> - memtx_space->replace(space, old_tuple, NULL,
> - DUP_REPLACE, &old_tuple) != 0)
> + return 0;
> +}
> +
> +/**
> + * Executes UPSERT operation
> + * for ephemral space.
> + *
> + * This function isn't involved in
> + * any transaction routine.
> + */
> +static int
> +memtx_space_ephemeral_upsert(struct space *space, struct txn *txn,
> + struct request *request)
> +{
> + assert(txn == NULL);
> + (void)txn;
> + struct tuple *old_tuple = NULL;
> + struct tuple *new_tuple = NULL;
> + if (memtx_space_upsert_impl(space, request, &new_tuple,
> + &old_tuple) != 0)
> return -1;
> - tuple_unref(old_tuple);
> return 0;
> }
>
> @@ -839,9 +910,14 @@ memtx_init_system_space(struct space *space)
> }
>
> static void
> -memtx_init_ephemeral_space(struct space *space)
> +memtx_init_ephemeral_space(struct space *space);
> +
> +static void
> +memtx_init_unsupported_space(struct space *space)
> {
> - memtx_space_add_primary_key(space);
> + (void)space;
> + diag_set(ClientError, ER_UNSUPPORTED, space_name(space),
> + "init_ephemeral_space");
> }
>
> static int
> @@ -940,8 +1016,6 @@ static const struct space_vtab memtx_space_vtab = {
> /* .execute_delete = */ memtx_space_execute_delete,
> /* .execute_update = */ memtx_space_execute_update,
> /* .execute_upsert = */ memtx_space_execute_upsert,
> - /* .ephemeral_replace = */ memtx_space_ephemeral_replace,
> - /* .ephemeral_delete = */ memtx_space_ephemeral_delete,
> /* .init_system_space = */ memtx_init_system_space,
> /* .init_ephemeral_space = */ memtx_init_ephemeral_space,
> /* .check_index_def = */ memtx_space_check_index_def,
> @@ -954,6 +1028,33 @@ static const struct space_vtab memtx_space_vtab = {
> /* .prepare_alter = */ memtx_space_prepare_alter,
> };
>
> +static const struct space_vtab memtx_space_ephemeral_vtab = {
> + /* .destroy = */ memtx_space_destroy,
> + /* .bsize = */ memtx_space_bsize,
> + /* .apply_initial_join_row = */ memtx_space_apply_initial_join_row,
> + /* .execute_replace = */ memtx_space_ephemeral_replace,
> + /* .execute_delete = */ memtx_space_ephemeral_delete,
> + /* .execute_update = */ memtx_space_ephemeral_update,
> + /* .execute_upsert = */ memtx_space_ephemeral_upsert,
> + /* .init_system_space = */ memtx_init_system_space,
> + /* .init_ephemeral_space = */ memtx_init_unsupported_space,
> + /* .check_index_def = */ memtx_space_check_index_def,
> + /* .create_index = */ memtx_space_create_index,
> + /* .add_primary_key = */ memtx_space_add_primary_key,
> + /* .drop_primary_key = */ memtx_space_drop_primary_key,
> + /* .check_format = */ memtx_space_check_format,
> + /* .build_index = */ memtx_space_build_index,
> + /* .swap_index = */ generic_space_swap_index,
> + /* .prepare_alter = */ memtx_space_prepare_alter,
> +};
> +
> +static void
> +memtx_init_ephemeral_space(struct space *space)
> +{
> + space->vtab = &memtx_space_ephemeral_vtab;
> + memtx_space_add_primary_key(space);
> +}
> +
> struct space *
> memtx_space_new(struct memtx_engine *memtx,
> struct space_def *def, struct rlist *key_list)
> diff --git a/src/box/space.h b/src/box/space.h
> index adf06b7..60fa603 100644
> --- a/src/box/space.h
> +++ b/src/box/space.h
> @@ -67,10 +67,6 @@ struct space_vtab {
> struct request *, struct tuple **result);
> int (*execute_upsert)(struct space *, struct txn *, struct request *);
>
> - int (*ephemeral_replace)(struct space *, const char *, const char *);
> -
> - int (*ephemeral_delete)(struct space *, const char *);
> -
> void (*init_system_space)(struct space *);
> /**
> * Initialize an ephemeral space instance.
> @@ -310,19 +306,6 @@ int
> space_execute_dml(struct space *space, struct txn *txn,
> struct request *request, struct tuple **result);
>
> -static inline int
> -space_ephemeral_replace(struct space *space, const char *tuple,
> - const char *tuple_end)
> -{
> - return space->vtab->ephemeral_replace(space, tuple, tuple_end);
> -}
> -
> -static inline int
> -space_ephemeral_delete(struct space *space, const char *key)
> -{
> - return space->vtab->ephemeral_delete(space, key);
> -}
> -
> /**
> * Generic implementation of space_vtab::swap_index
> * that simply swaps the two indexes in index maps.
> diff --git a/src/box/sql.c b/src/box/sql.c
> index d2cc0a9..4077890 100644
> --- a/src/box/sql.c
> +++ b/src/box/sql.c
> @@ -451,7 +451,13 @@ int tarantoolSqlite3EphemeralInsert(struct space *space, const char *tuple,
> {
> assert(space != NULL);
> mp_tuple_assert(tuple, tuple_end);
> - if (space_ephemeral_replace(space, tuple, tuple_end) != 0)
> + struct request request;
> + memset(&request, 0, sizeof(request));
> + request.type = IPROTO_REPLACE;
> + request.tuple = tuple;
> + request.tuple_end = tuple_end;
> + struct tuple *result;
> + if (space_execute_dml(space, NULL, &request, &result) != 0)
> return SQL_TARANTOOL_INSERT_FAIL;
> return SQLITE_OK;
> }
> @@ -516,11 +522,20 @@ int tarantoolSqlite3EphemeralDelete(BtCursor *pCur)
> if (key == NULL)
> return SQL_TARANTOOL_DELETE_FAIL;
>
> - int rc = space_ephemeral_delete(pCur->space, key);
> - if (rc != 0) {
> + mp_tuple_assert(key, key + key_size);
> + struct request request;
> + struct tuple *result;
> + memset(&request, 0, sizeof(request));
> + request.type = IPROTO_DELETE;
> + request.key = key;
> + request.key_end = key + key_size;
> +
> + if (space_execute_dml(pCur->space, NULL, &request, &result) != 0) {
> diag_log();
> return SQL_TARANTOOL_DELETE_FAIL;
> }
> + if (result != NULL)
> + box_tuple_unref(result);
> return SQLITE_OK;
> }
>
> @@ -596,14 +611,24 @@ int tarantoolSqlite3EphemeralClearTable(BtCursor *pCur)
> struct tuple *tuple;
> char *key;
> uint32_t key_size;
> + struct request request;
> + struct tuple *result;
> + memset(&request, 0, sizeof(request));
> + request.type = IPROTO_DELETE;
>
> while (iterator_next(it, &tuple) == 0 && tuple != NULL) {
> key = tuple_extract_key(tuple, it->index->def->key_def,
> &key_size);
> - if (space_ephemeral_delete(pCur->space, key) != 0) {
> + mp_tuple_assert(key, key + key_size);
> + request.key = key;
> + request.key_end = key + key_size;
> + if (space_execute_dml(pCur->space, NULL, &request,
> + &result) != 0) {
> iterator_delete(it);
> return SQL_TARANTOOL_DELETE_FAIL;
> }
> + if (result != NULL)
> + box_tuple_unref(result);
> }
> iterator_delete(it);
>
> diff --git a/src/box/sysview_engine.c b/src/box/sysview_engine.c
> index bd9432a..3067175 100644
> --- a/src/box/sysview_engine.c
> +++ b/src/box/sysview_engine.c
> @@ -99,26 +99,6 @@ sysview_space_execute_upsert(struct space *space, struct txn *txn,
> return -1;
> }
>
> -static int
> -sysview_space_ephemeral_replace(struct space *space, const char *tuple,
> - const char *tuple_end)
> -{
> - (void)space;
> - (void)tuple;
> - (void)tuple_end;
> - unreachable();
> - return -1;
> -}
> -
> -static int
> -sysview_space_ephemeral_delete(struct space *space, const char *key)
> -{
> - (void)key;
> - (void)space;
> - unreachable();
> - return -1;
> -}
> -
> static void
> sysview_init_system_space(struct space *space)
> {
> @@ -197,8 +177,6 @@ static const struct space_vtab sysview_space_vtab = {
> /* .execute_delete = */ sysview_space_execute_delete,
> /* .execute_update = */ sysview_space_execute_update,
> /* .execute_upsert = */ sysview_space_execute_upsert,
> - /* .ephemeral_replace = */ sysview_space_ephemeral_replace,
> - /* .ephemeral_delete = */ sysview_space_ephemeral_delete,
> /* .init_system_space = */ sysview_init_system_space,
> /* .init_ephemeral_space = */ sysview_init_ephemeral_space,
> /* .check_index_def = */ sysview_space_check_index_def,
> diff --git a/src/box/vinyl.c b/src/box/vinyl.c
> index 10ec3ca..d089f68 100644
> --- a/src/box/vinyl.c
> +++ b/src/box/vinyl.c
> @@ -2325,26 +2325,6 @@ vinyl_space_execute_upsert(struct space *space, struct txn *txn,
> return vy_upsert(env, tx, stmt, space, request);
> }
>
> -static int
> -vinyl_space_ephemeral_replace(struct space *space, const char *tuple,
> - const char *tuple_end)
> -{
> - (void)space;
> - (void)tuple;
> - (void)tuple_end;
> - unreachable();
> - return -1;
> -}
> -
> -static int
> -vinyl_space_ephemeral_delete(struct space *space, const char *key)
> -{
> - (void)space;
> - (void)key;
> - unreachable();
> - return -1;
> -}
> -
> static inline void
> txn_stmt_unref_tuples(struct txn_stmt *stmt)
> {
> @@ -4494,8 +4474,6 @@ static const struct space_vtab vinyl_space_vtab = {
> /* .execute_delete = */ vinyl_space_execute_delete,
> /* .execute_update = */ vinyl_space_execute_update,
> /* .execute_upsert = */ vinyl_space_execute_upsert,
> - /* .ephemeral_replace = */ vinyl_space_ephemeral_replace,
> - /* .ephemeral_delete = */ vinyl_space_ephemeral_delete,
> /* .init_system_space = */ vinyl_init_system_space,
> /* .init_ephemeral_space = */ vinyl_init_ephemeral_space,
> /* .check_index_def = */ vinyl_space_check_index_def,
>
next prev parent reply other threads:[~2018-07-18 8:06 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <cover.1531837331.git.imeevma@gmail.com>
2018-07-17 14:55 ` [tarantool-patches] " imeevma
2018-07-18 8:06 ` Vladislav Shpilevoy [this message]
2018-07-18 11:52 ` [tarantool-patches] " Kirill Shcherbatov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55802bb0-8ab5-7531-14bd-e55283c43603@tarantool.org \
--to=v.shpilevoy@tarantool.org \
--cc=imeevma@tarantool.org \
--cc=kshcherbatov@tarantool.org \
--cc=tarantool-patches@freelists.org \
--subject='[tarantool-patches] Re: [PATCH v2 3/7] box: create new methods for ephemeral spaces' \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox