Re: [Tarantool-patches] [PATCH] relay: yield explicitly every N sent rows

[Serge Petrenko via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

18.02.2021 00:11, Vladislav Shpilevoy пишет:
> Hi! Thanks for the fixes!

Thanks for the review!

>
>> diff --git a/src/box/relay.cc b/src/box/relay.cc
>> index 1d8edf116..6d9269e1d 100644
>> --- a/src/box/relay.cc
>> +++ b/src/box/relay.cc
>> @@ -117,6 +117,11 @@ struct relay {
>>           * is passed by the replica on subscribe.
>>           */
>>          uint32_t id_filter;
>> +       /**
>> +        * How many rows has this relay sent to the replica. Used to yield once
>> +        * in a while when reading a WAL to unblock the event loop.
>> +        */
>> +       size_t row_cnt;
> 1. But it is not a size of anything, right? Maybe make it
> int64_t then?

uint64_t, probably?
I'm fine with it. Fixed on the branch.


>
>> @@ -846,7 +851,7 @@ relay_send(struct relay *relay, struct xrow_header *packet)
>>           * It may happen that the socket is always ready for write, so yield
>>           * explicitly every now and then to not block the event loop.
>>           */
>> -       if (++row_cnt % WAL_ROWS_PER_YIELD == 0)
>> +       if (++relay->row_cnt % WAL_ROWS_PER_YIELD == 0)
> 2. I just found WAL_ROWS_PER_YIELD is not a power of 2. This means
> '%' may be expensive. If WAL_ROWS_PER_YIELD would be a power of 2,
> '%' would be turned by the compiler into '&' to cut the remainder off.
>
> Maybe worth changing in a separate non-related to this bug commit?

Good idea, thanks!
Pushed on top of the original commit.


-- 
Serge Petrenko