[tarantool-patches] Re: [PATCH v2 2/4] wal: wal memory buffer
v.shpilevoy at tarantool.org
Sun Sep 22 19:14:17 MSK 2019
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
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
More information about the Tarantool-patches