[Tarantool-patches] [PATCH 1/2] swim: add SO_BROADCAST option
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Fri Mar 19 01:34:35 MSK 2021
>>>> diff --git a/src/lib/swim/swim_io.c b/src/lib/swim/swim_io.c
>>>> index c8558c43e..45df36ba4 100644
>>>> --- a/src/lib/swim/swim_io.c
>>>> +++ b/src/lib/swim/swim_io.c
>>>> @@ -512,8 +512,23 @@ static inline void
>>>> swim_complete_send(struct swim_scheduler *scheduler, struct swim_task *task,
>>>> ssize_t size)
>>>> {
>>>> - if (size < 0)
>>>> - diag_log();
>>>> + if (size < 0) {
>>>> + bool is_critical = false;
>>>> +#if TARGET_OS_DARWIN
>>>> + /*
>>>> + * On Mac this error happens regularly if SWIM is bound to
>>>> + * the localhost and tries to broadcast out of the machine. This
>>>> + * is not critical, because will happen in the tests a lot, and
>>>> + * in prod it simply should not bind to localhost if there are
>>>> + * multiple machines in the cluster. Besides, Mac as a platform
>>>> + * is not supposed to be used in prod.
>>>> + */
>>>> + struct error *last = diag_last_error(diag_get());
>>>> + is_critical = (last->saved_errno == EADDRNOTAVAIL);
>>>> +#endif
>>>> + if (is_critical)
>>>> + diag_log();
>>>> + }
>>>> if (task->complete != NULL)
>>>> task->complete(task, scheduler, size);
>>>
>>> Vlad, I don't understand. For non-mac users this @is_critical will
>>> always be false, maybe we better move the whoe branch to TARGET_OS_DARWIN
>>> then? Ie
>>>
>>> if (size < 0) {
>>> #if TARGET_OS_DARWIN
>>> if (is_critical)
>>> diag_log();
>>> #endif
>>> }
>>>
>>> or you made it this way to escape unused parameter compiler warning?
>>
>> I added the variable on all platforms on purpose. Because if the code would
>> be done the way you propose above, non-Mac platforms won't log anything.
>
> The do not log anything with the patch. Lets drop TARGET_OS_DARWIN path
> from your patch and we will have
>
> swim_complete_send(struct swim_scheduler *scheduler, struct swim_task *task,
> ssize_t size)
> {
> if (size < 0) {
> bool is_critical = false;
> if (is_critical)
> diag_log();
> }
> }
>
> The is_critical is always false, or there is a typo in the code above.
Oh shit, indeed :)
More information about the Tarantool-patches
mailing list