From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp62.i.mail.ru (smtp62.i.mail.ru [217.69.128.42]) (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 449A34696C3 for ; Fri, 10 Apr 2020 15:25:07 +0300 (MSK) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.80.23.2.2\)) From: Serge Petrenko In-Reply-To: Date: Fri, 10 Apr 2020 15:25:06 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <9DB87F34-3674-4316-BA19-8DA801F4BBE9@tarantool.org> References: Subject: Re: [Tarantool-patches] [PATCH v6 1/3] replication: omit 0-th vclock component in replication responses List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy Cc: tarantool-patches@dev.tarantool.org > 10 =D0=B0=D0=BF=D1=80. 2020 =D0=B3., =D0=B2 02:08, Vladislav Shpilevoy = =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0=D0=BB(=D0= =B0): >=20 > Hi! Thanks for the patch! Hi! Thanks for your review! >=20 >> diff --git a/src/box/xrow.c b/src/box/xrow.c >> index be026a43c..21a68220a 100644 >> --- a/src/box/xrow.c >> +++ b/src/box/xrow.c >> @@ -51,7 +51,7 @@ static_assert(IPROTO_DATA < 0x7f && IPROTO_METADATA = < 0x7f && >> static inline uint32_t >> mp_sizeof_vclock(const struct vclock *vclock) >> { >> - uint32_t size =3D vclock_size(vclock); >> + uint32_t size =3D vclock_size_ignore0(vclock); >=20 > This doesn't look right to silently ignore 0 component in a > function, which says nothing about that: mp_sizeof_vclock(). > Not in the function name, nor in a comment. Maybe worth renaming > it to mp_sizeof_vclock_ignore0(). Along with mp_encode_vclock(). >=20 > I am picky to the names, because we still actually have places, > where 0 component *is* needed (we have, right?). And we need to > say explicitly, which places ignore 0, which don't. >=20 > Also seems mp_decode_vclock() does not ignore 0. Is it possible > that we will get a connection from tarantool, which still does not > ignore 0 component, and will send it to us? For example, it was > upgraded recently on the latest branch, right before we pushed this > patchset. Ok, renamed mp_decode_vclock -> mp_decode_vclock_ignore0 mp_encode_vclock -> mp_encode_vclock_ignore0 I also replaced back the now unnecessary vclock_compare_ignore0 to vclock_compare. The diff=E2=80=99s below. diff --git a/src/box/relay.cc b/src/box/relay.cc index fec9f07d1..c634348a4 100644 --- a/src/box/relay.cc +++ b/src/box/relay.cc @@ -464,7 +464,7 @@ relay_schedule_pending_gc(struct relay *relay, const = struct vclock *vclock) * the greater signatures is due to changes pulled * from other members of the cluster. */ - if (vclock_compare_ignore0(&curr->vclock, vclock) > 0) + if (vclock_compare(&curr->vclock, vclock) > 0) break; stailq_shift(&relay->pending_gc); free(gc_msg); diff --git a/src/box/replication.cc b/src/box/replication.cc index ac71e0d89..7c10fb6f2 100644 --- a/src/box/replication.cc +++ b/src/box/replication.cc @@ -775,8 +775,8 @@ replicaset_needs_rejoin(struct replica **master) continue; =20 const struct ballot *ballot =3D &applier->ballot; - if (vclock_compare_ignore0(&ballot->gc_vclock, - &replicaset.vclock) <=3D 0) { + if (vclock_compare(&ballot->gc_vclock, + &replicaset.vclock) <=3D 0) { /* * There's at least one master that still stores * WALs needed by this instance. Proceed to = local diff --git a/src/box/xrow.c b/src/box/xrow.c index 21a68220a..51de49387 100644 --- a/src/box/xrow.c +++ b/src/box/xrow.c @@ -49,7 +49,7 @@ static_assert(IPROTO_DATA < 0x7f && IPROTO_METADATA < = 0x7f && "one byte"); =20 static inline uint32_t -mp_sizeof_vclock(const struct vclock *vclock) +mp_sizeof_vclock_ignore0(const struct vclock *vclock) { uint32_t size =3D vclock_size_ignore0(vclock); return mp_sizeof_map(size) + size * (mp_sizeof_uint(UINT32_MAX) = + @@ -57,7 +57,7 @@ mp_sizeof_vclock(const struct vclock *vclock) } =20 static inline char * -mp_encode_vclock(char *data, const struct vclock *vclock) +mp_encode_vclock_ignore0(char *data, const struct vclock *vclock) { data =3D mp_encode_map(data, vclock_size_ignore0(vclock)); struct vclock_iterator it; @@ -74,7 +74,7 @@ mp_encode_vclock(char *data, const struct vclock = *vclock) } =20 static int -mp_decode_vclock(const char **data, struct vclock *vclock) +mp_decode_vclock_ignore_0(const char **data, struct vclock *vclock) { vclock_create(vclock); if (mp_typeof(**data) !=3D MP_MAP) @@ -86,6 +86,14 @@ mp_decode_vclock(const char **data, struct vclock = *vclock) uint32_t id =3D mp_decode_uint(data); if (mp_typeof(**data) !=3D MP_UINT) return -1; + /* + * Skip vclock[0] coming from the remote + * instances. + */ + if (id =3D=3D 0) { + mp_next(data); + continue; + } int64_t lsn =3D mp_decode_uint(data); if (lsn > 0) vclock_follow(vclock, id, lsn); @@ -412,7 +420,7 @@ iproto_reply_vclock(struct obuf *out, const struct = vclock *vclock, uint64_t sync, uint32_t schema_version) { size_t max_size =3D IPROTO_HEADER_LEN + mp_sizeof_map(1) + - mp_sizeof_uint(UINT32_MAX) + mp_sizeof_vclock(vclock); + mp_sizeof_uint(UINT32_MAX) + = mp_sizeof_vclock_ignore0(vclock); =20 char *buf =3D obuf_reserve(out, max_size); if (buf =3D=3D NULL) { @@ -424,7 +432,7 @@ iproto_reply_vclock(struct obuf *out, const struct = vclock *vclock, char *data =3D buf + IPROTO_HEADER_LEN; data =3D mp_encode_map(data, 1); data =3D mp_encode_uint(data, IPROTO_VCLOCK); - data =3D mp_encode_vclock(data, vclock); + data =3D mp_encode_vclock_ignore0(data, vclock); size_t size =3D data - buf; assert(size <=3D max_size); =20 @@ -445,8 +453,10 @@ iproto_reply_vote(struct obuf *out, const struct = ballot *ballot, mp_sizeof_uint(UINT32_MAX) + mp_sizeof_map(4) + mp_sizeof_uint(UINT32_MAX) + = mp_sizeof_bool(ballot->is_ro) + mp_sizeof_uint(UINT32_MAX) + = mp_sizeof_bool(ballot->is_loading) + - mp_sizeof_uint(UINT32_MAX) + = mp_sizeof_vclock(&ballot->vclock) + - mp_sizeof_uint(UINT32_MAX) + = mp_sizeof_vclock(&ballot->gc_vclock); + mp_sizeof_uint(UINT32_MAX) + + mp_sizeof_vclock_ignore0(&ballot->vclock) + + mp_sizeof_uint(UINT32_MAX) + + mp_sizeof_vclock_ignore0(&ballot->gc_vclock); =20 char *buf =3D obuf_reserve(out, max_size); if (buf =3D=3D NULL) { @@ -464,9 +474,9 @@ iproto_reply_vote(struct obuf *out, const struct = ballot *ballot, data =3D mp_encode_uint(data, IPROTO_BALLOT_IS_LOADING); data =3D mp_encode_bool(data, ballot->is_loading); data =3D mp_encode_uint(data, IPROTO_BALLOT_VCLOCK); - data =3D mp_encode_vclock(data, &ballot->vclock); + data =3D mp_encode_vclock_ignore0(data, &ballot->vclock); data =3D mp_encode_uint(data, IPROTO_BALLOT_GC_VCLOCK); - data =3D mp_encode_vclock(data, &ballot->gc_vclock); + data =3D mp_encode_vclock_ignore0(data, &ballot->gc_vclock); size_t size =3D data - buf; assert(size <=3D max_size); =20 @@ -1244,11 +1254,13 @@ xrow_decode_ballot(struct xrow_header *row, = struct ballot *ballot) ballot->is_loading =3D mp_decode_bool(&data); break; case IPROTO_BALLOT_VCLOCK: - if (mp_decode_vclock(&data, &ballot->vclock) !=3D = 0) + if (mp_decode_vclock_ignore_0(&data, + &ballot->vclock) = !=3D 0) goto err; break; case IPROTO_BALLOT_GC_VCLOCK: - if (mp_decode_vclock(&data, &ballot->gc_vclock) = !=3D 0) + if (mp_decode_vclock_ignore_0(&data, + = &ballot->gc_vclock) !=3D 0) goto err; break; default: @@ -1270,7 +1282,8 @@ xrow_encode_register(struct xrow_header *row, size_t size =3D mp_sizeof_map(2) + mp_sizeof_uint(IPROTO_INSTANCE_UUID) + mp_sizeof_str(UUID_STR_LEN) + - mp_sizeof_uint(IPROTO_VCLOCK) + = mp_sizeof_vclock(vclock); + mp_sizeof_uint(IPROTO_VCLOCK) + + mp_sizeof_vclock_ignore0(vclock); char *buf =3D (char *) region_alloc(&fiber()->gc, size); if (buf =3D=3D NULL) { diag_set(OutOfMemory, size, "region_alloc", "buf"); @@ -1281,7 +1294,7 @@ xrow_encode_register(struct xrow_header *row, data =3D mp_encode_uint(data, IPROTO_INSTANCE_UUID); data =3D xrow_encode_uuid(data, instance_uuid); data =3D mp_encode_uint(data, IPROTO_VCLOCK); - data =3D mp_encode_vclock(data, vclock); + data =3D mp_encode_vclock_ignore0(data, vclock); assert(data <=3D buf + size); row->body[0].iov_base =3D buf; row->body[0].iov_len =3D (data - buf); @@ -1298,7 +1311,8 @@ xrow_encode_subscribe(struct xrow_header *row, uint32_t id_filter) { memset(row, 0, sizeof(*row)); - size_t size =3D XROW_BODY_LEN_MAX + mp_sizeof_vclock(vclock); + size_t size =3D XROW_BODY_LEN_MAX + + mp_sizeof_vclock_ignore0(vclock); char *buf =3D (char *) region_alloc(&fiber()->gc, size); if (buf =3D=3D NULL) { diag_set(OutOfMemory, size, "region_alloc", "buf"); @@ -1312,7 +1326,7 @@ xrow_encode_subscribe(struct xrow_header *row, data =3D mp_encode_uint(data, IPROTO_INSTANCE_UUID); data =3D xrow_encode_uuid(data, instance_uuid); data =3D mp_encode_uint(data, IPROTO_VCLOCK); - data =3D mp_encode_vclock(data, vclock); + data =3D mp_encode_vclock_ignore0(data, vclock); data =3D mp_encode_uint(data, IPROTO_SERVER_VERSION); data =3D mp_encode_uint(data, tarantool_version_id()); data =3D mp_encode_uint(data, IPROTO_REPLICA_ANON); @@ -1391,7 +1405,7 @@ xrow_decode_subscribe(struct xrow_header *row, = struct tt_uuid *replicaset_uuid, case IPROTO_VCLOCK: if (vclock =3D=3D NULL) goto skip; - if (mp_decode_vclock(&d, vclock) !=3D 0) { + if (mp_decode_vclock_ignore_0(&d, vclock) !=3D = 0) { xrow_on_decode_err(data, end, = ER_INVALID_MSGPACK, "invalid VCLOCK"); return -1; @@ -1473,7 +1487,7 @@ xrow_encode_vclock(struct xrow_header *row, const = struct vclock *vclock) memset(row, 0, sizeof(*row)); =20 /* Add vclock to response body */ - size_t size =3D 8 + mp_sizeof_vclock(vclock); + size_t size =3D 8 + mp_sizeof_vclock_ignore0(vclock); char *buf =3D (char *) region_alloc(&fiber()->gc, size); if (buf =3D=3D NULL) { diag_set(OutOfMemory, size, "region_alloc", "buf"); @@ -1482,7 +1496,7 @@ xrow_encode_vclock(struct xrow_header *row, const = struct vclock *vclock) char *data =3D buf; data =3D mp_encode_map(data, 1); data =3D mp_encode_uint(data, IPROTO_VCLOCK); - data =3D mp_encode_vclock(data, vclock); + data =3D mp_encode_vclock_ignore0(data, vclock); assert(data <=3D buf + size); row->body[0].iov_base =3D buf; row->body[0].iov_len =3D (data - buf); @@ -1498,7 +1512,8 @@ xrow_encode_subscribe_response(struct xrow_header = *row, { memset(row, 0, sizeof(*row)); size_t size =3D mp_sizeof_map(2) + - mp_sizeof_uint(IPROTO_VCLOCK) + = mp_sizeof_vclock(vclock) + + mp_sizeof_uint(IPROTO_VCLOCK) + + mp_sizeof_vclock_ignore0(vclock) + mp_sizeof_uint(IPROTO_CLUSTER_UUID) + mp_sizeof_str(UUID_STR_LEN); char *buf =3D (char *) region_alloc(&fiber()->gc, size); @@ -1509,7 +1524,7 @@ xrow_encode_subscribe_response(struct xrow_header = *row, char *data =3D buf; data =3D mp_encode_map(data, 2); data =3D mp_encode_uint(data, IPROTO_VCLOCK); - data =3D mp_encode_vclock(data, vclock); + data =3D mp_encode_vclock_ignore0(data, vclock); data =3D mp_encode_uint(data, IPROTO_CLUSTER_UUID); data =3D xrow_encode_uuid(data, replicaset_uuid); assert(data <=3D buf + size); >=20 >> return mp_sizeof_map(size) + size * (mp_sizeof_uint(UINT32_MAX) = + >> = mp_sizeof_uint(UINT64_MAX)); >> } -- Serge Petrenko sergepetrenko@tarantool.org