From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id E6ABB6C7D7; Tue, 27 Apr 2021 00:20:33 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org E6ABB6C7D7 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1619472034; bh=ZHbtUdH+0/0nBJirD+lqa4as1h/7uRwyAFTLFy8ZQP8=; h=To:Cc:References:Date:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=TIODG5tyjyzyIaNugHLOY+4A5hHxCxceVMJ/EI3Egn0CVgHhD697uhiNzvi+bEFr5 Wm51hGZUizIbQQfygwGl2kMCoK02dqia2R/TLXSfRqbJS8PTyPGjLJxdJns7POkFbd b1palXQNuOi2cRACGUsyWyA3jsCY+OZvVKN6HGHc= Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 50E726FC8F for ; Tue, 27 Apr 2021 00:20:32 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 50E726FC8F Received: by smtpng3.m.smailru.net with esmtpa (envelope-from ) id 1lb8ed-0004hm-G5; Tue, 27 Apr 2021 00:20:31 +0300 To: Serge Petrenko , gorcunov@gmail.com Cc: tarantool-patches@dev.tarantool.org References: <20210426165954.46474-1-sergepetrenko@tarantool.org> Message-ID: Date: Mon, 26 Apr 2021 23:20:30 +0200 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.10.0 MIME-Version: 1.0 In-Reply-To: <20210426165954.46474-1-sergepetrenko@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD9ECFD8CE5F0594010172C8F5787C1B7A21B115C62DFC52C90182A05F538085040DB79FF4AD1E29B476B47E6D79E3AFA59AFD800CAF2D3EB788B6A2F48662F2ACC X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7599DC91D9EF62C21EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006378B6D775AC58C227F8638F802B75D45FF914D58D5BE9E6BC1A93B80C6DEB9DEE97C6FB206A91F05B2BD48109174E1715AE3F13393C2081628F04B652EEC242312D2E47CDBA5A96583C09775C1D3CA48CFA333A05395E4745B117882F4460429724CE54428C33FAD30A8DF7F3B2552694AC26CFBAC0749D213D2E47CDBA5A9658378DA827A17800CE754B4581DB7A805049FA2833FD35BB23DF004C906525384302BEBFE083D3B9BA71A620F70A64A45A98AA50765F79006372E808ACE2090B5E1725E5C173C3A84C3C5EA940A35A165FF2DBA43225CD8A89F83C798A30B85E16BA91E23F1B6B78B78B5C8C57E37DE458BEDA766A37F9254B7 X-C1DE0DAB: 0D63561A33F958A5397306F61C5786124ACD443479367773F96167679441AF31D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA7502E6951B79FF9A3F410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D341B5517184E88C1BD869666C59486467F86D8870FB9096603CDA233B93C5AEC1FF8DC149AF37EABC21D7E09C32AA3244C14865DFCE9D8AAFFA979F235E5A3116B05AB220A9D022EBCFACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojmUj/IHfqgKdn+p1FILPAlA== X-Mailru-Sender: 689FA8AB762F73936BC43F508A06382279C4740DEFBF75C210426DA6A1D862903841015FED1DE5223CC9A89AB576DD93FB559BB5D741EB963CF37A108A312F5C27E8A8C3839CE0E267EA787935ED9F1B X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH] recovery: make it yield when positioning in a WAL X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Vladislav Shpilevoy via Tarantool-patches Reply-To: Vladislav Shpilevoy Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi! Thanks for the patch! See 2 questions, 1 comment. On 26.04.2021 18:59, Serge Petrenko wrote: > We had various places in box.cc and relay.cc which counted processed > rows and yielded every now and then. These yields didn't cover cases, > when recovery has to position inside a long WAL file: > > For example, when tarantool exits without leaving an empty WAL file > which'll be used to recover instance vclock on restart. In this case > the instance freezes while processing the last availabe WAL in order > to recover the vclock. > > Another issue is with replication. If a replica connects and needs data > from the end of a really long WAL, recovery will read up to the needed > position without yields, making relay disconnect by timeout. > > In order to fix the issue, make recovery decide when a yield should > happen. Introduce a new callback: schedule_yield, which is called by > recovery once it processes (no matter how, either simply skips or calls > xstream_write) enough rows (WAL_ROWS_PER_YIELD). > > schedule_yield either yields right away, in case of relay, or saves the > yield for later, in case of local recovery, because it might be in the > middle of a transaction. 1. Did you consider an option to yield explicitly in recovery code when it skips rows? If they are being skipped, it does not matter what are their transaction borders. Then the whole patch would be to add the yield once per WAL_ROWS_PER_YIELD to recovery_scan(), correct? > The only place with explicit row counting and manual yielding is now in > relay_initial_join, since its row sources are engines rather than recovery > with its WAL files. > > Closes #5979 > --- > https://github.com/tarantool/tarantool/tree/sp/gh-5979-recovery-yield > https://github.com/tarantool/tarantool/issues/5979 > > diff --git a/src/box/box.cc b/src/box/box.cc > index 59925962d..69a8f87eb 100644 > --- a/src/box/box.cc > +++ b/src/box/box.cc > @@ -3101,6 +3087,19 @@ bootstrap(const struct tt_uuid *instance_uuid, > } > } > > +struct wal_stream wal_stream; 2. This must be static. > + > +/** > + * Plan a yield in recovery stream. Wal stream will execute it as soon as it's > + * ready. > + */ > +static void > +wal_stream_schedule_yield(void) > +{ > + wal_stream.has_yield = true; > + wal_stream_try_yield(&wal_stream); > +} > diff --git a/src/box/recovery.cc b/src/box/recovery.cc > index cd33e7635..5351d8524 100644 > --- a/src/box/recovery.cc > +++ b/src/box/recovery.cc > @@ -241,10 +248,16 @@ static void > recover_xlog(struct recovery *r, struct xstream *stream, > const struct vclock *stop_vclock) > { > + /* Imitate old behaviour. Rows are counted separately for each xlog. */ > + r->row_count = 0; 3. But why do you need to imitate it? Does it mean if the files are too small to yield even once in each, but in total their number is huge, there won't be yields? Also does it mean "1M rows processed" was not ever printed in that case? > struct xrow_header row; > - uint64_t row_count = 0; > while (xlog_cursor_next_xc(&r->cursor, &row, > r->wal_dir.force_recovery) == 0) { > + if (++r->row_count % WAL_ROWS_PER_YIELD == 0) { > + r->schedule_yield(); > + } > + if (r->row_count % 100000 == 0) > + say_info("%.1fM rows processed", r->row_count / 1000000.); > /* > * Read the next row from xlog file. > * > @@ -273,12 +286,7 @@ recover_xlog(struct recovery *r, struct xstream *stream, > * failed row anyway. > */ > vclock_follow_xrow(&r->vclock, &row); > - if (xstream_write(stream, &row) == 0) { > - ++row_count; > - if (row_count % 100000 == 0) > - say_info("%.1fM rows processed", > - row_count / 1000000.); > - } else { > + if (xstream_write(stream, &row) != 0) { > if (!r->wal_dir.force_recovery) > diag_raise(); >