[Tarantool-patches] [PATCH v2] replication: add box.info.replication_anon

[Tarantool-patches] [PATCH v2] replication: add box.info.replication_anon

[Serge Petrenko]

Closes #4900

@TarantoolBot document
Title: add new field to box.info: replication_anon

It is now possible to list all the anonymous replicas following the
instance with a call to `box.info.replication_anon()`
The output is similar to the one produced by `box.info.replication` with
an exception that anonymous replicas are indexed by their uuid strings
rather then server ids, since server ids have no meaning for anonymous
replicas.

Example:
```
tarantool> box.info.replication_anon
---
- []
...

tarantool> box.info.replication_anon()
---
- 02757524-1887-4c75-9137-d5c191fba507:
    id: 0
    uuid: 02757524-1887-4c75-9137-d5c191fba507
    lsn: 0
    downstream:
      status: follow
      idle: 0.43651100000716
      vclock: {1: 1}
  937a8292-8f99-4d11-aede-b97e9f5e9473:
    id: 0
    uuid: 937a8292-8f99-4d11-aede-b97e9f5e9473
    lsn: 0
    downstream:
      status: follow
      idle: 0.5189309999987
      vclock: {1: 1}
...

```

Note, that anonymous replicas hide their lsn from the others, so
anonymous replica lsn will always be reported as zero, even if anonymous
replicas perform some local space operations.
To know the anonymous replica's lsn, you have to issue `box.info.lsn` on
it.
---
https://github.com/tarantool/tarantool/issues/4900
https://github.com/tarantool/tarantool/tree/sp/gh-4900-list-anon-replicas

Changes in v2:
  - move anon replicas output to
    box.info.replication_anon()

@ChangeLog
  - Add `box.info.replication_anon`. When called, it lists anonymous replicas
    in the same format as `box.info.replication`, the only exception is that
    anon replicas are indexed by their uuid strings (gh-4900).

 src/box/lua/info.c             | 45 ++++++++++++++++++++++++++++++++++
 test/box/info.result           |  1 +
 test/replication/anon.result   | 24 +++++++++++++++---
 test/replication/anon.test.lua | 11 +++++++--
 4 files changed, 76 insertions(+), 5 deletions(-)

diff --git a/src/box/lua/info.c b/src/box/lua/info.c
index a789f7822..c4f90bda1 100644
--- a/src/box/lua/info.c
+++ b/src/box/lua/info.c
@@ -214,6 +214,50 @@ lbox_info_replication(struct lua_State *L)
 	return 1;
 }
 
+static int
+lbox_info_replication_anon_call(struct lua_State *L)
+{
+	lua_newtable(L); /* box.info.replication_anon */
+
+	/* Nice formatting */
+	lua_newtable(L); /* metatable */
+	lua_pushliteral(L, "mapping");
+	lua_setfield(L, -2, "__serialize");
+	lua_setmetatable(L, -2);
+
+	replicaset_foreach(replica) {
+		if (!replica->anon)
+			continue;
+
+		lua_pushstring(L, tt_uuid_str(&replica->uuid));
+		lbox_pushreplica(L, replica);
+
+		lua_settable(L, -3);
+	}
+
+	return 1;
+}
+
+static int
+lbox_info_replication_anon(struct lua_State *L)
+{
+	/*
+	 * Make the .replication_anon field callable in order to
+	 * not flood the output with possibly lots of anonymous
+	 * replicas on box.info call.
+	 */
+	lua_newtable(L);
+
+	lua_newtable(L); /* metatable */
+
+	lua_pushstring(L, "__call");
+	lua_pushcfunction(L, lbox_info_replication_anon_call);
+	lua_settable(L, -3);
+
+	lua_setmetatable(L, -2);
+	return 1;
+}
+
 static int
 lbox_info_id(struct lua_State *L)
 {
@@ -534,6 +578,7 @@ static const struct luaL_Reg lbox_info_dynamic_meta[] = {
 	{"vclock", lbox_info_vclock},
 	{"ro", lbox_info_ro},
 	{"replication", lbox_info_replication},
+	{"replication_anon", lbox_info_replication_anon},
 	{"status", lbox_info_status},
 	{"uptime", lbox_info_uptime},
 	{"pid", lbox_info_pid},
diff --git a/test/box/info.result b/test/box/info.result
index e6d0ba6aa..40eeae069 100644
--- a/test/box/info.result
+++ b/test/box/info.result
@@ -83,6 +83,7 @@ t
   - package
   - pid
   - replication
+  - replication_anon
   - ro
   - signature
   - sql
diff --git a/test/replication/anon.result b/test/replication/anon.result
index cbbeeef09..f3f9c60ed 100644
--- a/test/replication/anon.result
+++ b/test/replication/anon.result
@@ -151,10 +151,28 @@ test_run:cmd('switch default')
  | - true
  | ...
 
--- Replica isn't visible on master.
-#box.info.replication
+-- Test box.info.replication_anon.
+box.info.replication_anon
+ | ---
+ | - []
+ | ...
+#box.info.replication_anon()
+ | ---
+ | - 0
+ | ...
+uuid, tbl = next(box.info.replication_anon())
+ | ---
+ | ...
+-- Anonymous replicas are indexed by uuid strings.
+require("uuid").fromstr(uuid) ~= nil
+ | ---
+ | - true
+ | ...
+-- Anonymous replicas share box.info representation with
+-- normal replicas.
+tbl.downstream.status
  | ---
- | - 1
+ | - follow
  | ...
 
 -- Test that replication (even anonymous) from an anonymous
diff --git a/test/replication/anon.test.lua b/test/replication/anon.test.lua
index 627dc5c8e..fef72d07d 100644
--- a/test/replication/anon.test.lua
+++ b/test/replication/anon.test.lua
@@ -50,8 +50,15 @@ box.space.loc:truncate()
 
 test_run:cmd('switch default')
 
--- Replica isn't visible on master.
-#box.info.replication
+-- Test box.info.replication_anon.
+box.info.replication_anon
+#box.info.replication_anon()
+uuid, tbl = next(box.info.replication_anon())
+-- Anonymous replicas are indexed by uuid strings.
+require("uuid").fromstr(uuid) ~= nil
+-- Anonymous replicas share box.info representation with
+-- normal replicas.
+tbl.downstream.status
 
 -- Test that replication (even anonymous) from an anonymous
 -- instance is forbidden. An anonymous replica will fetch
-- 
2.21.1 (Apple Git-122.3)

Re: [Tarantool-patches] [PATCH v2] replication: add box.info.replication_anon

[Konstantin Osipov]

* Serge Petrenko <sergepetrenko@tarantool.org> [20/04/24 19:33]:
> Closes #4900
> 
> @TarantoolBot document
> Title: add new field to box.info: replication_anon
> 
> It is now possible to list all the anonymous replicas following the
> instance with a call to `box.info.replication_anon()`
> The output is similar to the one produced by `box.info.replication` with
> an exception that anonymous replicas are indexed by their uuid strings
> rather then server ids, since server ids have no meaning for anonymous
> replicas.
> 
> Example:
> ```
> tarantool> box.info.replication_anon
> ---
> - []
> ...
> 
> tarantool> box.info.replication_anon()

Why do you have to use ()? Isn't it inconsistent with
box.info.replication?


-- 
Konstantin Osipov, Moscow, Russia
https://scylladb.com

Re: [Tarantool-patches] [PATCH v2] replication: add box.info.replication_anon

[Serge Petrenko]

> 24 апр. 2020 г., в 19:39, Konstantin Osipov <kostja.osipov@gmail.com> написал(а):
> 
> * Serge Petrenko <sergepetrenko@tarantool.org> [20/04/24 19:33]:
>> Closes #4900
>> 
>> @TarantoolBot document
>> Title: add new field to box.info: replication_anon
>> 
>> It is now possible to list all the anonymous replicas following the
>> instance with a call to `box.info.replication_anon()`
>> The output is similar to the one produced by `box.info.replication` with
>> an exception that anonymous replicas are indexed by their uuid strings
>> rather then server ids, since server ids have no meaning for anonymous
>> replicas.
>> 
>> Example:
>> ```
>> tarantool> box.info.replication_anon
>> ---
>> - []
>> ...
>> 
>> tarantool> box.info.replication_anon()
> 
> Why do you have to use ()? Isn't it inconsistent with
> box.info.replication?
> 

It’s done in order to shorten `box.info` output. There may be lots
of anonymous replicas following a master. I’ve updated the commit message
correspondingly.

It was also agreed verbally, that we should output the number of anonymous
replicas following a master on `box.info.replication_anon` without the `()`.

The diff is below.

diff --git a/src/box/lua/info.c b/src/box/lua/info.c
index c4f90bda1..e7318fcd1 100644
--- a/src/box/lua/info.c
+++ b/src/box/lua/info.c
@@ -248,6 +248,10 @@ lbox_info_replication_anon(struct lua_State *L)
 	 */
 	lua_newtable(L);
 
+	lua_pushliteral(L, "count");
+	lua_pushinteger(L, replicaset.anon_count);
+	lua_settable(L, -3);
+
 	lua_newtable(L); /* metatable */
 
 	lua_pushstring(L, "__call");
diff --git a/src/box/replication.cc b/src/box/replication.cc
index 7c10fb6f2..1b61c92c0 100644
--- a/src/box/replication.cc
+++ b/src/box/replication.cc
@@ -219,6 +219,7 @@ replicaset_add_anon(const struct tt_uuid *replica_uuid)
 	replica->uuid = *replica_uuid;
 	replica_hash_insert(&replicaset.hash, replica);
 	replica->anon = true;
+	replicaset.anon_count++;
 	return replica;
 }
 
@@ -909,6 +910,8 @@ replica_on_relay_stop(struct replica *replica)
 			 * replicas.
 			 */
 			assert(replica->applier == NULL);
+			assert(replicaset.anon_count > 0);
+			replicaset.anon_count--;
 		}
 	}
 	if (replica_is_orphan(replica)) {
diff --git a/src/box/replication.h b/src/box/replication.h
index 9df91e611..93a25c8a7 100644
--- a/src/box/replication.h
+++ b/src/box/replication.h
@@ -203,6 +203,8 @@ struct replicaset {
 	 * from a remote master.
 	 */
 	bool is_joining;
+	/* A number of anonymous replicas following this instance. */
+	int anon_count;
 	/** Applier state. */
 	struct {
 		/**
diff --git a/test/replication/anon.result b/test/replication/anon.result
index f3f9c60ed..a7e244d3f 100644
--- a/test/replication/anon.result
+++ b/test/replication/anon.result
@@ -154,7 +154,7 @@ test_run:cmd('switch default')
 -- Test box.info.replication_anon.
 box.info.replication_anon
  | ---
- | - []
+ | - count: 1
  | ...
 #box.info.replication_anon()
  | ---


> 
> -- 
> Konstantin Osipov, Moscow, Russia
> https://scylladb.com

--
Serge Petrenko
sergepetrenko@tarantool.org

Re: [Tarantool-patches] [PATCH v2] replication: add box.info.replication_anon

[Konstantin Osipov]

* Serge Petrenko <sergepetrenko@tarantool.org> [20/04/27 15:24]:

OK, I see, lgtm.

-- 
Konstantin Osipov, Moscow, Russia

Re: [Tarantool-patches] [PATCH v2] replication: add box.info.replication_anon

[Vladislav Shpilevoy]

Hi! Thanks for the patch!

>     Closes #4900
>     
>     @TarantoolBot document
>     Title: add new field to box.info: replication_anon
>     
>     It is now possible to list all the anonymous replicas following the
>     instance with a call to `box.info.replication_anon()`
>     The output is similar to the one produced by `box.info.replication` with
>     an exception that anonymous replicas are indexed by their uuid strings
>     rather then server ids, since server ids have no meaning for anonymous
>     replicas.
>     
>     Note, that when you issue a plain `box.info.replication_anon`, the only
>     info returned is the number of anonymous replicas following the current
>     instance. In order to see the full stats, you have to call
>     `box.info.replication_anon()` . This is done to not overload the `box.info`
>     output with excess  info, since there may be lots of anonymous replicas.

Double whitespaces on this line and previous.

Also see 2 nits about comments below.

> diff --git a/src/box/lua/info.c b/src/box/lua/info.c
> index a789f7822..e7318fcd1 100644
> --- a/src/box/lua/info.c
> +++ b/src/box/lua/info.c
> @@ -214,6 +214,54 @@ lbox_info_replication(struct lua_State *L)
>  	return 1;
>  }
>  
> +static int
> +lbox_info_replication_anon_call(struct lua_State *L)
> +{
> +	lua_newtable(L); /* box.info.replication_anon */
> +
> +	/* Nice formatting */
> +	lua_newtable(L); /* metatable */

1. Better write comments on separate lines, use '.' in the end
of sentences and capital letters when start them.

> +	lua_pushliteral(L, "mapping");
> +	lua_setfield(L, -2, "__serialize");
> +	lua_setmetatable(L, -2);
> +
> +	replicaset_foreach(replica) {
> +		if (!replica->anon)
> +			continue;
> +
> +		lua_pushstring(L, tt_uuid_str(&replica->uuid));
> +		lbox_pushreplica(L, replica);
> +
> +		lua_settable(L, -3);
> +	}
> +
> +	return 1;
> +}
> +
> +static int
> +lbox_info_replication_anon(struct lua_State *L)
> +{
> +	/*
> +	 * Make the .replication_anon field callable in order to
> +	 * not flood the output with possibly lots of anonymous
> +	 * replicas on box.info call.
> +	 */
> +	lua_newtable(L);
> +
> +	lua_pushliteral(L, "count");
> +	lua_pushinteger(L, replicaset.anon_count);
> +	lua_settable(L, -3);
> +
> +	lua_newtable(L); /* metatable */

2. Here too.

> +
> +	lua_pushstring(L, "__call");
> +	lua_pushcfunction(L, lbox_info_replication_anon_call);
> +	lua_settable(L, -3);
> +
> +	lua_setmetatable(L, -2);
> +	return 1;
> +}

Re: [Tarantool-patches] [PATCH v2] replication: add box.info.replication_anon

[Serge Petrenko]

01.05.2020 22:56, Vladislav Shpilevoy пишет:
> Hi! Thanks for the patch! 
Hi! Thanks for the review!


>> Closes #4900 @TarantoolBot document Title: add new field to box.info: 
>> replication_anon It is now possible to list all the anonymous 
>> replicas following the instance with a call to 
>> `box.info.replication_anon()` The output is similar to the one 
>> produced by `box.info.replication` with an exception that anonymous 
>> replicas are indexed by their uuid strings rather then server ids, 
>> since server ids have no meaning for anonymous replicas. Note, that 
>> when you issue a plain `box.info.replication_anon`, the only info 
>> returned is the number of anonymous replicas following the current 
>> instance. In order to see the full stats, you have to call 
>> `box.info.replication_anon()` . This is done to not overload the 
>> `box.info` output with excess info, since there may be lots of 
>> anonymous replicas. 
> Double whitespaces on this line and previous.
Fixed.
> Also see 2 nits about comments below.
>> diff --git a/src/box/lua/info.c b/src/box/lua/info.c index 
>> a789f7822..e7318fcd1 100644 --- a/src/box/lua/info.c +++ 
>> b/src/box/lua/info.c @@ -214,6 +214,54 @@ 
>> lbox_info_replication(struct lua_State *L) return 1; } +static int 
>> +lbox_info_replication_anon_call(struct lua_State *L) +{ + 
>> lua_newtable(L); /* box.info.replication_anon */ + + /* Nice 
>> formatting */ + lua_newtable(L); /* metatable */ 
> 1. Better write comments on separate lines, use '.' in the end of 
> sentences and capital letters when start them.
I wanted to copy the comment  style from other places  in the file.
Fixed.
>> + lua_pushliteral(L, "mapping"); + lua_setfield(L, -2, 
>> "__serialize"); + lua_setmetatable(L, -2); + + 
>> replicaset_foreach(replica) { + if (!replica->anon) + continue; + + 
>> lua_pushstring(L, tt_uuid_str(&replica->uuid)); + lbox_pushreplica(L, 
>> replica); + + lua_settable(L, -3); + } + + return 1; +} + +static int 
>> +lbox_info_replication_anon(struct lua_State *L) +{ + /* + * Make the 
>> .replication_anon field callable in order to + * not flood the output 
>> with possibly lots of anonymous + * replicas on box.info call. + */ + 
>> lua_newtable(L); + + lua_pushliteral(L, "count"); + 
>> lua_pushinteger(L, replicaset.anon_count); + lua_settable(L, -3); + + 
>> lua_newtable(L); /* metatable */ 
> 2. Here too.
Fixed.  The diff's below.
>> + + lua_pushstring(L, "__call"); + lua_pushcfunction(L, 
>> lbox_info_replication_anon_call); + lua_settable(L, -3); + + 
>> lua_setmetatable(L, -2); + return 1; +}
diff --git a/src/box/lua/info.c b/src/box/lua/info.c
index e7318fcd1..1e77737fd 100644
--- a/src/box/lua/info.c
+++ b/src/box/lua/info.c
@@ -217,10 +217,10 @@ lbox_info_replication(struct lua_State *L)
static int
lbox_info_replication_anon_call(struct lua_State *L)
{
- lua_newtable(L); /* box.info.replication_anon */
+ lua_newtable(L);

- /* Nice formatting */
- lua_newtable(L); /* metatable */
+ /* Metatable. */
+ lua_newtable(L);
lua_pushliteral(L, "mapping");
lua_setfield(L, -2, "__serialize");
lua_setmetatable(L, -2);
@@ -252,7 +252,8 @@ lbox_info_replication_anon(struct lua_State *L)
lua_pushinteger(L, replicaset.anon_count);
lua_settable(L, -3);

- lua_newtable(L); /* metatable */
+ /* Metatable. */
+ lua_newtable(L);

lua_pushstring(L, "__call");
lua_pushcfunction(L, lbox_info_replication_anon_call);



--

Serge Petrenko

Re: [Tarantool-patches] [PATCH v2] replication: add box.info.replication_anon

[Vladislav Shpilevoy]

Hi! Thanks for the fixes!

LGTM.

Re: [Tarantool-patches] [PATCH v2] replication: add box.info.replication_anon

[Kirill Yukhin]

Hello,

On 24 апр 19:28, Serge Petrenko wrote:
> Closes #4900
> 
> @TarantoolBot document
> Title: add new field to box.info: replication_anon
> 
> It is now possible to list all the anonymous replicas following the
> instance with a call to `box.info.replication_anon()`
> The output is similar to the one produced by `box.info.replication` with
> an exception that anonymous replicas are indexed by their uuid strings
> rather then server ids, since server ids have no meaning for anonymous
> replicas.
> 
> Example:
> ```
> tarantool> box.info.replication_anon
> ---
> - []
> ...
> 
> tarantool> box.info.replication_anon()
> ---
> - 02757524-1887-4c75-9137-d5c191fba507:
>     id: 0
>     uuid: 02757524-1887-4c75-9137-d5c191fba507
>     lsn: 0
>     downstream:
>       status: follow
>       idle: 0.43651100000716
>       vclock: {1: 1}
>   937a8292-8f99-4d11-aede-b97e9f5e9473:
>     id: 0
>     uuid: 937a8292-8f99-4d11-aede-b97e9f5e9473
>     lsn: 0
>     downstream:
>       status: follow
>       idle: 0.5189309999987
>       vclock: {1: 1}
> ...
> 
> ```
> 
> Note, that anonymous replicas hide their lsn from the others, so
> anonymous replica lsn will always be reported as zero, even if anonymous
> replicas perform some local space operations.
> To know the anonymous replica's lsn, you have to issue `box.info.lsn` on
> it.
> ---
> https://github.com/tarantool/tarantool/issues/4900
> https://github.com/tarantool/tarantool/tree/sp/gh-4900-list-anon-replicas

I've checked your patch into master.

--
Regards, Kirill Yukhin