[Tarantool-patches] [PATCH v6 1/3] replication: omit 0-th vclock component in replication responses
Serge Petrenko
sergepetrenko at tarantool.org
Fri Apr 10 15:25:06 MSK 2020
> 10 апр. 2020 г., в 02:08, Vladislav Shpilevoy <v.shpilevoy at tarantool.org> написал(а):
>
> Hi! Thanks for the patch!
Hi! Thanks for your review!
>
>> 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 = vclock_size(vclock);
>> + uint32_t size = vclock_size_ignore0(vclock);
>
> 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().
>
> 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.
>
> 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’s 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;
const struct ballot *ballot = &applier->ballot;
- if (vclock_compare_ignore0(&ballot->gc_vclock,
- &replicaset.vclock) <= 0) {
+ if (vclock_compare(&ballot->gc_vclock,
+ &replicaset.vclock) <= 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");
static inline uint32_t
-mp_sizeof_vclock(const struct vclock *vclock)
+mp_sizeof_vclock_ignore0(const struct vclock *vclock)
{
uint32_t size = 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)
}
static inline char *
-mp_encode_vclock(char *data, const struct vclock *vclock)
+mp_encode_vclock_ignore0(char *data, const struct vclock *vclock)
{
data = 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)
}
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) != MP_MAP)
@@ -86,6 +86,14 @@ mp_decode_vclock(const char **data, struct vclock *vclock)
uint32_t id = mp_decode_uint(data);
if (mp_typeof(**data) != MP_UINT)
return -1;
+ /*
+ * Skip vclock[0] coming from the remote
+ * instances.
+ */
+ if (id == 0) {
+ mp_next(data);
+ continue;
+ }
int64_t lsn = 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 = 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);
char *buf = obuf_reserve(out, max_size);
if (buf == NULL) {
@@ -424,7 +432,7 @@ iproto_reply_vclock(struct obuf *out, const struct vclock *vclock,
char *data = buf + IPROTO_HEADER_LEN;
data = mp_encode_map(data, 1);
data = mp_encode_uint(data, IPROTO_VCLOCK);
- data = mp_encode_vclock(data, vclock);
+ data = mp_encode_vclock_ignore0(data, vclock);
size_t size = data - buf;
assert(size <= max_size);
@@ -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);
char *buf = obuf_reserve(out, max_size);
if (buf == NULL) {
@@ -464,9 +474,9 @@ iproto_reply_vote(struct obuf *out, const struct ballot *ballot,
data = mp_encode_uint(data, IPROTO_BALLOT_IS_LOADING);
data = mp_encode_bool(data, ballot->is_loading);
data = mp_encode_uint(data, IPROTO_BALLOT_VCLOCK);
- data = mp_encode_vclock(data, &ballot->vclock);
+ data = mp_encode_vclock_ignore0(data, &ballot->vclock);
data = mp_encode_uint(data, IPROTO_BALLOT_GC_VCLOCK);
- data = mp_encode_vclock(data, &ballot->gc_vclock);
+ data = mp_encode_vclock_ignore0(data, &ballot->gc_vclock);
size_t size = data - buf;
assert(size <= max_size);
@@ -1244,11 +1254,13 @@ xrow_decode_ballot(struct xrow_header *row, struct ballot *ballot)
ballot->is_loading = mp_decode_bool(&data);
break;
case IPROTO_BALLOT_VCLOCK:
- if (mp_decode_vclock(&data, &ballot->vclock) != 0)
+ if (mp_decode_vclock_ignore_0(&data,
+ &ballot->vclock) != 0)
goto err;
break;
case IPROTO_BALLOT_GC_VCLOCK:
- if (mp_decode_vclock(&data, &ballot->gc_vclock) != 0)
+ if (mp_decode_vclock_ignore_0(&data,
+ &ballot->gc_vclock) != 0)
goto err;
break;
default:
@@ -1270,7 +1282,8 @@ xrow_encode_register(struct xrow_header *row,
size_t size = 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 = (char *) region_alloc(&fiber()->gc, size);
if (buf == NULL) {
diag_set(OutOfMemory, size, "region_alloc", "buf");
@@ -1281,7 +1294,7 @@ xrow_encode_register(struct xrow_header *row,
data = mp_encode_uint(data, IPROTO_INSTANCE_UUID);
data = xrow_encode_uuid(data, instance_uuid);
data = mp_encode_uint(data, IPROTO_VCLOCK);
- data = mp_encode_vclock(data, vclock);
+ data = mp_encode_vclock_ignore0(data, vclock);
assert(data <= buf + size);
row->body[0].iov_base = buf;
row->body[0].iov_len = (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 = XROW_BODY_LEN_MAX + mp_sizeof_vclock(vclock);
+ size_t size = XROW_BODY_LEN_MAX +
+ mp_sizeof_vclock_ignore0(vclock);
char *buf = (char *) region_alloc(&fiber()->gc, size);
if (buf == NULL) {
diag_set(OutOfMemory, size, "region_alloc", "buf");
@@ -1312,7 +1326,7 @@ xrow_encode_subscribe(struct xrow_header *row,
data = mp_encode_uint(data, IPROTO_INSTANCE_UUID);
data = xrow_encode_uuid(data, instance_uuid);
data = mp_encode_uint(data, IPROTO_VCLOCK);
- data = mp_encode_vclock(data, vclock);
+ data = mp_encode_vclock_ignore0(data, vclock);
data = mp_encode_uint(data, IPROTO_SERVER_VERSION);
data = mp_encode_uint(data, tarantool_version_id());
data = 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 == NULL)
goto skip;
- if (mp_decode_vclock(&d, vclock) != 0) {
+ if (mp_decode_vclock_ignore_0(&d, vclock) != 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));
/* Add vclock to response body */
- size_t size = 8 + mp_sizeof_vclock(vclock);
+ size_t size = 8 + mp_sizeof_vclock_ignore0(vclock);
char *buf = (char *) region_alloc(&fiber()->gc, size);
if (buf == 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 = buf;
data = mp_encode_map(data, 1);
data = mp_encode_uint(data, IPROTO_VCLOCK);
- data = mp_encode_vclock(data, vclock);
+ data = mp_encode_vclock_ignore0(data, vclock);
assert(data <= buf + size);
row->body[0].iov_base = buf;
row->body[0].iov_len = (data - buf);
@@ -1498,7 +1512,8 @@ xrow_encode_subscribe_response(struct xrow_header *row,
{
memset(row, 0, sizeof(*row));
size_t size = 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 = (char *) region_alloc(&fiber()->gc, size);
@@ -1509,7 +1524,7 @@ xrow_encode_subscribe_response(struct xrow_header *row,
char *data = buf;
data = mp_encode_map(data, 2);
data = mp_encode_uint(data, IPROTO_VCLOCK);
- data = mp_encode_vclock(data, vclock);
+ data = mp_encode_vclock_ignore0(data, vclock);
data = mp_encode_uint(data, IPROTO_CLUSTER_UUID);
data = xrow_encode_uuid(data, replicaset_uuid);
assert(data <= buf + size);
>
>> return mp_sizeof_map(size) + size * (mp_sizeof_uint(UINT32_MAX) +
>> mp_sizeof_uint(UINT64_MAX));
>> }
--
Serge Petrenko
sergepetrenko at tarantool.org
More information about the Tarantool-patches
mailing list