Re: [tarantool-patches] [PATCH] memtx: add yields during index build

[Serge Petrenko <sergepetrenko@tarantool.org>]

[-- Attachment #1: Type: text/plain, Size: 2496 bytes --]



> 22 мая 2019 г., в 19:28, Vladimir Davydov <vdavydov.dev@gmail.com> написал(а):
> 
> On Wed, May 22, 2019 at 07:11:42PM +0300, Serge Petrenko wrote:
>> Memtx index build used to stall event loop for all the build period.
>> Add occasional yields so that the loop is not blocked for too long.
>> 
>> Closes #3976
>> ---
>> https://github.com/tarantool/tarantool/issues/3976
>> https://github.com/tarantool/tarantool/tree/sp/gh-3976-background-index-build
>> 
>> src/box/memtx_space.c                         | 13 +++++
>> test/box/memtx_background_index_build.result  | 55 +++++++++++++++++++
>> .../box/memtx_background_index_build.test.lua | 32 +++++++++++
>> test/box/suite.ini                            |  2 +-
>> 4 files changed, 101 insertions(+), 1 deletion(-)
>> create mode 100644 test/box/memtx_background_index_build.result
>> create mode 100644 test/box/memtx_background_index_build.test.lua
>> 
>> diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c
>> index 5ddb4f7ee..b90e2707e 100644
>> --- a/src/box/memtx_space.c
>> +++ b/src/box/memtx_space.c
>> @@ -874,6 +874,15 @@ static int
>> memtx_space_build_index(struct space *src_space, struct index *new_index,
>> 			struct tuple_format *new_format)
>> {
>> +	/*
>> +	 * Yield every 1K tuples.
>> +	 * In debug mode yield more often for testing purposes.
>> +	 */
>> +#ifdef NDEBUG
>> +	enum { YIELD_LOOPS = 1000 };
>> +#else
>> +	enum { YIELD_LOOPS = 10 };
>> +#endif
>> 	/**
>> 	 * If it's a secondary key, and we're not building them
>> 	 * yet (i.e. it's snapshot recovery for memtx), do nothing.
>> @@ -909,6 +918,7 @@ memtx_space_build_index(struct space *src_space, struct index *new_index,
>> 	/* Build the new index. */
>> 	int rc;
>> 	struct tuple *tuple;
>> +	size_t count = 0;
>> 	while ((rc = iterator_next(it, &tuple)) == 0 && tuple != NULL) {
>> 		/*
>> 		 * Check that the tuple is OK according to the
>> @@ -933,6 +943,9 @@ memtx_space_build_index(struct space *src_space, struct index *new_index,
>> 		 */
>> 		if (new_index->def->iid == 0)
>> 			tuple_ref(tuple);
>> +		if (++count % YIELD_LOOPS == 0) {
>> +			fiber_sleep(0);
>> +		}
> 
> This isn't enough as tuples may be inserted into / deleted from the
> space while DDL is in progress. We need to propagate these changes
> to the new index using on_replace trigger. Take a look at how vinyl
> handles it.

Thank you for pointing this out! Please see v2 in the mailing list.

[-- Attachment #2: Type: text/html, Size: 10187 bytes --]