<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><br class="">
<div><br class=""><blockquote type="cite" class=""><div class="">27 февр. 2020 г., в 02:54, Vladislav Shpilevoy <<a href="mailto:v.shpilevoy@tarantool.org" class="">v.shpilevoy@tarantool.org</a>> написал(а):</div><br class="Apple-interchange-newline"><div class=""><div class="">Thanks for the patch!<br class=""><br class="">See 8 comments below.<br class=""></div></div></blockquote><div><br class=""></div><div>Hi! Thanks for the review!</div><div>Please find my comments inline and the diff below.</div><br class=""><blockquote type="cite" class=""><div class=""><div class=""><br class=""><blockquote type="cite" class=""> replication: implement an instance id filter for relay<br class=""><br class=""> Add a filter for relay to skip rows coming from unwanted instances.<br class=""> A list of instance ids whose rows replica doesn't want to fetch is encoded<br class=""> together with SUBSCRIBE request after a freshly introduced flag IPROTO_ID_FILTER.<br class=""><br class=""> Filtering rows is needed to prevent an instance from fetching its own<br class=""> rows from a remote master, which is useful on initial configuration and<br class=""> harmful on resubscribe.<br class=""><br class=""> Prerequisite #4739, #3294<br class=""><br class=""> @TarantoolBot document<br class=""><br class=""> Title: document new binary protocol key and subscribe request changes<br class=""><br class=""> Add key `IPROTO_ID_FILTER = 0x51` to the internals reference.<br class=""> This is an optional key used in SUBSCRIBE request followed by an array<br class=""> of ids of instances whose rows won't be relayed to the replica.<br class=""><br class=""> SUBSCRIBE request is supplemented with an optional field of the<br class=""> following structure:<br class=""> ```<br class=""> +====================+<br class=""> | ID_FILTER |<br class=""> | 0x51 : ID LIST |<br class=""> | MP_INT : MP_ARRRAY |<br class=""> | |<br class=""> +====================+<br class=""> ```<br class=""> The field is encoded only when the id list is not empty.<br class=""><br class="">diff --git a/src/box/<a href="http://relay.cc" class="">relay.cc</a> b/src/box/<a href="http://relay.cc" class="">relay.cc</a><br class="">index b89632273..dc89b90e2 100644<br class="">--- a/src/box/<a href="http://relay.cc" class="">relay.cc</a><br class="">+++ b/src/box/<a href="http://relay.cc" class="">relay.cc</a><br class="">@@ -109,6 +109,7 @@ struct relay {<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>struct vclock recv_vclock;<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>/** Replicatoin slave version. */<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>uint32_t version_id;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>unsigned int id_filter;<br class=""></blockquote><br class="">1. Please, add a comment here explaining what is it,<br class="">and why is needed.<br class=""></div></div></blockquote><div><br class=""></div><div>Ok.</div><br class=""><blockquote type="cite" class=""><div class=""><div class=""><br class=""><blockquote type="cite" class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>/**<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span> * Local vclock at the moment of subscribe, used to check<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span> * dataset on the other side and send missing data rows if any.<br class="">@@ -763,6 +767,9 @@ relay_send_row(struct xstream *stream, struct xrow_header *packet)<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>packet->group_id = GROUP_DEFAULT;<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>packet->bodycnt = 0;<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>}<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>/* Check if the rows from the instance are filtered. */<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>if (1 << packet->replica_id & relay->id_filter)<br class=""></blockquote><br class="">2. Please use explicit != 0 here and below when check 'filter_size'<br class="">variable.<br class=""></div></div></blockquote><div><br class=""></div>No problem.</div><div><br class=""><blockquote type="cite" class=""><div class=""><div class=""><br class=""><blockquote type="cite" class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>return;<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>/*<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span> * We're feeding a WAL, thus responding to FINAL JOIN or SUBSCRIBE<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span> * request. If this is FINAL JOIN (i.e. relay->replica is NULL),<br class="">diff --git a/src/box/xrow.c b/src/box/xrow.c<br class="">index 968c3a202..10edbf6a8 100644<br class="">--- a/src/box/xrow.c<br class="">+++ b/src/box/xrow.c<br class="">@@ -1194,17 +1194,23 @@ int<br class=""> xrow_encode_subscribe(struct xrow_header *row,<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span> const struct tt_uuid *replicaset_uuid,<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span> const struct tt_uuid *instance_uuid,<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span> const struct vclock *vclock, bool anon)<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span> const struct vclock *vclock, bool anon,<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span> unsigned int id_filter)<br class=""></blockquote><br class="">3. Nit - it would be better to have it as uint32_t explicitly.<br class="">Becuase max id count is 32. Unsigned int does not have size<br class="">guarantees, formally speaking. It is at least 16 bits, no more<br class="">info.<br class=""></div></div></blockquote><div><br class=""></div><div>Done, and I’ll send a followup to vclock.h later, as Kostja suggests.</div><br class=""><blockquote type="cite" class=""><div class=""><div class=""><br class=""><blockquote type="cite" class=""> {<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>memset(row, 0, sizeof(*row));<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>size_t size = XROW_BODY_LEN_MAX + mp_sizeof_vclock(vclock);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>unsigned int filter_size = __builtin_popcount(id_filter);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>if (filter_size) {<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>size += mp_sizeof_array(filter_size) + filter_size *<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>mp_sizeof_uint(VCLOCK_MAX);<br class=""></blockquote><br class="">4. You didn't add mp_sizeof_uint(IPROTO_ID_FILTER) here, but you<br class="">didn't update XROW_BODY_LEN_MAX either. Does it fit in the current<br class="">value? Also seems like it can't be called XROW_BODY_LEN_MAX anymore.<br class="">Because clearly it is not enough to fit the filter array, if you<br class="">needed to patch the allocation here.<br class=""></div></div></blockquote><div><br class=""></div><div>Let’s then just double XROW_BODY_LEN_MAX up.</div><br class=""><blockquote type="cite" class=""><div class=""><div class=""><br class="">You could also update XROW_BODY_LEN_MAX so as it includes the biggest<br class="">possible filter. Max filter size is just 34 bytes. This is the<br class="">simplest option, I think.<br class=""><br class=""><blockquote type="cite" class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>}<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>char *buf = (char *) region_alloc(&fiber()->gc, size);<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>if (buf == NULL) {<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>diag_set(OutOfMemory, size, "region_alloc", "buf");<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>return -1;<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>}<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>char *data = buf;<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span>data = mp_encode_map(data, 5);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>data = mp_encode_map(data, filter_size ? 6 : 5);<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>data = mp_encode_uint(data, IPROTO_CLUSTER_UUID);<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>data = xrow_encode_uuid(data, replicaset_uuid);<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>data = mp_encode_uint(data, IPROTO_INSTANCE_UUID);<br class="">@@ -1215,6 +1221,17 @@ xrow_encode_subscribe(struct xrow_header *row,<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>data = mp_encode_uint(data, tarantool_version_id());<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>data = mp_encode_uint(data, IPROTO_REPLICA_ANON);<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>data = mp_encode_bool(data, anon);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>if (filter_size) {<br class=""></blockquote><br class="">5. I wouldn't make it optional in encoding, since this is sent<br class="">really rare, but could simplify the code a bit. However, up to<br class="">you.<br class=""></div></div></blockquote><div><br class=""></div><div>I don’t like the idea to pass zero length arrays in case there is no filter,</div><div>Lets just not encode this part. The filter is initially empty on decoding side</div><div>anyway.</div><br class=""><blockquote type="cite" class=""><div class=""><div class=""><br class="">Also, won't it break replication from an old version? You<br class="">will send subscribe with this new key, and the old instance<br class="">should ignore it. Does it? I don't remember.<br class=""><br class=""></div></div></blockquote><div><br class=""></div><div>Yes, the old instance ignores all unknown keys.</div><br class=""><blockquote type="cite" class=""><div class=""><div class="">If the old instance would ignore it, it means that the bug<br class="">still can explode when replicating from an old version, right?<br class="">I don't know how to fix that, but if it is true, we need to<br class="">document that, at least.<br class=""></div></div></blockquote><div><br class=""></div><div>Yes, the bug’s still here when replicating from an old instance.</div><div>To trigger it, you have to set up master-master replication between</div><div>the old and the new instances and fit some additional conditions.</div><div>I don’t think it’s usual to set up master-master when upgrading, as</div><div>Kostja has pointed out.</div><br class=""><blockquote type="cite" class=""><div class=""><div class=""><br class=""><blockquote type="cite" class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>data = mp_encode_uint(data, IPROTO_ID_FILTER);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>data = mp_encode_array(data, filter_size);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>struct bit_iterator it;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>bit_iterator_init(&it, &id_filter, sizeof(id_filter),<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span> true);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>for (size_t id = bit_iterator_next(&it); id < VCLOCK_MAX;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span> id = bit_iterator_next(&it)) {<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>data = mp_encode_uint(data, id);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>}<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>}<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>assert(data <= buf + size);<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>row->body[0].iov_base = buf;<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>row->body[0].iov_len = (data - buf);<br class="">@@ -1301,6 +1321,21 @@ xrow_decode_subscribe(struct xrow_header *row, struct tt_uuid *replicaset_uuid,<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>}<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>*anon = mp_decode_bool(&d);<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>break;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>case IPROTO_ID_FILTER:<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>if (id_filter == NULL)<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>goto skip;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>if (mp_typeof(*d) != MP_ARRAY) {<br class="">+decode_err:<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>xrow_on_decode_err(data, end, ER_INVALID_MSGPACK,<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span> "invalid id_filter");<br class=""></blockquote><br class="">6. Lets say it in caps: ID_FILTER, just like with other decode<br class="">error messages. Also I would name this label 'id_filter_decode_err',<br class="">becuase other keys can't jump here.<br class=""></div></div></blockquote><div><br class=""></div>Okay.</div><div><br class=""><blockquote type="cite" class=""><div class=""><div class=""><br class=""><blockquote type="cite" class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>return -1;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>}<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>uint32_t len = mp_decode_array(&d);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>for(uint32_t i = 0; i < len; ++i) {<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>if (mp_typeof(*d) != MP_UINT)<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>goto decode_err;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>*id_filter |= 1 << mp_decode_uint(&d);<br class=""></blockquote><br class="">7. If someone would send a big ID (a program, pretending to be a Tarantool<br class="">instance), it would cause unsigned bit shift overflow, which is undefined<br class="">behaviour. Lets check that it is not bigger than 31.<br class=""><br class="">However this won't really help much. This code will crash even if I will<br class="">just send a truncated packet. From what I see.<br class=""></div></div></blockquote><div><br class=""></div><div>I’m not sure I understand what you’re speaking about. This piece of code is</div><div>similar to the one we have in mp_decode_vclock. The situation didn’t get worse,</div><div>at least.</div><div><br class=""></div><blockquote type="cite" class=""><div class=""><div class=""><br class="">Up to you whether you want to fix the bit shift.<br class=""></div></div></blockquote><div><br class=""></div>Fixed.</div><div><br class=""></div><div><blockquote type="cite" class=""><div class=""><div class=""><br class=""><blockquote type="cite" class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>}<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>break;<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>default: skip:<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>mp_next(&d); /* value */<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>}<br class="">diff --git a/src/box/xrow.h b/src/box/xrow.h<br class="">index 0973c497d..8e5716b30 100644<br class="">--- a/src/box/xrow.h<br class="">+++ b/src/box/xrow.h<br class="">@@ -322,6 +322,8 @@ xrow_encode_register(struct xrow_header *row,<br class=""> * @param instance_uuid Instance uuid.<br class=""> * @param vclock Replication clock.<br class=""> * @param anon Whether it is an anonymous subscribe request or not.<br class="">+ * @param id_filter A List of replica ids to skip rows from<br class="">+ * when feeding a replica.<br class=""></blockquote><br class="">8. Looks like a huge indentation. Keep if you want. I noticed<br class="">just in case it was an accident. In the next @param description<br class="">you aligned it differently.<br class=""></div></div></blockquote><div><br class=""></div><div>Thanks! I fixed both pieces.</div><br class=""><blockquote type="cite" class=""><div class=""><div class=""><br class=""><blockquote type="cite" class=""> *<br class=""> * @retval 0 Success.<br class=""> * @retval -1 Memory error.<br class="">@@ -330,7 +332,8 @@ int<br class="">@@ -340,6 +343,8 @@ xrow_encode_subscribe(struct xrow_header *row,<br class=""> * @param[out] vclock.<br class=""> * @param[out] version_id.<br class=""> * @param[out] anon Whether it is an anonymous subscribe.<br class="">+ * @param[out] id_filter A list of ids to skip rows from when<br class="">+ * feeding replica.<br class=""> *<br class=""> * @retval 0 Success.<br class=""> * @retval -1 Memory or format error.<br class=""></blockquote></div></div></blockquote><br class=""></div><div><br class=""></div><span class="">diff --git a/src/box/<a href="http://box.cc" class="">box.cc</a> b/src/box/<a href="http://box.cc" class="">box.cc</a><br class="">index 94267c74e..09dd67ab4 100644<br class="">--- a/src/box/<a href="http://box.cc" class="">box.cc</a><br class="">+++ b/src/box/<a href="http://box.cc" class="">box.cc</a><br class="">@@ -1787,7 +1787,7 @@ box_process_subscribe(struct ev_io *io, struct xrow_header *header)<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>uint32_t replica_version_id;<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>vclock_create(&replica_clock);<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>bool anon;<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span>unsigned int id_filter;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>uint32_t id_filter;<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>xrow_decode_subscribe_xc(header, NULL, &replica_uuid, &replica_clock,<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span> &replica_version_id, &anon, &id_filter);<br class=""> <br class="">diff --git a/src/box/<a href="http://relay.cc" class="">relay.cc</a> b/src/box/<a href="http://relay.cc" class="">relay.cc</a><br class="">index dc89b90e2..95245a3cf 100644<br class="">--- a/src/box/<a href="http://relay.cc" class="">relay.cc</a><br class="">+++ b/src/box/<a href="http://relay.cc" class="">relay.cc</a><br class="">@@ -109,7 +109,13 @@ struct relay {<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>struct vclock recv_vclock;<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>/** Replicatoin slave version. */<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>uint32_t version_id;<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span>unsigned int id_filter;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>/**<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span> * A filter of replica ids whose rows should be ignored.<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span> * Each set filter bit corresponds to a replica id whose<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span> * rows shouldn't be relayed. The list of ids to ignore<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span> * is passed by the replica on subscribe.<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span> */<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>uint32_t id_filter;<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>/**<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span> * Local vclock at the moment of subscribe, used to check<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span> * dataset on the other side and send missing data rows if any.<br class="">@@ -678,7 +684,7 @@ relay_subscribe_f(va_list ap)<br class=""> void<br class=""> relay_subscribe(struct replica *replica, int fd, uint64_t sync,<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>struct vclock *replica_clock, uint32_t replica_version_id,<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span>unsigned int replica_id_filter)<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>uint32_t replica_id_filter)<br class=""> {<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>assert(replica->anon || replica->id != REPLICA_ID_NIL);<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>struct relay *relay = replica->relay;<br class="">@@ -768,7 +774,7 @@ relay_send_row(struct xstream *stream, struct xrow_header *packet)<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>packet->bodycnt = 0;<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>}<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>/* Check if the rows from the instance are filtered. */<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span>if (1 << packet->replica_id & relay->id_filter)<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>if ((1 << packet->replica_id & relay->id_filter) != 0)<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>return;<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>/*<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span> * We're feeding a WAL, thus responding to FINAL JOIN or SUBSCRIBE<br class="">diff --git a/src/box/relay.h b/src/box/relay.h<br class="">index 6e7eebab1..0632fa912 100644<br class="">--- a/src/box/relay.h<br class="">+++ b/src/box/relay.h<br class="">@@ -125,6 +125,6 @@ relay_final_join(int fd, uint64_t sync, struct vclock *start_vclock,<br class=""> void<br class=""> relay_subscribe(struct replica *replica, int fd, uint64_t sync,<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>struct vclock *replica_vclock, uint32_t replica_version_id,<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span>unsigned int replica_id_filter);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>uint32_t replica_id_filter);<br class=""> <br class=""> #endif /* TARANTOOL_REPLICATION_RELAY_H_INCLUDED */<br class="">diff --git a/src/box/xrow.c b/src/box/xrow.c<br class="">index 10edbf6a8..602049004 100644<br class="">--- a/src/box/xrow.c<br class="">+++ b/src/box/xrow.c<br class="">@@ -1195,22 +1195,18 @@ xrow_encode_subscribe(struct xrow_header *row,<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span> const struct tt_uuid *replicaset_uuid,<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span> const struct tt_uuid *instance_uuid,<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span> const struct vclock *vclock, bool anon,<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span> unsigned int id_filter)<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span> uint32_t id_filter)<br class=""> {<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>memset(row, 0, sizeof(*row));<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>size_t size = XROW_BODY_LEN_MAX + mp_sizeof_vclock(vclock);<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span>unsigned int filter_size = __builtin_popcount(id_filter);<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span>if (filter_size) {<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span>size += mp_sizeof_array(filter_size) + filter_size *<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span>mp_sizeof_uint(VCLOCK_MAX);<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span>}<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>char *buf = (char *) region_alloc(&fiber()->gc, size);<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>if (buf == NULL) {<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>diag_set(OutOfMemory, size, "region_alloc", "buf");<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>return -1;<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>}<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>char *data = buf;<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span>data = mp_encode_map(data, filter_size ? 6 : 5);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>int filter_size = __builtin_popcount(id_filter);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>data = mp_encode_map(data, filter_size != 0 ? 6 : 5);<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>data = mp_encode_uint(data, IPROTO_CLUSTER_UUID);<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>data = xrow_encode_uuid(data, replicaset_uuid);<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>data = mp_encode_uint(data, IPROTO_INSTANCE_UUID);<br class="">@@ -1221,7 +1217,7 @@ xrow_encode_subscribe(struct xrow_header *row,<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>data = mp_encode_uint(data, tarantool_version_id());<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>data = mp_encode_uint(data, IPROTO_REPLICA_ANON);<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>data = mp_encode_bool(data, anon);<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span>if (filter_size) {<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>if (filter_size != 0) {<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>data = mp_encode_uint(data, IPROTO_ID_FILTER);<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>data = mp_encode_array(data, filter_size);<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>struct bit_iterator it;<br class="">@@ -1244,7 +1240,7 @@ int<br class=""> xrow_decode_subscribe(struct xrow_header *row, struct tt_uuid *replicaset_uuid,<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span> struct tt_uuid *instance_uuid, struct vclock *vclock,<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span> uint32_t *version_id, bool *anon,<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span> unsigned int *id_filter)<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span> uint32_t *id_filter)<br class=""> {<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>if (row->bodycnt == 0) {<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>diag_set(ClientError, ER_INVALID_MSGPACK, "request body");<br class="">@@ -1325,15 +1321,18 @@ xrow_decode_subscribe(struct xrow_header *row, struct tt_uuid *replicaset_uuid,<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>if (id_filter == NULL)<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>goto skip;<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>if (mp_typeof(*d) != MP_ARRAY) {<br class="">-decode_err:<span class="Apple-tab-span" style="white-space:pre"> </span>xrow_on_decode_err(data, end, ER_INVALID_MSGPACK,<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span> "invalid id_filter");<br class="">+id_filter_decode_err:<span class="Apple-tab-span" style="white-space:pre"> </span>xrow_on_decode_err(data, end, ER_INVALID_MSGPACK,<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span> "invalid ID_FILTER");<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>return -1;<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>}<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>uint32_t len = mp_decode_array(&d);<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span>for(uint32_t i = 0; i < len; ++i) {<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>for (uint32_t i = 0; i < len; ++i) {<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>if (mp_typeof(*d) != MP_UINT)<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span>goto decode_err;<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span>*id_filter |= 1 << mp_decode_uint(&d);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>goto id_filter_decode_err;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>uint64_t val = mp_decode_uint(&d);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>if (val >= VCLOCK_MAX)<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>goto id_filter_decode_err;<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>*id_filter |= 1 << val;<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>}<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>break;<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>default: skip:<br class="">diff --git a/src/box/xrow.h b/src/box/xrow.h<br class="">index 8e5716b30..2a0a9c852 100644<br class="">--- a/src/box/xrow.h<br class="">+++ b/src/box/xrow.h<br class="">@@ -48,7 +48,7 @@ enum {<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>XROW_BODY_IOVMAX = 2,<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>XROW_IOVMAX = XROW_HEADER_IOVMAX + XROW_BODY_IOVMAX,<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>XROW_HEADER_LEN_MAX = 52,<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span>XROW_BODY_LEN_MAX = 128,<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>XROW_BODY_LEN_MAX = 256,<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>IPROTO_HEADER_LEN = 28,<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>/** 7 = sizeof(iproto_body_bin). */<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>IPROTO_SELECT_HEADER_LEN = IPROTO_HEADER_LEN + 7,<br class="">@@ -323,7 +323,7 @@ xrow_encode_register(struct xrow_header *row,<br class=""> * @param vclock Replication clock.<br class=""> * @param anon Whether it is an anonymous subscribe request or not.<br class=""> * @param id_filter A List of replica ids to skip rows from<br class="">- * when feeding a replica.<br class="">+ *<span class="Apple-tab-span" style="white-space:pre"> </span> when feeding a replica.<br class=""> *<br class=""> * @retval 0 Success.<br class=""> * @retval -1 Memory error.<br class="">@@ -333,7 +333,7 @@ xrow_encode_subscribe(struct xrow_header *row,<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span> const struct tt_uuid *replicaset_uuid,<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span> const struct tt_uuid *instance_uuid,<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span> const struct vclock *vclock, bool anon,<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span> unsigned int id_filter);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span> uint32_t id_filter);<br class=""> <br class=""> /**<br class=""> * Decode SUBSCRIBE command.<br class="">@@ -344,7 +344,7 @@ xrow_encode_subscribe(struct xrow_header *row,<br class=""> * @param[out] version_id.<br class=""> * @param[out] anon Whether it is an anonymous subscribe.<br class=""> * @param[out] id_filter A list of ids to skip rows from when<br class="">- * feeding replica.<br class="">+ *<span class="Apple-tab-span" style="white-space:pre"> </span> feeding a replica.<br class=""> *<br class=""> * @retval 0 Success.<br class=""> * @retval -1 Memory or format error.<br class="">@@ -353,7 +353,7 @@ int<br class=""> xrow_decode_subscribe(struct xrow_header *row, struct tt_uuid *replicaset_uuid,<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span> struct tt_uuid *instance_uuid, struct vclock *vclock,<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span> uint32_t *version_id, bool *anon,<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span> unsigned int *id_filter);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span> uint32_t *id_filter);<br class=""> <br class=""> /**<br class=""> * Encode JOIN command.<br class="">@@ -827,7 +827,7 @@ xrow_encode_subscribe_xc(struct xrow_header *row,<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span> const struct tt_uuid *replicaset_uuid,<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span> const struct tt_uuid *instance_uuid,<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span> const struct vclock *vclock, bool anon,<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span> unsigned int id_filter)<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span> uint32_t id_filter)<br class=""> {<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>if (xrow_encode_subscribe(row, replicaset_uuid, instance_uuid,<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span> vclock, anon, id_filter) != 0)<br class="">@@ -840,7 +840,7 @@ xrow_decode_subscribe_xc(struct xrow_header *row,<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span> struct tt_uuid *replicaset_uuid,<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span> struct tt_uuid *instance_uuid, struct vclock *vclock,<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span> uint32_t *replica_version_id, bool *anon,<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span> unsigned int *id_filter)<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span> uint32_t *id_filter)<br class=""> {<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>if (xrow_decode_subscribe(row, replicaset_uuid, instance_uuid,<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span> vclock, replica_version_id, anon,<br class=""></span><span class=""><br class=""></span><div><div dir="auto" style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div>--</div><div>Serge Petrenko</div><div><a href="mailto:sergepetrenko@tarantool.org" class="">sergepetrenko@tarantool.org</a></div><div><br class=""></div></div></div><br class=""></body></html>