From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp45.i.mail.ru (smtp45.i.mail.ru [94.100.177.105]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 3420A469719 for ; Sat, 22 Feb 2020 11:34:49 +0300 (MSK) Date: Sat, 22 Feb 2020 11:34:46 +0300 From: Sergey Ostanevich Message-ID: <20200222083446.GF68447@tarantool.org> References: <20200221195140.GE68447@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20200221195140.GE68447@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH 1/1] iproto: show real port in logs and box.info.listen List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy Cc: tarantool-patches@dev.tarantool.org 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/: for UNIX domain sockets; > > - : for IPv4; > > - [ip]: 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) > >