From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 139206EC56; Fri, 19 Mar 2021 00:27:26 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 139206EC56 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1616102846; bh=n9Drcc3ebKT0ekIm/Eik82hdak5v66gLGbwxJc2Tino=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=syACJrOX8GBK47AhcNxZYetBQdvuKKkJ2bDvsMdMPXOoaUHUVGOZVAtk4iYCto4by 15xPoVtgmYwBOV+lMzV1CLSgzXxc2TKPOUgIa0qLaBXfeVn6+bncrfXeqGhXloHUuX qeiweyTFG1fkLeRW06bWOkM7EcMNlN8domIFE/bA= Received: from mail-lf1-f43.google.com (mail-lf1-f43.google.com [209.85.167.43]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id B07F46EC56 for ; Fri, 19 Mar 2021 00:27:24 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org B07F46EC56 Received: by mail-lf1-f43.google.com with SMTP id m12so6820662lfq.10 for ; Thu, 18 Mar 2021 14:27:24 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=0asLgbxDEKFhsM7YiQXXcsdY3ejevnbo0TabaVLgPp8=; b=DDOH3YYHV+qeRWMMYPjCIZZPTY+yXQ46mDVXmGMGadPV7bxdJ+8hxZicWO08uVTMzr PqUxdYE1et9SA78Gekx5BxeyLIWAj7Bnw4JODSqb0ZSNH1uK4Whq192DwXOcjgTk/4B2 wzyezmgtHtHfvuCAv39xrMKaIYTXJ/uJLKXKkWZdiYPq65HlU94FdsGemSEKOdL4OYa4 3CSSsQPnaxDzH/BahM7Nl4+a1Kv0lzFxQq8j3kH4BnN6Na7F/zgWbqhV8uv8wJ2v+ZYR ICINhXc6uYqJUi4qAlqHp4nEDuSjmzIYXH8KzsoCbp8Dl2TBMjSYSUtG0S66Ow7qX1pJ otDA== X-Gm-Message-State: AOAM533c1ZHRNWCtsGFt0RSGFbQImcVtJqDoZPkL7pobOv7KqskM4Txs zai0JrbSnlPxbCwsoVm8cgHMluJvBow= X-Google-Smtp-Source: ABdhPJx2sgc3jz5JmTU3GGOYuWqHcmzjHCbKvlzCxYHDBwlbeIgGULNugDywULxz5T63qlh0RTrVYw== X-Received: by 2002:a19:ef02:: with SMTP id n2mr6248572lfh.141.1616102843537; Thu, 18 Mar 2021 14:27:23 -0700 (PDT) Received: from grain.localdomain ([5.18.171.94]) by smtp.gmail.com with ESMTPSA id 21sm367160lfh.231.2021.03.18.14.27.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 18 Mar 2021 14:27:22 -0700 (PDT) Received: by grain.localdomain (Postfix, from userid 1000) id BCACA560133; Fri, 19 Mar 2021 00:27:21 +0300 (MSK) Date: Fri, 19 Mar 2021 00:27:21 +0300 To: Vladislav Shpilevoy Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/2.0.5 (2021-01-21) Subject: Re: [Tarantool-patches] [PATCH 1/2] swim: add SO_BROADCAST option X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Cyrill Gorcunov via Tarantool-patches Reply-To: Cyrill Gorcunov Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" On Thu, Mar 18, 2021 at 09:47:36PM +0100, Vladislav Shpilevoy wrote: > Hi! Thanks for the review! > > >> 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. > I, on the contrary, want to log on all platforms, but not certain errors > in certain platforms. Only EADDRNOTAVAIL and only on Mac. Everything else > must be logged. Because the user does not call 'send' directly, and has no > other way to know there was an error except via the logs.