* Re: [Tarantool-patches] [PATCH 1/1] iproto: show real port in logs and box.info.listen
2019-11-12 22:03 [Tarantool-patches] [PATCH 1/1] iproto: show real port in logs and box.info.listen Vladislav Shpilevoy
@ 2019-11-13 0:32 ` Vladislav Shpilevoy
2019-11-13 22:31 ` Vladislav Shpilevoy
` (4 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Vladislav Shpilevoy @ 2019-11-13 0:32 UTC (permalink / raw)
To: tarantool-patches, kostja.osipov, alexander.turenko
Sorry, forgot a test. Will add tomorrow.
On 12/11/2019 23:03, Vladislav Shpilevoy wrote:
> Box.cfg{listen = 0} automatically chooses a port. But it was
> impossible to learn a real port the instance is bound to.
>
> An ability to see a real port may help to make test-run more
> robust, because it won't depend on which ports are free, and
> won't need to pre-choose them in advance.
>
> Now box.info.listen shows a real address, or nil when listen is
> turned off. Also a real address is logged instead of the dummy
> 0-port one.
>
> Closes #4620
>
> @TarantoolBot document
> Title: box.info.listen - real address
>
> New value in box.info - listen. It is a real address to which the
> instance was bound. For example, if box.cfg.listen was set with
> a zero port, box.info.listen will show a real port. The address
> is stored as a string:
>
> - unix/:<path> for UNIX domain sockets;
> - <ip>:<port> for IPv4;
> - [ip]:<port> for IPv6.
>
> If the instance does not listen anything, box.info.listen is nil.
> ---
> Branch: https://github.com/tarantool/tarantool/tree/gerold103/gh-4620-show-listen-port
> Issue: https://github.com/tarantool/tarantool/issues/4620
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [Tarantool-patches] [PATCH 1/1] iproto: show real port in logs and box.info.listen
2019-11-12 22:03 [Tarantool-patches] [PATCH 1/1] iproto: show real port in logs and box.info.listen Vladislav Shpilevoy
2019-11-13 0:32 ` Vladislav Shpilevoy
@ 2019-11-13 22:31 ` Vladislav Shpilevoy
2020-02-21 19:51 ` Sergey Ostanevich
` (3 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Vladislav Shpilevoy @ 2019-11-13 22:31 UTC (permalink / raw)
To: tarantool-patches, kostja.osipov, alexander.turenko
Added a test. Now, please, do a review. Especially
Alexander - AFAIU you need this feature.
====================================================================
diff --git a/test/box-tap/cfg.test.lua b/test/box-tap/cfg.test.lua
index d529447bb..569b5f463 100755
--- a/test/box-tap/cfg.test.lua
+++ b/test/box-tap/cfg.test.lua
@@ -6,7 +6,7 @@ local socket = require('socket')
local fio = require('fio')
local uuid = require('uuid')
local msgpack = require('msgpack')
-test:plan(104)
+test:plan(108)
--------------------------------------------------------------------------------
-- Invalid values
@@ -592,6 +592,18 @@ box.cfg{read_only=true}
]]
test:is(run_script(code), PANIC, "panic on bootstrapping a read-only instance as master")
+--
+-- gh-4620: box.info.listen.
+--
+box.cfg{listen = box.NULL}
+test:is(nil, box.info.listen, 'no cfg.listen - no info.listen')
+
+box.cfg{listen = '127.0.0.1:0'}
+test:ok(box.info.listen:match('127.0.0.1'), 'real IP in info.listen')
+test:ok(not box.info.listen:match(':0'), 'real port in info.listen')
+
+box.cfg{listen = box.NULL}
+test:is(nil, box.info.listen, 'cfg.listen reset drops info.listen')
test:check()
os.exit(0)
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [Tarantool-patches] [PATCH 1/1] iproto: show real port in logs and box.info.listen
2019-11-12 22:03 [Tarantool-patches] [PATCH 1/1] iproto: show real port in logs and box.info.listen Vladislav Shpilevoy
2019-11-13 0:32 ` Vladislav Shpilevoy
2019-11-13 22:31 ` Vladislav Shpilevoy
@ 2020-02-21 19:51 ` Sergey Ostanevich
2020-02-22 8:34 ` Sergey Ostanevich
2020-02-22 15:00 ` Vladislav Shpilevoy
2020-02-22 15:00 ` Vladislav Shpilevoy
` (2 subsequent siblings)
5 siblings, 2 replies; 13+ messages in thread
From: Sergey Ostanevich @ 2020-02-21 19:51 UTC (permalink / raw)
To: Vladislav Shpilevoy; +Cc: tarantool-patches
Hi!
Thanks for the patch, LGTM with some comments nits below.
Sergos.
On 12 ноя 23:03, Vladislav Shpilevoy wrote:
> Box.cfg{listen = 0} automatically chooses a port. But it was
> impossible to learn a real port the instance is bound to.
^^^^^ obtain?
>
> An ability to see a real port may help to make test-run more
> robust, because it won't depend on which ports are free, and
> won't need to pre-choose them in advance.
>
> Now box.info.listen shows a real address, or nil when listen is
> turned off. Also a real address is logged instead of the dummy
> 0-port one.
>
> Closes #4620
>
> @TarantoolBot document
> Title: box.info.listen - real address
>
> New value in box.info - listen. It is a real address to which the
> instance was bound. For example, if box.cfg.listen was set with
> a zero port, box.info.listen will show a real port. The address
> is stored as a string:
>
> - unix/:<path> for UNIX domain sockets;
> - <ip>:<port> for IPv4;
> - [ip]:<port> for IPv6.
>
> If the instance does not listen anything, box.info.listen is nil.
> ---
> Branch: https://github.com/tarantool/tarantool/tree/gerold103/gh-4620-show-listen-port
> Issue: https://github.com/tarantool/tarantool/issues/4620
>
> src/box/iproto.cc | 31 +++++++++++++++++++++++++++++--
> src/box/iproto.h | 7 +++++++
> src/box/lua/info.c | 9 +++++++++
> src/lib/core/evio.c | 8 ++++++++
> test/box/info.result | 1 +
> 5 files changed, 54 insertions(+), 2 deletions(-)
>
> diff --git a/src/box/iproto.cc b/src/box/iproto.cc
> index 34c8f469a..522c066be 100644
> --- a/src/box/iproto.cc
> +++ b/src/box/iproto.cc
> @@ -129,6 +129,23 @@ unsigned iproto_readahead = 16320;
> /* The maximal number of iproto messages in fly. */
> static int iproto_msg_max = IPROTO_MSG_MAX_MIN;
>
> +/**
> + * Address the iproto listens for, stored in TX
> + * thread. Is kept in TX to be showed in box.info.
shown, since V3?
> + */
> +static struct sockaddr_storage iproto_bound_address_storage;
> +/** 0 means that no address is listened. */
> +static socklen_t iproto_bound_address_len;
> +
> +const char *
> +iproto_bound_address(void)
> +{
> + if (iproto_bound_address_len == 0)
> + return NULL;
> + return sio_strfaddr((struct sockaddr *) &iproto_bound_address_storage,
> + iproto_bound_address_len);
> +}
> +
> /**
> * How big is a buffer which needs to be shrunk before
> * it is put back into buffer cache.
> @@ -2081,8 +2098,14 @@ struct iproto_cfg_msg: public cbus_call_msg
> /** Operation to execute in iproto thread. */
> enum iproto_cfg_op op;
> union {
> - /** New URI to bind to. */
> - const char *uri;
> + struct {
> + /** New URI to bind to. */
> + const char *uri;
> + /** Result address. */
> + struct sockaddr_storage addr;
> + /** Address length. */
> + socklen_t addrlen;
> + };
>
> /** New iproto max message count. */
> int iproto_msg_max;
> @@ -2118,6 +2141,8 @@ iproto_do_cfg_f(struct cbus_call_msg *m)
> (evio_service_bind(&binary, cfg_msg->uri) != 0 ||
> evio_service_listen(&binary) != 0))
> diag_raise();
> + cfg_msg->addrlen = binary.addr_len;
> + cfg_msg->addr = binary.addrstorage;
> break;
> default:
> unreachable();
> @@ -2143,6 +2168,8 @@ iproto_listen(const char *uri)
> iproto_cfg_msg_create(&cfg_msg, IPROTO_CFG_LISTEN);
> cfg_msg.uri = uri;
> iproto_do_cfg(&cfg_msg);
> + iproto_bound_address_storage = cfg_msg.addr;
> + iproto_bound_address_len = cfg_msg.addrlen;
> }
>
> size_t
> diff --git a/src/box/iproto.h b/src/box/iproto.h
> index edb24a7ed..201e09df5 100644
> --- a/src/box/iproto.h
> +++ b/src/box/iproto.h
> @@ -80,6 +80,13 @@ iproto_request_count(void);
> void
> iproto_reset_stat(void);
>
> +/**
> + * String representation of the address served by
> + * iproto. To be showed in box.info.
shown
> + */
> +const char *
> +iproto_bound_address(void);
> +
> #if defined(__cplusplus)
> } /* extern "C" */
>
> diff --git a/src/box/lua/info.c b/src/box/lua/info.c
> index 55382fd77..98579e8df 100644
> --- a/src/box/lua/info.c
> +++ b/src/box/lua/info.c
> @@ -485,6 +485,14 @@ lbox_info_vinyl(struct lua_State *L)
> return 1;
> }
>
> +static int
> +lbox_info_listen(struct lua_State *L)
> +{
> + /* NULL is ok, no need to check. */
> + lua_pushstring(L, iproto_bound_address());
> + return 1;
> +}
> +
> static const struct luaL_Reg lbox_info_dynamic_meta[] = {
> {"id", lbox_info_id},
> {"uuid", lbox_info_uuid},
> @@ -500,6 +508,7 @@ static const struct luaL_Reg lbox_info_dynamic_meta[] = {
> {"memory", lbox_info_memory},
> {"gc", lbox_info_gc},
> {"vinyl", lbox_info_vinyl},
> + {"listen", lbox_info_listen},
> {NULL, NULL}
> };
>
> diff --git a/src/lib/core/evio.c b/src/lib/core/evio.c
> index 2152c15e6..fc8f00e0e 100644
> --- a/src/lib/core/evio.c
> +++ b/src/lib/core/evio.c
> @@ -269,6 +269,13 @@ evio_service_bind_addr(struct evio_service *service)
> }
> }
>
> + /*
> + * After binding a result address may be different. For
> + * example, if a port was 0.
> + */
> + if (sio_getsockname(fd, &service->addr, &service->addr_len) != 0)
> + goto error;
> +
> say_info("%s: bound to %s", evio_service_name(service),
> sio_strfaddr(&service->addr, service->addr_len));
>
> @@ -400,6 +407,7 @@ evio_service_stop(struct evio_service *service)
>
> if (ev_is_active(&service->ev)) {
> ev_io_stop(service->loop, &service->ev);
> + service->addr_len = 0;
> }
>
> if (service->ev.fd >= 0) {
> diff --git a/test/box/info.result b/test/box/info.result
> index af81f7add..4dc888616 100644
> --- a/test/box/info.result
> +++ b/test/box/info.result
> @@ -77,6 +77,7 @@ t
> - - cluster
> - gc
> - id
> + - listen
> - lsn
> - memory
> - package
> --
> 2.21.0 (Apple Git-122.2)
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [Tarantool-patches] [PATCH 1/1] iproto: show real port in logs and box.info.listen
2020-02-21 19:51 ` Sergey Ostanevich
@ 2020-02-22 8:34 ` Sergey Ostanevich
2020-02-22 14:47 ` Vladislav Shpilevoy
2020-02-22 15:00 ` Vladislav Shpilevoy
1 sibling, 1 reply; 13+ messages in thread
From: Sergey Ostanevich @ 2020-02-22 8:34 UTC (permalink / raw)
To: Vladislav Shpilevoy; +Cc: tarantool-patches
Vlad, after thinking over the patch some more I've got a feeling it can
break backward compatibility. Can Tarantool with these changes in iproto
successfully communicate to the one without the changes?
@Totktonada we need to formulate the cross-version testing.
Regards,
Sergos
On 21 фев 22:51, Sergey Ostanevich wrote:
> Hi!
>
> Thanks for the patch, LGTM with some comments nits below.
>
> Sergos.
>
>
> On 12 ноя 23:03, Vladislav Shpilevoy wrote:
> > Box.cfg{listen = 0} automatically chooses a port. But it was
> > impossible to learn a real port the instance is bound to.
> ^^^^^ obtain?
> >
> > An ability to see a real port may help to make test-run more
> > robust, because it won't depend on which ports are free, and
> > won't need to pre-choose them in advance.
> >
> > Now box.info.listen shows a real address, or nil when listen is
> > turned off. Also a real address is logged instead of the dummy
> > 0-port one.
> >
> > Closes #4620
> >
> > @TarantoolBot document
> > Title: box.info.listen - real address
> >
> > New value in box.info - listen. It is a real address to which the
> > instance was bound. For example, if box.cfg.listen was set with
> > a zero port, box.info.listen will show a real port. The address
> > is stored as a string:
> >
> > - unix/:<path> for UNIX domain sockets;
> > - <ip>:<port> for IPv4;
> > - [ip]:<port> for IPv6.
> >
> > If the instance does not listen anything, box.info.listen is nil.
> > ---
> > Branch: https://github.com/tarantool/tarantool/tree/gerold103/gh-4620-show-listen-port
> > Issue: https://github.com/tarantool/tarantool/issues/4620
> >
> > src/box/iproto.cc | 31 +++++++++++++++++++++++++++++--
> > src/box/iproto.h | 7 +++++++
> > src/box/lua/info.c | 9 +++++++++
> > src/lib/core/evio.c | 8 ++++++++
> > test/box/info.result | 1 +
> > 5 files changed, 54 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/box/iproto.cc b/src/box/iproto.cc
> > index 34c8f469a..522c066be 100644
> > --- a/src/box/iproto.cc
> > +++ b/src/box/iproto.cc
> > @@ -129,6 +129,23 @@ unsigned iproto_readahead = 16320;
> > /* The maximal number of iproto messages in fly. */
> > static int iproto_msg_max = IPROTO_MSG_MAX_MIN;
> >
> > +/**
> > + * Address the iproto listens for, stored in TX
> > + * thread. Is kept in TX to be showed in box.info.
> shown, since V3?
> > + */
> > +static struct sockaddr_storage iproto_bound_address_storage;
> > +/** 0 means that no address is listened. */
> > +static socklen_t iproto_bound_address_len;
> > +
> > +const char *
> > +iproto_bound_address(void)
> > +{
> > + if (iproto_bound_address_len == 0)
> > + return NULL;
> > + return sio_strfaddr((struct sockaddr *) &iproto_bound_address_storage,
> > + iproto_bound_address_len);
> > +}
> > +
> > /**
> > * How big is a buffer which needs to be shrunk before
> > * it is put back into buffer cache.
> > @@ -2081,8 +2098,14 @@ struct iproto_cfg_msg: public cbus_call_msg
> > /** Operation to execute in iproto thread. */
> > enum iproto_cfg_op op;
> > union {
> > - /** New URI to bind to. */
> > - const char *uri;
> > + struct {
> > + /** New URI to bind to. */
> > + const char *uri;
> > + /** Result address. */
> > + struct sockaddr_storage addr;
> > + /** Address length. */
> > + socklen_t addrlen;
> > + };
> >
> > /** New iproto max message count. */
> > int iproto_msg_max;
> > @@ -2118,6 +2141,8 @@ iproto_do_cfg_f(struct cbus_call_msg *m)
> > (evio_service_bind(&binary, cfg_msg->uri) != 0 ||
> > evio_service_listen(&binary) != 0))
> > diag_raise();
> > + cfg_msg->addrlen = binary.addr_len;
> > + cfg_msg->addr = binary.addrstorage;
> > break;
> > default:
> > unreachable();
> > @@ -2143,6 +2168,8 @@ iproto_listen(const char *uri)
> > iproto_cfg_msg_create(&cfg_msg, IPROTO_CFG_LISTEN);
> > cfg_msg.uri = uri;
> > iproto_do_cfg(&cfg_msg);
> > + iproto_bound_address_storage = cfg_msg.addr;
> > + iproto_bound_address_len = cfg_msg.addrlen;
> > }
> >
> > size_t
> > diff --git a/src/box/iproto.h b/src/box/iproto.h
> > index edb24a7ed..201e09df5 100644
> > --- a/src/box/iproto.h
> > +++ b/src/box/iproto.h
> > @@ -80,6 +80,13 @@ iproto_request_count(void);
> > void
> > iproto_reset_stat(void);
> >
> > +/**
> > + * String representation of the address served by
> > + * iproto. To be showed in box.info.
> shown
> > + */
> > +const char *
> > +iproto_bound_address(void);
> > +
> > #if defined(__cplusplus)
> > } /* extern "C" */
> >
> > diff --git a/src/box/lua/info.c b/src/box/lua/info.c
> > index 55382fd77..98579e8df 100644
> > --- a/src/box/lua/info.c
> > +++ b/src/box/lua/info.c
> > @@ -485,6 +485,14 @@ lbox_info_vinyl(struct lua_State *L)
> > return 1;
> > }
> >
> > +static int
> > +lbox_info_listen(struct lua_State *L)
> > +{
> > + /* NULL is ok, no need to check. */
> > + lua_pushstring(L, iproto_bound_address());
> > + return 1;
> > +}
> > +
> > static const struct luaL_Reg lbox_info_dynamic_meta[] = {
> > {"id", lbox_info_id},
> > {"uuid", lbox_info_uuid},
> > @@ -500,6 +508,7 @@ static const struct luaL_Reg lbox_info_dynamic_meta[] = {
> > {"memory", lbox_info_memory},
> > {"gc", lbox_info_gc},
> > {"vinyl", lbox_info_vinyl},
> > + {"listen", lbox_info_listen},
> > {NULL, NULL}
> > };
> >
> > diff --git a/src/lib/core/evio.c b/src/lib/core/evio.c
> > index 2152c15e6..fc8f00e0e 100644
> > --- a/src/lib/core/evio.c
> > +++ b/src/lib/core/evio.c
> > @@ -269,6 +269,13 @@ evio_service_bind_addr(struct evio_service *service)
> > }
> > }
> >
> > + /*
> > + * After binding a result address may be different. For
> > + * example, if a port was 0.
> > + */
> > + if (sio_getsockname(fd, &service->addr, &service->addr_len) != 0)
> > + goto error;
> > +
> > say_info("%s: bound to %s", evio_service_name(service),
> > sio_strfaddr(&service->addr, service->addr_len));
> >
> > @@ -400,6 +407,7 @@ evio_service_stop(struct evio_service *service)
> >
> > if (ev_is_active(&service->ev)) {
> > ev_io_stop(service->loop, &service->ev);
> > + service->addr_len = 0;
> > }
> >
> > if (service->ev.fd >= 0) {
> > diff --git a/test/box/info.result b/test/box/info.result
> > index af81f7add..4dc888616 100644
> > --- a/test/box/info.result
> > +++ b/test/box/info.result
> > @@ -77,6 +77,7 @@ t
> > - - cluster
> > - gc
> > - id
> > + - listen
> > - lsn
> > - memory
> > - package
> > --
> > 2.21.0 (Apple Git-122.2)
> >
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [Tarantool-patches] [PATCH 1/1] iproto: show real port in logs and box.info.listen
2020-02-22 8:34 ` Sergey Ostanevich
@ 2020-02-22 14:47 ` Vladislav Shpilevoy
0 siblings, 0 replies; 13+ messages in thread
From: Vladislav Shpilevoy @ 2020-02-22 14:47 UTC (permalink / raw)
To: Sergey Ostanevich; +Cc: tarantool-patches
On 22/02/2020 09:34, Sergey Ostanevich wrote:
> Vlad, after thinking over the patch some more I've got a feeling it can
> break backward compatibility. Can Tarantool with these changes in iproto
> successfully communicate to the one without the changes?
Why would it break anything? This has nothing to do with the protocol
or network communication. This is just a new record in box.info table.
I don't see how is it related to cross version interaction.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Tarantool-patches] [PATCH 1/1] iproto: show real port in logs and box.info.listen
2020-02-21 19:51 ` Sergey Ostanevich
2020-02-22 8:34 ` Sergey Ostanevich
@ 2020-02-22 15:00 ` Vladislav Shpilevoy
2020-03-03 13:26 ` Sergey Ostanevich
1 sibling, 1 reply; 13+ messages in thread
From: Vladislav Shpilevoy @ 2020-02-22 15:00 UTC (permalink / raw)
To: Sergey Ostanevich; +Cc: tarantool-patches
Hi! Thanks for the review!
>> Box.cfg{listen = 0} automatically chooses a port. But it was
>> impossible to learn a real port the instance is bound to.
> ^^^^^ obtain?
Done.
>> diff --git a/src/box/iproto.cc b/src/box/iproto.cc
>> index 34c8f469a..522c066be 100644
>> --- a/src/box/iproto.cc
>> +++ b/src/box/iproto.cc
>> @@ -129,6 +129,23 @@ unsigned iproto_readahead = 16320;
>> /* The maximal number of iproto messages in fly. */
>> static int iproto_msg_max = IPROTO_MSG_MAX_MIN;
>>
>> +/**
>> + * Address the iproto listens for, stored in TX
>> + * thread. Is kept in TX to be showed in box.info.
> shown, since V3?
Done.
>> + */
>> +static struct sockaddr_storage iproto_bound_address_storage;
>> +/** 0 means that no address is listened. */
>> +static socklen_t iproto_bound_address_len;
>> +
>> +const char *
>> +iproto_bound_address(void)
>> +{
>> + if (iproto_bound_address_len == 0)
>> + return NULL;
>> + return sio_strfaddr((struct sockaddr *) &iproto_bound_address_storage,
>> + iproto_bound_address_len);
>> +}
>> diff --git a/src/box/iproto.h b/src/box/iproto.h
>> index edb24a7ed..201e09df5 100644
>> --- a/src/box/iproto.h
>> +++ b/src/box/iproto.h
>> @@ -80,6 +80,13 @@ iproto_request_count(void);
>> void
>> iproto_reset_stat(void);
>>
>> +/**
>> + * String representation of the address served by
>> + * iproto. To be showed in box.info.
> shown
Done.
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [Tarantool-patches] [PATCH 1/1] iproto: show real port in logs and box.info.listen
2020-02-22 15:00 ` Vladislav Shpilevoy
@ 2020-03-03 13:26 ` Sergey Ostanevich
0 siblings, 0 replies; 13+ messages in thread
From: Sergey Ostanevich @ 2020-03-03 13:26 UTC (permalink / raw)
To: Vladislav Shpilevoy; +Cc: tarantool-patches
Thanks, LGTM.
The misunderstanding with protocol was my off-the-source fault.
Sergos
On 22 фев 16:00, Vladislav Shpilevoy wrote:
> Hi! Thanks for the review!
>
> >> Box.cfg{listen = 0} automatically chooses a port. But it was
> >> impossible to learn a real port the instance is bound to.
> > ^^^^^ obtain?
>
> Done.
>
> >> diff --git a/src/box/iproto.cc b/src/box/iproto.cc
> >> index 34c8f469a..522c066be 100644
> >> --- a/src/box/iproto.cc
> >> +++ b/src/box/iproto.cc
> >> @@ -129,6 +129,23 @@ unsigned iproto_readahead = 16320;
> >> /* The maximal number of iproto messages in fly. */
> >> static int iproto_msg_max = IPROTO_MSG_MAX_MIN;
> >>
> >> +/**
> >> + * Address the iproto listens for, stored in TX
> >> + * thread. Is kept in TX to be showed in box.info.
> > shown, since V3?
>
> Done.
>
> >> + */
> >> +static struct sockaddr_storage iproto_bound_address_storage;
> >> +/** 0 means that no address is listened. */
> >> +static socklen_t iproto_bound_address_len;
> >> +
> >> +const char *
> >> +iproto_bound_address(void)
> >> +{
> >> + if (iproto_bound_address_len == 0)
> >> + return NULL;
> >> + return sio_strfaddr((struct sockaddr *) &iproto_bound_address_storage,
> >> + iproto_bound_address_len);
> >> +}
> >> diff --git a/src/box/iproto.h b/src/box/iproto.h
> >> index edb24a7ed..201e09df5 100644
> >> --- a/src/box/iproto.h
> >> +++ b/src/box/iproto.h
> >> @@ -80,6 +80,13 @@ iproto_request_count(void);
> >> void
> >> iproto_reset_stat(void);
> >>
> >> +/**
> >> + * String representation of the address served by
> >> + * iproto. To be showed in box.info.
> > shown
>
> Done.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Tarantool-patches] [PATCH 1/1] iproto: show real port in logs and box.info.listen
2019-11-12 22:03 [Tarantool-patches] [PATCH 1/1] iproto: show real port in logs and box.info.listen Vladislav Shpilevoy
` (2 preceding siblings ...)
2020-02-21 19:51 ` Sergey Ostanevich
@ 2020-02-22 15:00 ` Vladislav Shpilevoy
2020-03-18 18:18 ` Serge Petrenko
2020-03-19 10:45 ` Kirill Yukhin
5 siblings, 0 replies; 13+ messages in thread
From: Vladislav Shpilevoy @ 2020-02-22 15:00 UTC (permalink / raw)
To: tarantool-patches, kostja.osipov, alexander.turenko
Since the patch is very old, there was no ChangeLog
record added. Here it is:
@ChangeLog
- box.info.listen - new record in box.info, which shows a
real port when bound to port 0. For example, if box.cfg
'listen' parameter was set to '127.0.0.1:0', box.info.listen
will show '127.0.0.1:<real port > 0>' (gh-4620).
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [Tarantool-patches] [PATCH 1/1] iproto: show real port in logs and box.info.listen
2019-11-12 22:03 [Tarantool-patches] [PATCH 1/1] iproto: show real port in logs and box.info.listen Vladislav Shpilevoy
` (3 preceding siblings ...)
2020-02-22 15:00 ` Vladislav Shpilevoy
@ 2020-03-18 18:18 ` Serge Petrenko
2020-03-18 22:38 ` Vladislav Shpilevoy
2020-03-19 10:45 ` Kirill Yukhin
5 siblings, 1 reply; 13+ messages in thread
From: Serge Petrenko @ 2020-03-18 18:18 UTC (permalink / raw)
To: Vladislav Shpilevoy; +Cc: tarantool-patches
[-- Attachment #1: Type: text/plain, Size: 6405 bytes --]
Hi! Thanks for the patch!
Alexander asked me to do the 2nd review.
The patch LGTM.
Sorry for nitpicking, but looks like your changelog request has a typo in it:
> @ChangeLog
> - box.info.listen - new record in box.info <http://box.info/>, which shows a
> real port when bound to port 0. For example, if box.cfg
> 'listen' parameter was set to '127.0.0.1:0', box.info.listen
> will show '127.0.0.1:<real port > 0>' (gh-4620).
just <real port>, without 0>, right?
--
Serge Petrenko
sergepetrenko@tarantool.org
> 13 нояб. 2019 г., в 01:03, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> написал(а):
>
> Box.cfg{listen = 0} automatically chooses a port. But it was
> impossible to learn a real port the instance is bound to.
>
> An ability to see a real port may help to make test-run more
> robust, because it won't depend on which ports are free, and
> won't need to pre-choose them in advance.
>
> Now box.info.listen shows a real address, or nil when listen is
> turned off. Also a real address is logged instead of the dummy
> 0-port one.
>
> Closes #4620
>
> @TarantoolBot document
> Title: box.info.listen - real address
>
> New value in box.info - listen. It is a real address to which the
> instance was bound. For example, if box.cfg.listen was set with
> a zero port, box.info.listen will show a real port. The address
> is stored as a string:
>
> - unix/:<path> for UNIX domain sockets;
> - <ip>:<port> for IPv4;
> - [ip]:<port> for IPv6.
>
> If the instance does not listen anything, box.info.listen is nil.
> ---
> Branch: https://github.com/tarantool/tarantool/tree/gerold103/gh-4620-show-listen-port
> Issue: https://github.com/tarantool/tarantool/issues/4620
>
> src/box/iproto.cc | 31 +++++++++++++++++++++++++++++--
> src/box/iproto.h | 7 +++++++
> src/box/lua/info.c | 9 +++++++++
> src/lib/core/evio.c | 8 ++++++++
> test/box/info.result | 1 +
> 5 files changed, 54 insertions(+), 2 deletions(-)
>
> diff --git a/src/box/iproto.cc b/src/box/iproto.cc
> index 34c8f469a..522c066be 100644
> --- a/src/box/iproto.cc
> +++ b/src/box/iproto.cc
> @@ -129,6 +129,23 @@ unsigned iproto_readahead = 16320;
> /* The maximal number of iproto messages in fly. */
> static int iproto_msg_max = IPROTO_MSG_MAX_MIN;
>
> +/**
> + * Address the iproto listens for, stored in TX
> + * thread. Is kept in TX to be showed in box.info.
> + */
> +static struct sockaddr_storage iproto_bound_address_storage;
> +/** 0 means that no address is listened. */
> +static socklen_t iproto_bound_address_len;
> +
> +const char *
> +iproto_bound_address(void)
> +{
> + if (iproto_bound_address_len == 0)
> + return NULL;
> + return sio_strfaddr((struct sockaddr *) &iproto_bound_address_storage,
> + iproto_bound_address_len);
> +}
> +
> /**
> * How big is a buffer which needs to be shrunk before
> * it is put back into buffer cache.
> @@ -2081,8 +2098,14 @@ struct iproto_cfg_msg: public cbus_call_msg
> /** Operation to execute in iproto thread. */
> enum iproto_cfg_op op;
> union {
> - /** New URI to bind to. */
> - const char *uri;
> + struct {
> + /** New URI to bind to. */
> + const char *uri;
> + /** Result address. */
> + struct sockaddr_storage addr;
> + /** Address length. */
> + socklen_t addrlen;
> + };
>
> /** New iproto max message count. */
> int iproto_msg_max;
> @@ -2118,6 +2141,8 @@ iproto_do_cfg_f(struct cbus_call_msg *m)
> (evio_service_bind(&binary, cfg_msg->uri) != 0 ||
> evio_service_listen(&binary) != 0))
> diag_raise();
> + cfg_msg->addrlen = binary.addr_len;
> + cfg_msg->addr = binary.addrstorage;
> break;
> default:
> unreachable();
> @@ -2143,6 +2168,8 @@ iproto_listen(const char *uri)
> iproto_cfg_msg_create(&cfg_msg, IPROTO_CFG_LISTEN);
> cfg_msg.uri = uri;
> iproto_do_cfg(&cfg_msg);
> + iproto_bound_address_storage = cfg_msg.addr;
> + iproto_bound_address_len = cfg_msg.addrlen;
> }
>
> size_t
> diff --git a/src/box/iproto.h b/src/box/iproto.h
> index edb24a7ed..201e09df5 100644
> --- a/src/box/iproto.h
> +++ b/src/box/iproto.h
> @@ -80,6 +80,13 @@ iproto_request_count(void);
> void
> iproto_reset_stat(void);
>
> +/**
> + * String representation of the address served by
> + * iproto. To be showed in box.info.
> + */
> +const char *
> +iproto_bound_address(void);
> +
> #if defined(__cplusplus)
> } /* extern "C" */
>
> diff --git a/src/box/lua/info.c b/src/box/lua/info.c
> index 55382fd77..98579e8df 100644
> --- a/src/box/lua/info.c
> +++ b/src/box/lua/info.c
> @@ -485,6 +485,14 @@ lbox_info_vinyl(struct lua_State *L)
> return 1;
> }
>
> +static int
> +lbox_info_listen(struct lua_State *L)
> +{
> + /* NULL is ok, no need to check. */
> + lua_pushstring(L, iproto_bound_address());
> + return 1;
> +}
> +
> static const struct luaL_Reg lbox_info_dynamic_meta[] = {
> {"id", lbox_info_id},
> {"uuid", lbox_info_uuid},
> @@ -500,6 +508,7 @@ static const struct luaL_Reg lbox_info_dynamic_meta[] = {
> {"memory", lbox_info_memory},
> {"gc", lbox_info_gc},
> {"vinyl", lbox_info_vinyl},
> + {"listen", lbox_info_listen},
> {NULL, NULL}
> };
>
> diff --git a/src/lib/core/evio.c b/src/lib/core/evio.c
> index 2152c15e6..fc8f00e0e 100644
> --- a/src/lib/core/evio.c
> +++ b/src/lib/core/evio.c
> @@ -269,6 +269,13 @@ evio_service_bind_addr(struct evio_service *service)
> }
> }
>
> + /*
> + * After binding a result address may be different. For
> + * example, if a port was 0.
> + */
> + if (sio_getsockname(fd, &service->addr, &service->addr_len) != 0)
> + goto error;
> +
> say_info("%s: bound to %s", evio_service_name(service),
> sio_strfaddr(&service->addr, service->addr_len));
>
> @@ -400,6 +407,7 @@ evio_service_stop(struct evio_service *service)
>
> if (ev_is_active(&service->ev)) {
> ev_io_stop(service->loop, &service->ev);
> + service->addr_len = 0;
> }
>
> if (service->ev.fd >= 0) {
> diff --git a/test/box/info.result b/test/box/info.result
> index af81f7add..4dc888616 100644
> --- a/test/box/info.result
> +++ b/test/box/info.result
> @@ -77,6 +77,7 @@ t
> - - cluster
> - gc
> - id
> + - listen
> - lsn
> - memory
> - package
> --
> 2.21.0 (Apple Git-122.2)
>
[-- Attachment #2: Type: text/html, Size: 16261 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [Tarantool-patches] [PATCH 1/1] iproto: show real port in logs and box.info.listen
2020-03-18 18:18 ` Serge Petrenko
@ 2020-03-18 22:38 ` Vladislav Shpilevoy
2020-03-19 10:24 ` Serge Petrenko
0 siblings, 1 reply; 13+ messages in thread
From: Vladislav Shpilevoy @ 2020-03-18 22:38 UTC (permalink / raw)
To: Serge Petrenko; +Cc: tarantool-patches
Thanks for the review!
On 18/03/2020 19:18, Serge Petrenko wrote:
> Hi! Thanks for the patch!
> Alexander asked me to do the 2nd review.
>
> The patch LGTM.
>
> Sorry for nitpicking, but looks like your changelog request has a typo in it:
>> @ChangeLog
>> - box.info.listen - new record in box.info <http://box.info>, which shows a
>> real port when bound to port 0. For example, if box.cfg
>> 'listen' parameter was set to '127.0.0.1:0', box.info.listen
>> will show '127.0.0.1:<real port > 0>' (gh-4620).
> just <real port>, without 0>, right?
Nope, I meant > 0. I wanted to emphasize, that the port is never
shown as 0 anymore. It is always some real port > 0.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Tarantool-patches] [PATCH 1/1] iproto: show real port in logs and box.info.listen
2020-03-18 22:38 ` Vladislav Shpilevoy
@ 2020-03-19 10:24 ` Serge Petrenko
0 siblings, 0 replies; 13+ messages in thread
From: Serge Petrenko @ 2020-03-19 10:24 UTC (permalink / raw)
To: Vladislav Shpilevoy; +Cc: tarantool-patches
> 19 марта 2020 г., в 01:38, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> написал(а):
>
> Thanks for the review!
>
> On 18/03/2020 19:18, Serge Petrenko wrote:
>> Hi! Thanks for the patch!
>> Alexander asked me to do the 2nd review.
>>
>> The patch LGTM.
>>
>> Sorry for nitpicking, but looks like your changelog request has a typo in it:
>>> @ChangeLog
>>> - box.info.listen - new record in box.info <http://box.info>, which shows a
>>> real port when bound to port 0. For example, if box.cfg
>>> 'listen' parameter was set to '127.0.0.1:0', box.info.listen
>>> will show '127.0.0.1:<real port > 0>' (gh-4620).
>> just <real port>, without 0>, right?
>
> Nope, I meant > 0. I wanted to emphasize, that the port is never
> shown as 0 anymore. It is always some real port > 0.
Ah, I see. Sorry for the misunderstanding.
--
Serge Petrenko
sergepetrenko@tarantool.org
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Tarantool-patches] [PATCH 1/1] iproto: show real port in logs and box.info.listen
2019-11-12 22:03 [Tarantool-patches] [PATCH 1/1] iproto: show real port in logs and box.info.listen Vladislav Shpilevoy
` (4 preceding siblings ...)
2020-03-18 18:18 ` Serge Petrenko
@ 2020-03-19 10:45 ` Kirill Yukhin
5 siblings, 0 replies; 13+ messages in thread
From: Kirill Yukhin @ 2020-03-19 10:45 UTC (permalink / raw)
To: Vladislav Shpilevoy; +Cc: tarantool-patches
Hello,
On 12 ноя 23:03, Vladislav Shpilevoy wrote:
> Box.cfg{listen = 0} automatically chooses a port. But it was
> impossible to learn a real port the instance is bound to.
>
> An ability to see a real port may help to make test-run more
> robust, because it won't depend on which ports are free, and
> won't need to pre-choose them in advance.
>
> Now box.info.listen shows a real address, or nil when listen is
> turned off. Also a real address is logged instead of the dummy
> 0-port one.
>
> Closes #4620
>
> @TarantoolBot document
> Title: box.info.listen - real address
>
> New value in box.info - listen. It is a real address to which the
> instance was bound. For example, if box.cfg.listen was set with
> a zero port, box.info.listen will show a real port. The address
> is stored as a string:
>
> - unix/:<path> for UNIX domain sockets;
> - <ip>:<port> for IPv4;
> - [ip]:<port> for IPv6.
>
> If the instance does not listen anything, box.info.listen is nil.
> ---
> Branch: https://github.com/tarantool/tarantool/tree/gerold103/gh-4620-show-listen-port
> Issue: https://github.com/tarantool/tarantool/issues/4620
I've checked your patch into master.
--
Regards, Kirill Yukhin
^ permalink raw reply [flat|nested] 13+ messages in thread