[Tarantool-patches] [PATCH v2 2/2] vinyl: clean-up unprocessed read views in *_build_read_views()
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Sun Apr 19 18:31:30 MSK 2020
Hi! Thanks for the patch!
See 2 comments below.
> diff --git a/src/box/vy_write_iterator.c b/src/box/vy_write_iterator.c
> index 7a6a20627..910e5a062 100644
> --- a/src/box/vy_write_iterator.c
> +++ b/src/box/vy_write_iterator.c
> @@ -790,8 +790,11 @@ next_lsn:
> * statement around if this is major compaction, because
> * there's no tuple it could overwrite.
> */
> - if (rc == 0 && stream->is_last_level &&
> - stream->deferred_delete_stmt != NULL) {
> + if (rc != 0) {
> + for (int i = 0; i < stream->rv_count; ++i)
> + stream->read_views[i].history = NULL;
1. Why are we sure it is safe to nullify the read view histories in
case of a fail? Looks like they can keep tuples which should be
unreferenced. If it would be enough to nullify them, we could
just do the same in vy_write_iterator_stop(). But it calls
vy_write_history_destroy() before nullification.
Why is not it called here? The same question for vy_read_view_merge().
> + } else if (stream->is_last_level &&
> + stream->deferred_delete_stmt != NULL) {
> vy_stmt_unref_if_possible(stream->deferred_delete_stmt);
> stream->deferred_delete_stmt = NULL;
> }
> @@ -834,6 +837,15 @@ vy_read_view_merge(struct vy_write_iterator *stream, struct tuple *hint,
> rv->history = NULL;
> return 0;
> }
> +#ifndef NDEBUG
> + struct errinj *inj =
> + errinj(ERRINJ_VY_READ_VIEW_MERGE_FAIL, ERRINJ_BOOL);
> + if (inj != NULL && inj->bparam) {
> + inj->bparam = false;
> + diag_set(OutOfMemory, 666, "malloc", "struct vy_stmt");
> + return -1;
> + }
2. Indentation is too big.
> +#endif
> @@ -983,8 +995,13 @@ vy_write_iterator_build_read_views(struct vy_write_iterator *stream, int *count)
> if (rv->history == NULL)
> continue;
> if (vy_read_view_merge(stream, hint, rv,
> - is_first_insert) != 0)
> + is_first_insert) != 0) {
> + while (rv >= &stream->read_views[0]) {
> + rv->history = NULL;
> + rv--;
> + }
> goto error;
> + }
> assert(rv->history == NULL);
> if (rv->tuple == NULL)
> continue;
More information about the Tarantool-patches
mailing list