Re: [Tarantool-patches] [PATCH v8 0/2] relay: provide downstream lag information

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

On Mon, Jun 07, 2021 at 09:20:50PM +0200, Vladislav Shpilevoy wrote:
> Hi! Thanks for the patchset!
> 
> Please, provide branch and issue links next time.
> https://github.com/tarantool/tarantool/wiki/Code-review-procedure#sending-the-patch
> 
> I assume your branch is gorcunov/gh-5447-relay-lag-8.

It was there, letme quote it for you, I think you might simply
miss it from a glance

 > branch gorcunov/gh-5447-relay-lag-8
 > issue https://github.com/tarantool/tarantool/issues/5447


> It seems after your changes some tests start failing. Looks
> like you didn't clear artifacts of your new test.
> https://github.com/tarantool/tarantool/runs/2765786321

I'll take a look, thanks!

> On 07.06.2021 17:55, Cyrill Gorcunov wrote:
> > Guys, take a look once time permit. Previous version is here
> > 
> > https://lists.tarantool.org/tarantool-patches/20210506214132.533913-1-gorcunov@gmail.com/
> > 
> > I still think we might need some general statistics engine because this
> > lag rather common data and we could move other "last" things like last_row_time
> > there but since I didn't get approve on the idea I keep the timestamp
> > in replica structure.
> 
> If you are talking about your perf metrics engine which we tried to
> design for replication, it was approved. The problem is that there was
> no a really good design on how should it look and work.

Not perf metrics (as we were discussed time back then seems plain perf is
more than enough for most of developers if I'm not mistaken). I rather though
of my previous RFC where I proposed to have some common and shared "stat" engine
with rw operations:

 > Not for merging yet! I think instead of applier_wal_stat structure we might need some
 > _commonly shared_ statistics structure, probably bound to WAL code and all other
 > threads will update it in a lockless way, because we might need to collect more
 > detais on WAL processing in future. I though of something like
 > 
 > 	enum {
 > 		WAL_ICNT__APPLIER_TXN_START_TM,
 > 
 > 		WAL_ICNT__MAX,
 > 	};
 > 
 > 	struct wal_stat {
 > 		int64_t	icnt[WAL_ICNT__MAX];
 > 	} wal_st[VCLOCK_MAX];
 > 
 > and introduce
 > 
 > 	wal_st__icnt_read(unsigned id);
 > 	wal_st__icnt_write(unsigned id);
 > 
 > then applier will simply push last timestamp to WAL_ICNT__APPLIER_TXN_START_TM
 > counter, and later when we need to send ACK we use wal_st__icnt_read() to
 > fetch it back. We won't need to allocate some dynamic memory for it but
 > rather use statically preallocated shared between threads.