[tarantool-patches] Re: [PATCH] replication: do not fetch records twice in a full mesh

[Vladislav Shpilevoy]

Hello. See my 6 comments below.

On 27/04/2018 13:23, Konstantin Belyavskiy wrote:
> ---
> Ticket: https://github.com/tarantool/tarantool/issues/3294
> Branch: https://github.com/tarantool/tarantool/compare/gh-3294-do-not-fetch-records-twice
> Link: https://github.com/tarantool/tarantool/blob/gh-3294-do-not-fetch-records-twice/doc/rfc/3294-implement-selective-subscribe-to-avoid-fetch-record-twice.md
> 
>   ...ective-subscribe-to-avoid-fetch-record-twice.md | 44 ++++++++++++++++++++++
>   1 file changed, 44 insertions(+)
>   create mode 100644 doc/rfc/3294-implement-selective-subscribe-to-avoid-fetch-record-twice.md
> 
> diff --git a/doc/rfc/3294-implement-selective-subscribe-to-avoid-fetch-record-twice.md b/doc/rfc/3294-implement-selective-subscribe-to-avoid-fetch-record-twice.md
> new file mode 100644
> index 000000000..aa97fec77
> --- /dev/null
> +++ b/doc/rfc/3294-implement-selective-subscribe-to-avoid-fetch-record-twice.md
> @@ -0,0 +1,44 @@
> +# Replication: do not fetch records twice in a full mesh
> +
> +* **Status**: In progress
> +* **Start date**: 25-04-2018
> +* **Authors**: Konstantin Belyavskiy @kbelyavs k.belyavskiy at tarantool.org, Georgy Kirichenko @georgy georgy at tarantool.org, Konstantin Osipov @kostja kostja at tarantool.org
> +* **Issues**: \[#3294\](https://github.com/tarantool/tarantool/issues/3294)

1. Please hide the URL using correct markup.

> +
> +## Summary
> +
> +replication: do not fetch records twice in a full mesh

2. Please, write a more informative summary. It is not a commit header, and is not limited
with one line and 50 symbols. For example, see https://github.com/tarantool/tarantool/blob/gh-3328-new-iproto/doc/rfc/3328-wire_protocol.md.

> +
> +## Background and motivation
> +
> +Extend IPROTO_SUBSCRIBE command with a list of server ids for which SUBSCRIBE should fetch changes. In a full mesh configuration, only download records originating from the immediate peer. Do not download the records from other peers twice.

3. Same - it is not a commit message. You must not write using imperative mood. Users, which will read this, do
not know that it IPROTO_SUBSCRIBE command.

> +
> +For example, imagine a full mesh of 3 replicas. Currently each Tarantool instance will download from all peers all records in their WAL excepts records with instance id equal to the self instance id. Instead, it could send a subscribe request to its peers with server ids which are not present in other subscribe requests.
> +
> +## Implementation
> +
> +After issuing IPROTO_REQUEST_VOTE to all peers we know a map of server ids, their peers and their vclocks. Sort 

4. RFC either must not contain implementation, or it must be at the end of the document.

the map by server id. Iterate over each server in the list of peers, and assign its  id to this server's SUBSCRIBE request. Assign all the remaining ids in \_cluster table to the last peer (alternatively, if there are many ids in

5. A new user does not know what it _cluster table, and it is a space, not a table.



> +## Rationale and alternatives
> +
> +Here existing alternatives are described, and why they appeared to be worse than an implemented thing.

6. Where are alternatives? If no one, then delete this template, please.