From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Serge Petrenko Message-Id: <52639C3D-E897-427C-BC2A-E71D418DC689@tarantool.org> Content-Type: multipart/alternative; boundary="Apple-Mail=_92FC4F6C-1979-4B41-A6D9-EB037C841808" Mime-Version: 1.0 (Mac OS X Mail 11.5 \(3445.9.1\)) Subject: Re: [tarantool-patches] [PATCH] [replication] make join stage more informative Date: Thu, 18 Oct 2018 16:38:06 +0300 In-Reply-To: <20181018124313.axjdkmf6ftklgs2c@esperanza> References: <20181016101053.21594-1-sergepetrenko@tarantool.org> <20181018124313.axjdkmf6ftklgs2c@esperanza> To: Vladimir Davydov Cc: tarantool-patches@freelists.org List-ID: --Apple-Mail=_92FC4F6C-1979-4B41-A6D9-EB037C841808 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 HI! Thank you for review. The new diff is below. > 18 =D0=BE=D0=BA=D1=82. 2018 =D0=B3., =D0=B2 15:43, Vladimir Davydov = =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0=D0=BB(=D0=B0= ): >=20 > On Tue, Oct 16, 2018 at 01:10:53PM +0300, Serge Petrenko wrote: >>=20 >> + if (row_count % 100000 =3D=3D 0) >> + say_info("%.1fM rows recieved", = row_count / 1e6); >=20 > Without knowing the total number of rows to be received, this isn't = very > informative IMO. The same is done in logging during recovery. Here it at least lets you = make a rough time estimation. >=20 >> } else if (row.type =3D=3D IPROTO_OK) { >> if (applier->version_id < version_id(1, 7, 0)) { >> /* >> @@ -354,6 +358,9 @@ applier_join(struct applier *applier) >> if (iproto_type_is_dml(row.type)) { >> vclock_follow_xrow(&replicaset.vclock, &row); >> xstream_write_xc(applier->subscribe_stream, = &row); >> + ++row_count; >> + if (row_count % 100000 =3D=3D 0) >> + say_info("%.1fM rows recieved", = row_count / 1e6); >=20 > When bootstrapping from Tarantool < 1.7, final join won't end here, it > will end in applier_subscribe(). Not sure if we want to handle this = case > though. I suggest not adding logging to the subscribe stage, it is not needed = IMO. >=20 >> } else if (row.type =3D=3D IPROTO_OK) { >> /* >> * Current vclock. This is not used now, >> diff --git a/src/box/lua/info.c b/src/box/lua/info.c >> index 655768ec4..4c25255c0 100644 >> --- a/src/box/lua/info.c >> +++ b/src/box/lua/info.c >> @@ -119,8 +119,13 @@ static void >> lbox_pushrelay(lua_State *L, struct relay *relay) >> { >> lua_newtable(L); >> - lua_pushstring(L, "vclock"); >> - lbox_pushvclock(L, relay_vclock(relay)); >> + if (relay_vclock(relay)->map =3D=3D 0) { >> + lua_pushstring(L, "status"); >> + lua_pushstring(L, "joining"); >> + } else { >> + lua_pushstring(L, "vclock"); >> + lbox_pushvclock(L, relay_vclock(relay)); >> + } >=20 > This wouldn't work, because struct replica doesn't exist before = initial > join is complete. Yes, you=E2=80=99re correct. Removed this. --- src/box/applier.cc | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/box/applier.cc b/src/box/applier.cc index 7da278e68..ba68c3d07 100644 --- a/src/box/applier.cc +++ b/src/box/applier.cc @@ -309,11 +309,15 @@ applier_join(struct applier *applier) * Receive initial data. */ assert(applier->join_stream !=3D NULL); + uint64_t row_count =3D 0; while (true) { coio_read_xrow(coio, ibuf, &row); applier->last_row_time =3D ev_monotonic_now(loop()); if (iproto_type_is_dml(row.type)) { xstream_write_xc(applier->join_stream, &row); + ++row_count; + if (row_count % 100000 =3D=3D 0) + say_info("%.1fM rows recieved", = row_count / 1e6); } else if (row.type =3D=3D IPROTO_OK) { if (applier->version_id < version_id(1, 7, 0)) { /* @@ -354,6 +358,9 @@ applier_join(struct applier *applier) if (iproto_type_is_dml(row.type)) { vclock_follow_xrow(&replicaset.vclock, &row); xstream_write_xc(applier->subscribe_stream, = &row); + ++row_count; + if (row_count % 100000 =3D=3D 0) + say_info("%.1fM rows recieved", = row_count / 1e6); } else if (row.type =3D=3D IPROTO_OK) { /* * Current vclock. This is not used now, --=20 2.17.1 (Apple Git-112) --Apple-Mail=_92FC4F6C-1979-4B41-A6D9-EB037C841808 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8 HI! Thank you for review.
The new diff is below.

18 =D0=BE=D0=BA=D1=82. 2018 =D0=B3., =D0=B2 = 15:43, Vladimir Davydov <vdavydov.dev@gmail.com> =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0= =B0=D0=BB(=D0=B0):

On Tue, Oct 16, 2018 at = 01:10:53PM +0300, Serge Petrenko wrote:

+ if = (row_count % 100000 =3D=3D 0)
+ = say_info("%.1fM rows recieved", row_count / 1e6);

Without knowing the total number = of rows to be received, this isn't very
informative = IMO.

The same is done in logging during recovery. Here it at least = lets you make a rough
time estimation.


} else if = (row.type =3D=3D IPROTO_OK) {
if (applier->version_id < = version_id(1, 7, 0)) {
/*
@@ -354,6 +358,9 = @@ applier_join(struct applier *applier)
if = (iproto_type_is_dml(row.type)) {
= vclock_follow_xrow(&replicaset.vclock, &row);
= = = xstream_write_xc(applier->subscribe_stream, &row);
+ = = = ++row_count;
+ if (row_count % 100000 =3D=3D = 0)
+ say_info("%.1fM rows recieved", = row_count / 1e6);

When = bootstrapping from Tarantool < 1.7, final join won't end here, it
will end in applier_subscribe(). Not sure if we want to = handle this case
though.

I suggest not adding logging to the = subscribe stage, it is not needed IMO.


} else if = (row.type =3D=3D IPROTO_OK) {
/*
 * = Current vclock. This is not used now,
diff --git = a/src/box/lua/info.c b/src/box/lua/info.c
index = 655768ec4..4c25255c0 100644
--- a/src/box/lua/info.c
+++ b/src/box/lua/info.c
@@ -119,8 +119,13 @@ = static void
lbox_pushrelay(lua_State *L, struct relay = *relay)
{
lua_newtable(L);
- = lua_pushstring(L, "vclock");
- = lbox_pushvclock(L, relay_vclock(relay));
+ if = (relay_vclock(relay)->map =3D=3D 0) {
+ = lua_pushstring(L, "status");
+ = lua_pushstring(L, "joining");
+ } else = {
+= = lua_pushstring(L, "vclock");
+ = lbox_pushvclock(L, relay_vclock(relay));
+ }

This wouldn't work, because = struct replica doesn't exist before initial
join is = complete.

Yes, you=E2=80=99re = correct. Removed this.

---
 src/box/applier.cc | 7 +++++++
 1 file = changed, 7 insertions(+)

diff --git = a/src/box/applier.cc = b/src/box/applier.cc
index 7da278e68..ba68c3d07 100644
--- = a/src/box/applier.cc
+++ b/src/box/applier.cc
@@ -309,11 +309,15 @@ = applier_join(struct applier *applier)
   * = Receive initial data.
   */
  = assert(applier->join_stream !=3D NULL);
+ uint64_t = row_count =3D 0;
  while (true) {
 = coio_read_xrow(coio, ibuf, &row);
 = applier->last_row_time =3D = ev_monotonic_now(loop());
  = if (iproto_type_is_dml(row.type)) {
  = xstream_write_xc(applier->join_stream, &row);
+ = ++row_count;
+ if (row_count % = 100000 =3D=3D 0)
+ = say_info("%.1fM rows recieved", row_count / 1e6);
 = } else if (row.type =3D=3D IPROTO_OK) {
 = if (applier->version_id < = version_id(1, 7, 0)) {
  /*
@@ -354,6 +358,9 @@ applier_join(struct applier *applier)
 = if (iproto_type_is_dml(row.type)) {
 = = vclock_follow_xrow(&replicaset.vclock, &row);
 = = xstream_write_xc(applier->subscribe_stream, &row);
+ = ++row_count;
+ if (row_count % = 100000 =3D=3D 0)
+ = say_info("%.1fM rows recieved", row_count / 1e6);
 = } else if (row.type =3D=3D IPROTO_OK) {
 = /*
  =  * Current vclock. This is not used now,
-- 
2.17.1 (Apple Git-112)

= --Apple-Mail=_92FC4F6C-1979-4B41-A6D9-EB037C841808--