[tarantool-patches] Re: [PATCH v2 2/4] wal: wal memory buffer

[Vladislav Shpilevoy]

Hi! Thanks for the answers!

My question about lsregion still is actual:

"
    A common question on the whole patch: why won't lsregion here
    work better than a ring of obufs/ibufs? lsregion works like a ring
    but of a dynamic size, and perhaps implements everything you need.
"

>>>>> +	/* Decoded xrow header. */
>>>>> +	struct xrow_header xrow;
>>>>> +	/* Pointer to the xrow encoded raw data. */
>>>>> +	void *data;
>>>>> +	/* xrow raw data size. */
>>>>> +	size_t size;
>>>>> +};
>>>>> +
>>>>> +/*
>>>>> + * Xrow memory data chunk info contains
>>>>> + *  a vclock just before the first stored row,
>>>>> + *  an ibuf with row descriptors
>>>>> + *  an obuf with encoded data
>>>>
>>>> 29. I don't think it really makes sense to just dupicate here
>>>> the comments of each attribute of the structure. Please,
>>>> better use that space to explain, why you need vclock, why
>>>> 'before first row' instead of just 'first row', or 'last row'.
>>>>
>>>> What is a 'row descriptor'?
>>>>
>>>> Why do you need both initial xrows and encoded? I thought, that
>>>> xrow_buf stores only encoded rows.
>>>
>>> Relay will need initial xrows to perform filtering (replication group, lsn
>>> and e.t.c.) without xrow encoded body to decode.
>>
>> Hm, it looks quite expensive just for filtering. But assume it is ok.
>> 1) Why can't you store only a header instead of the whole xrow with a
>> body? As I understand, for any filtering the header is enough. Or not?
> I should also store xrow bodies and then encode the xrow as many times as 
> count of replicas I have.

Why do you need to encode it multiple times? Why not to send the same
encoded record?
Also, I don't see where do you encode more than once.

> Because of this I store not only a xrow_header 
> struct but also the xrow encoded data location and size.
>>
>> 2) Suppose, that you found a row which should be filtered out. That
>> row is already encoded somewhere inside obuf of that chunk. How will
>> you find where is it encoded so as not to send this part to a replica?
>> Where is that mapping of index in ibuf rows <-> range in obuf data?
> You could see it in the next patch.

Only in the next? So my assumption below is wrong, and xrow_buf_chunk.data
is not what I supposed?

>>
>> Actually, after I wrote the second question, I looked at the code again,
>> and found xrow_buf_row_info.data. Is it this pointer I was talking about?
>> If so, then it is super-ultra not obvious, that it is a pointer at some
>> place inside xrow_buf_chunk.data. Such a link should be documented in a
>> very detailed comment.
> Ok, will add the comment
>