Tarantool development patches archive
 help / color / mirror / Atom feed
* [tarantool-patches] [PATCH][vshard] Log warning on duplicate replica.name
@ 2018-06-07 11:50 AKhatskevich
  2018-06-07 14:49 ` [tarantool-patches] " Vladislav Shpilevoy
  0 siblings, 1 reply; 4+ messages in thread
From: AKhatskevich @ 2018-06-07 11:50 UTC (permalink / raw)
  To: v.shpilevoy, tarantool-patches

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/test/unit/config.result b/test/unit/config.result
index 8449ec3..b82303b 100644
--- a/test/unit/config.result
+++ b/test/unit/config.result
@@ -120,13 +120,13 @@ cfg.sharding['rsid2'] = nil
 ---
 ...
 -- UUID duplicate in different replicasets.
-replicaset2 = {replicas = {['id3'] = {uri = 'uri:uri@uri2', name = 'storage', master = true}}}
+replicaset2 = {replicas = {['id3'] = {uri = 'uri:uri@uri2', name = 'storage2', master = true}}}
 ---
 ...
 cfg.sharding['rsid2'] = replicaset2
 ---
 ...
-replicaset3 = {replicas = {['id3'] = {uri = 'uri:uri@uri3', name = 'storage', master = true}}}
+replicaset3 = {replicas = {['id3'] = {uri = 'uri:uri@uri3', name = 'storage3', master = false}}}
 ---
 ...
 cfg.sharding['rsid3'] = replicaset3
@@ -136,6 +136,64 @@ check(cfg)
 ---
 - Duplicate uuid id3
 ...
+cfg.sharding['rsid3'] = nil
+---
+...
+cfg.sharding['rsid2'] = nil
+---
+...
+--
+-- gh-101: Log warning in case replica.name duplicate found.
+--
+-- name duplicate in one replicaset.
+replica2_1 = {uri = 'uri:uri@uri2_1', name = 'dup_name1', master = true}
+---
+...
+replica2_2 = {uri = 'uri:uri@uri2_2', name = 'dup_name1', master = false}
+---
+...
+replicaset2 = {replicas = {['id2'] = replica2_1, ['id3'] = replica2_2}}
+---
+...
+cfg.sharding['rsid2'] = replicaset2
+---
+...
+_ = check(cfg)
+---
+...
+test_run:grep_log('default', 'Duplicate replica.name found: dup_name1')
+---
+- 'Duplicate replica.name found: dup_name1'
+...
+cfg.sharding['rsid2'] = nil
+---
+...
+-- name duplicate in different replicasets.
+replica2 = {uri = 'uri:uri@uri2', name = 'dup_name2', master = true}
+---
+...
+replica3 = {uri = 'uri:uri@uri3', name = 'dup_name2', master = true}
+---
+...
+replicaset2 = {replicas = {['id2'] = replica2}}
+---
+...
+replicaset3 = {replicas = {['id3'] = replica3}}
+---
+...
+cfg.sharding['rsid2'] = replicaset2
+---
+...
+cfg.sharding['rsid3'] = replicaset3
+---
+...
+_ = check(cfg)
+---
+...
+test_run:grep_log('default', 'Duplicate replica.name found: dup_name2')
+---
+- 'Duplicate replica.name found: dup_name2'
+...
 cfg.sharding['rsid2'] = nil
 ---
 ...
diff --git a/test/unit/config.test.lua b/test/unit/config.test.lua
index 0b01b3e..9b2b7be 100644
--- a/test/unit/config.test.lua
+++ b/test/unit/config.test.lua
@@ -57,11 +57,36 @@ check(cfg)
 cfg.sharding['rsid2'] = nil
 
 -- UUID duplicate in different replicasets.
-replicaset2 = {replicas = {['id3'] = {uri = 'uri:uri@uri2', name = 'storage', master = true}}}
+replicaset2 = {replicas = {['id3'] = {uri = 'uri:uri@uri2', name = 'storage2', master = true}}}
 cfg.sharding['rsid2'] = replicaset2
-replicaset3 = {replicas = {['id3'] = {uri = 'uri:uri@uri3', name = 'storage', master = true}}}
+replicaset3 = {replicas = {['id3'] = {uri = 'uri:uri@uri3', name = 'storage3', master = false}}}
 cfg.sharding['rsid3'] = replicaset3
 check(cfg)
+cfg.sharding['rsid3'] = nil
+cfg.sharding['rsid2'] = nil
+
+--
+-- gh-101: Log warning in case replica.name duplicate found.
+--
+
+-- name duplicate in one replicaset.
+replica2_1 = {uri = 'uri:uri@uri2_1', name = 'dup_name1', master = true}
+replica2_2 = {uri = 'uri:uri@uri2_2', name = 'dup_name1', master = false}
+replicaset2 = {replicas = {['id2'] = replica2_1, ['id3'] = replica2_2}}
+cfg.sharding['rsid2'] = replicaset2
+_ = check(cfg)
+test_run:grep_log('default', 'Duplicate replica.name found: dup_name1')
+cfg.sharding['rsid2'] = nil
+
+-- name duplicate in different replicasets.
+replica2 = {uri = 'uri:uri@uri2', name = 'dup_name2', master = true}
+replica3 = {uri = 'uri:uri@uri3', name = 'dup_name2', master = true}
+replicaset2 = {replicas = {['id2'] = replica2}}
+replicaset3 = {replicas = {['id3'] = replica3}}
+cfg.sharding['rsid2'] = replicaset2
+cfg.sharding['rsid3'] = replicaset3
+_ = check(cfg)
+test_run:grep_log('default', 'Duplicate replica.name found: dup_name2')
 cfg.sharding['rsid2'] = nil
 cfg.sharding['rsid3'] = nil
 
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
+            end
         end
     end
 end
-- 
2.14.1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [tarantool-patches] Re: [PATCH][vshard] Log warning on duplicate replica.name
  2018-06-07 11:50 [tarantool-patches] [PATCH][vshard] Log warning on duplicate replica.name AKhatskevich
@ 2018-06-07 14:49 ` Vladislav Shpilevoy
  2018-06-07 16:44   ` Alex Khatskevich
  0 siblings, 1 reply; 4+ messages in thread
From: Vladislav Shpilevoy @ 2018-06-07 14:49 UTC (permalink / raw)
  To: tarantool-patches, AKhatskevich

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
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [tarantool-patches] Re: [PATCH][vshard] Log warning on duplicate replica.name
  2018-06-07 14:49 ` [tarantool-patches] " Vladislav Shpilevoy
@ 2018-06-07 16:44   ` Alex Khatskevich
  2018-06-07 19:05     ` Vladislav Shpilevoy
  0 siblings, 1 reply; 4+ messages in thread
From: Alex Khatskevich @ 2018-06-07 16:44 UTC (permalink / raw)
  To: Vladislav Shpilevoy, tarantool-patches


> 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.
Fixed

New diff:

commit c1d573d035994723c11b2c0e7151f52f67dd5d55
Author: AKhatskevich <avkhatskevich@tarantool.org>
Date:   Thu Jun 7 14:43:16 2018 +0300

     Log warning on duplicate replica.name

     In case duplicate replica.name found, log warning.
     Message appears once for each unique duplicate.

     Closes #101

diff --git a/test/unit/config.result b/test/unit/config.result
index 8449ec3..b82303b 100644
--- a/test/unit/config.result
+++ b/test/unit/config.result
@@ -120,13 +120,13 @@ cfg.sharding['rsid2'] = nil
  ---
  ...
  -- UUID duplicate in different replicasets.
-replicaset2 = {replicas = {['id3'] = {uri = 'uri:uri@uri2', name = 
'storage', master = true}}}
+replicaset2 = {replicas = {['id3'] = {uri = 'uri:uri@uri2', name = 
'storage2', master = true}}}
  ---
  ...
  cfg.sharding['rsid2'] = replicaset2
  ---
  ...
-replicaset3 = {replicas = {['id3'] = {uri = 'uri:uri@uri3', name = 
'storage', master = true}}}
+replicaset3 = {replicas = {['id3'] = {uri = 'uri:uri@uri3', name = 
'storage3', master = false}}}
  ---
  ...
  cfg.sharding['rsid3'] = replicaset3
@@ -136,6 +136,64 @@ check(cfg)
  ---
  - Duplicate uuid id3
  ...
+cfg.sharding['rsid3'] = nil
+---
+...
+cfg.sharding['rsid2'] = nil
+---
+...
+--
+-- gh-101: Log warning in case replica.name duplicate found.
+--
+-- name duplicate in one replicaset.
+replica2_1 = {uri = 'uri:uri@uri2_1', name = 'dup_name1', master = true}
+---
+...
+replica2_2 = {uri = 'uri:uri@uri2_2', name = 'dup_name1', master = false}
+---
+...
+replicaset2 = {replicas = {['id2'] = replica2_1, ['id3'] = replica2_2}}
+---
+...
+cfg.sharding['rsid2'] = replicaset2
+---
+...
+_ = check(cfg)
+---
+...
+test_run:grep_log('default', 'Duplicate replica.name found: dup_name1')
+---
+- 'Duplicate replica.name found: dup_name1'
+...
+cfg.sharding['rsid2'] = nil
+---
+...
+-- name duplicate in different replicasets.
+replica2 = {uri = 'uri:uri@uri2', name = 'dup_name2', master = true}
+---
+...
+replica3 = {uri = 'uri:uri@uri3', name = 'dup_name2', master = true}
+---
+...
+replicaset2 = {replicas = {['id2'] = replica2}}
+---
+...
+replicaset3 = {replicas = {['id3'] = replica3}}
+---
+...
+cfg.sharding['rsid2'] = replicaset2
+---
+...
+cfg.sharding['rsid3'] = replicaset3
+---
+...
+_ = check(cfg)
+---
+...
+test_run:grep_log('default', 'Duplicate replica.name found: dup_name2')
+---
+- 'Duplicate replica.name found: dup_name2'
+...
  cfg.sharding['rsid2'] = nil
  ---
  ...
diff --git a/test/unit/config.test.lua b/test/unit/config.test.lua
index 0b01b3e..9b2b7be 100644
--- a/test/unit/config.test.lua
+++ b/test/unit/config.test.lua
@@ -57,11 +57,36 @@ check(cfg)
  cfg.sharding['rsid2'] = nil

  -- UUID duplicate in different replicasets.
-replicaset2 = {replicas = {['id3'] = {uri = 'uri:uri@uri2', name = 
'storage', master = true}}}
+replicaset2 = {replicas = {['id3'] = {uri = 'uri:uri@uri2', name = 
'storage2', master = true}}}
  cfg.sharding['rsid2'] = replicaset2
-replicaset3 = {replicas = {['id3'] = {uri = 'uri:uri@uri3', name = 
'storage', master = true}}}
+replicaset3 = {replicas = {['id3'] = {uri = 'uri:uri@uri3', name = 
'storage3', master = false}}}
  cfg.sharding['rsid3'] = replicaset3
  check(cfg)
+cfg.sharding['rsid3'] = nil
+cfg.sharding['rsid2'] = nil
+
+--
+-- gh-101: Log warning in case replica.name duplicate found.
+--
+
+-- name duplicate in one replicaset.
+replica2_1 = {uri = 'uri:uri@uri2_1', name = 'dup_name1', master = true}
+replica2_2 = {uri = 'uri:uri@uri2_2', name = 'dup_name1', master = false}
+replicaset2 = {replicas = {['id2'] = replica2_1, ['id3'] = replica2_2}}
+cfg.sharding['rsid2'] = replicaset2
+_ = check(cfg)
+test_run:grep_log('default', 'Duplicate replica.name found: dup_name1')
+cfg.sharding['rsid2'] = nil
+
+-- name duplicate in different replicasets.
+replica2 = {uri = 'uri:uri@uri2', name = 'dup_name2', master = true}
+replica3 = {uri = 'uri:uri@uri3', name = 'dup_name2', master = true}
+replicaset2 = {replicas = {['id2'] = replica2}}
+replicaset3 = {replicas = {['id3'] = replica3}}
+cfg.sharding['rsid2'] = replicaset2
+cfg.sharding['rsid3'] = replicaset3
+_ = check(cfg)
+test_run:grep_log('default', 'Duplicate replica.name found: dup_name2')
  cfg.sharding['rsid2'] = nil
  cfg.sharding['rsid3'] = nil

diff --git a/vshard/cfg.lua b/vshard/cfg.lua
index a389eea..7a25cc9 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,18 @@ 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.
+            -- Message appears once for each unique duplicate.
+            local name = replica.name
+            if name then
+                if names[name] == nil then
+                    names[name] = 1
+                elseif names[name] == 1 then
+                    log.warn('Duplicate replica.name found: %s', name)
+                    -- Next duplicates should not be reported.
+                    names[name] = 2
+                end
+            end
          end
      end
  end

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [tarantool-patches] Re: [PATCH][vshard] Log warning on duplicate replica.name
  2018-06-07 16:44   ` Alex Khatskevich
@ 2018-06-07 19:05     ` Vladislav Shpilevoy
  0 siblings, 0 replies; 4+ messages in thread
From: Vladislav Shpilevoy @ 2018-06-07 19:05 UTC (permalink / raw)
  To: tarantool-patches, Alex Khatskevich

Great! Thanks for the fixes! But see the diff that I applied and pushed
into the master branch:

diff --git a/test/unit/config.result b/test/unit/config.result
index b82303b..6b4f87b 100644
--- a/test/unit/config.result
+++ b/test/unit/config.result
@@ -161,9 +161,9 @@ cfg.sharding['rsid2'] = replicaset2
  _ = check(cfg)
  ---
  ...
-test_run:grep_log('default', 'Duplicate replica.name found: dup_name1')
+test_run:grep_log('default', 'Duplicate replica.name is found: dup_name1')
  ---
-- 'Duplicate replica.name found: dup_name1'
+- 'Duplicate replica.name is found: dup_name1'
  ...
  cfg.sharding['rsid2'] = nil
  ---
@@ -190,9 +190,9 @@ cfg.sharding['rsid3'] = replicaset3
  _ = check(cfg)
  ---
  ...
-test_run:grep_log('default', 'Duplicate replica.name found: dup_name2')
+test_run:grep_log('default', 'Duplicate replica.name is found: dup_name2')
  ---
-- 'Duplicate replica.name found: dup_name2'
+- 'Duplicate replica.name is found: dup_name2'
  ...
  cfg.sharding['rsid2'] = nil
  ---
diff --git a/test/unit/config.test.lua b/test/unit/config.test.lua
index 9b2b7be..0f549d3 100644
--- a/test/unit/config.test.lua
+++ b/test/unit/config.test.lua
@@ -75,7 +75,7 @@ replica2_2 = {uri = 'uri:uri@uri2_2', name = 'dup_name1', master = false}
  replicaset2 = {replicas = {['id2'] = replica2_1, ['id3'] = replica2_2}}
  cfg.sharding['rsid2'] = replicaset2
  _ = check(cfg)
-test_run:grep_log('default', 'Duplicate replica.name found: dup_name1')
+test_run:grep_log('default', 'Duplicate replica.name is found: dup_name1')
  cfg.sharding['rsid2'] = nil
  
  -- name duplicate in different replicasets.
@@ -86,7 +86,7 @@ replicaset3 = {replicas = {['id3'] = replica3}}
  cfg.sharding['rsid2'] = replicaset2
  cfg.sharding['rsid3'] = replicaset3
  _ = check(cfg)
-test_run:grep_log('default', 'Duplicate replica.name found: dup_name2')
+test_run:grep_log('default', 'Duplicate replica.name is found: dup_name2')
  cfg.sharding['rsid2'] = nil
  cfg.sharding['rsid3'] = nil
  
diff --git a/vshard/cfg.lua b/vshard/cfg.lua
index 7a25cc9..f5db4c0 100644
--- a/vshard/cfg.lua
+++ b/vshard/cfg.lua
@@ -160,14 +160,15 @@ 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.
-            -- Message appears once for each unique duplicate.
+            -- Log warning in case replica.name duplicate is
+            -- found. Message appears once for each unique
+            -- duplicate.
              local name = replica.name
              if name then
                  if names[name] == nil then
                      names[name] = 1
                  elseif names[name] == 1 then
-                    log.warn('Duplicate replica.name found: %s', name)
+                    log.warn('Duplicate replica.name is found: %s', name)
                      -- Next duplicates should not be reported.
                      names[name] = 2
                  end

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-06-07 19:05 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-07 11:50 [tarantool-patches] [PATCH][vshard] Log warning on duplicate replica.name AKhatskevich
2018-06-07 14:49 ` [tarantool-patches] " Vladislav Shpilevoy
2018-06-07 16:44   ` Alex Khatskevich
2018-06-07 19:05     ` Vladislav Shpilevoy

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