Re: [Tarantool-patches] [PATCH v2] qsync: provide box.info interface for monitoring

[Cyrill Gorcunov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

On Wed, Apr 07, 2021 at 10:53:27PM +0200, Vladislav Shpilevoy wrote:
> Hi! Thanks for the patch!
> 
> See 7 comments below.
> 
> >     qsync: provide box.info interface for monitoring
> >     
> >     Since commit 14fa5fd82 we support symbolic evaluation of
> 
> 1. Please, after a commit hash provide the referenced commit
> title in parentheses and quotes.
> https://github.com/tarantool/tarantool/wiki/Code-review-procedure#commit-message

Maybe worth to add an example then? Something like

[cyrill@grain tarantool.git] git log --pretty=reference -1 14fa5fd82
14fa5fd82 (cfg: support symbolic evaluation of replication_synchro_quorum, 2020-12-03)

> >     Thus we introduce the "synchro" leaf in box.info interface.
> >     For now only a few entries are printed out
> >     
> >      | tarantool> box.info.synchro
> >      | ---
> >      | - queue:
> >      |     1:
> >      |       len: 0
> >      |   quorum: 1
> >      | ...
> 
> 2. AFAIK, it was decided not to return an array, and return only
> the currently used queue. But even if it would be an array, as I
> said in the previous review, the indexing must be by box.info.id,
> not by any order. Assume there are multiple limbos, and the master is
> not instance_id = 1. How is a user supposed to find what is the
> queue in the currently used limbo? And what is the currently used
> limbo?

Wait, Vlad, this is v2, while I sent v3 already where tried to address
the formatting

https://lists.tarantool.org/tarantool-patches/YGt0TACA8xe6N7Rr@grain/

In short I use

Thus we introduce the "synchro" leaf in box.info interface.
For now only a few entries are printed out

 | tarantool> box.info.synchro
 | ---
 | - queue:
 |     len: 0
 |   quorum: 1
 | ...

The `queue` represents limbo instance and `len` member
shows the number of entries in the queue. Note that if
we gonna support multiple queues then we might create
a separate `queues` array to cover all possible instances.

The `quorum` member shows the evaluated value of
`replication_synchro_quorum` parameter.

Your comments are still relevant and I'll address them
in v4 but please make sure the output above is exactly
what you prefer?

> 
> Besides, the commit message and the docbot request are almost the
> same. You could drop the message and leave only the docbot request
> if you don't want to support both places. Also it would save time
> on reading that, anyway the information is the same.

I'll address all the rest of comments. Thanks for review!

	Cyrill