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

[Serge Petrenko <sergepetrenko@tarantool.org>]

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