Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: tarantool-patches@freelists.org,
	AKhatskevich <avkhatskevich@tarantool.org>
Subject: [tarantool-patches] Re: [PATCH][vshard] Log warning on duplicate replica.name
Date: Thu, 7 Jun 2018 17:49:09 +0300	[thread overview]
Message-ID: <b1d85339-1a28-0d4b-3472-3055541b8a35@tarantool.org> (raw)
In-Reply-To: <20180607115043.3851-1-avkhatskevich@tarantool.org>

Thanks the patch! Almost perfect. But one minor comment. See it below.

On 07/06/2018 14:50, AKhatskevich wrote:
> In case duplicate replica.name found, log warning.
> 
> Closes #101
> ---
> Branch: https://github.com/tarantool/vshard/tree/kh/gh-101-duplicate_name_warning
> Issue: https://github.com/tarantool/vshard/issues/101
>   test/unit/config.result   | 62 +++++++++++++++++++++++++++++++++++++++++++++--
>   test/unit/config.test.lua | 29 ++++++++++++++++++++--
>   vshard/cfg.lua            |  8 ++++++
>   3 files changed, 95 insertions(+), 4 deletions(-)
> 
> diff --git a/vshard/cfg.lua b/vshard/cfg.lua
> index a389eea..c3cf5d7 100644
> --- a/vshard/cfg.lua
> +++ b/vshard/cfg.lua
> @@ -141,6 +141,7 @@ end
>   local function check_sharding(sharding)
>       local uuids = {}
>       local uris = {}
> +    local names = {}
>       for replicaset_uuid, replicaset in pairs(sharding) do
>           if uuids[replicaset_uuid] then
>               error(string.format('Duplicate uuid %s', replicaset_uuid))
> @@ -159,6 +160,13 @@ local function check_sharding(sharding)
>                   error(string.format('Duplicate uuid %s', replica_uuid))
>               end
>               uuids[replica_uuid] = true
> +            -- log warning in case replica.name duplicate found
> +            if replica.name then
> +                if names[replica.name] then
> +                    log.warn('Duplicate replica.name found: %s', replica.name)
> +                end
> +                names[replica.name] = true

Please, print the warning on each duplicate only once. Now if I use the same name
N >= 2 times, the warning for the name is printed N - 1 times.

Example:

     name1, name1, name1, name1

In the log I see:

     Duplicate replica.name found: name1
     Duplicate replica.name found: name1
     Duplicate replica.name found: name1

Must be:

     Duplicate replica.name found: name1

For example, you can store in 'names' not booleans, but count of name
duplicates. And print the warning only when the count is 2.

> +            end
>           end
>       end
>   end
> 

  reply	other threads:[~2018-06-07 14:49 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-07 11:50 [tarantool-patches] " AKhatskevich
2018-06-07 14:49 ` Vladislav Shpilevoy [this message]
2018-06-07 16:44   ` [tarantool-patches] " Alex Khatskevich
2018-06-07 19:05     ` Vladislav Shpilevoy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b1d85339-1a28-0d4b-3472-3055541b8a35@tarantool.org \
    --to=v.shpilevoy@tarantool.org \
    --cc=avkhatskevich@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='[tarantool-patches] Re: [PATCH][vshard] Log warning on duplicate replica.name' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox