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 CBDDC6EC41; Mon, 5 Jul 2021 23:54:01 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org CBDDC6EC41 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1625518441; bh=VWFvP2Ny6zWnZ2tta+9IKKpvy8C9loMW862jJHCSg10=; h=To:References:Date:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=c3bW0MLcDjQ7moUpDlhtJsuXC2TJPtPz7tezZdUFYmrJ22zLRg09faEQTt0O/0Zfk CCPC55062KPLLIDe6EmntEcwIPrF27/4YpeLiJaq5wNk371B6Yb3gZUpHBmp98jjS6 JDZ0ZcslHcPifpaOcRl8xpsEtv5zyHkjQ53DubJo= Received: from smtpng3.i.mail.ru (smtpng3.i.mail.ru [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 DFA306EC40 for ; Mon, 5 Jul 2021 23:53:59 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org DFA306EC40 Received: by smtpng3.m.smailru.net with esmtpa (envelope-from ) id 1m0VbK-0003hz-R8; Mon, 05 Jul 2021 23:53:59 +0300 To: Oleg Babin , tarantool-patches@dev.tarantool.org, yaroslav.dynnikov@tarantool.org References: <6d8c2a728366edf5b0d208aeed9e027f870aa699.1625177222.git.v.shpilevoy@tarantool.org> <900c73e7-9824-ad3c-88f8-aa07b0382986@tarantool.org> Message-ID: <07f4dc34-0453-ce22-747e-6c50a2bcac12@tarantool.org> Date: Mon, 5 Jul 2021 22:53:57 +0200 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD954DFF1DC42D673FB96E19CC2B9345E2B1F8975EC27617E56182A05F53808504030CEA01894D8302F79E2E845F3002A28167F42E19B5EF89FF79E071A378CBB10 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE771540F9ECFC94C4BEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006373745FD4183B699148638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8E51E20BFF97F030B515EF9AEF658D80C117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCECADA55FE5B58BB7A471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352026055571C92BF10FF6B57BC7E6449061A352F6E88A58FB86F5D81C698A659EA7E827F84554CEF5019E625A9149C048EE9ECD01F8117BC8BEE2021AF6380DFAD18AA50765F790063735872C767BF85DA227C277FBC8AE2E8BAEB924C2B054B06E75ECD9A6C639B01B4E70A05D1297E1BBCB5012B2E24CD356 X-B7AD71C0: AC4F5C86D027EB782CDD5689AFBDA7A2AD77751E876CB595E8F7B195E1C97831E06C73D23BFD9DBFD3DF300CDFC134A3 X-C1DE0DAB: 0D63561A33F958A561690E92194464E6CC77EFE8927E1E54338E16C4693A5AD0D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA75342909995EBBA6E4410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D349A949488F6BF46DD977F922B2E9A60D6B718BB4509A12CEE07A58919C1D6A53F246ED7E3C3FE361F1D7E09C32AA3244CC7C99DE6456CAA98B2CAACF71E78F24B3C6EB905E3A8056BFACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojotfFaHYUgbC5+UHVcMsJvQ== X-Mailru-Sender: 689FA8AB762F7393C37E3C1AEC41BA5D2E721C1DF382331C32C4B2FB9F16FF843841015FED1DE5223CC9A89AB576DD93FB559BB5D741EB963CF37A108A312F5C27E8A8C3839CE0E267EA787935ED9F1B X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH vshard 5/6] router: introduce automatic master discovery 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" >>>>    diff --git a/vshard/replicaset.lua b/vshard/replicaset.lua >>>> index fa048c9..7747258 100644 >>>> --- a/vshard/replicaset.lua >>>> +++ b/vshard/replicaset.lua >>>> @@ -345,18 +377,30 @@ local function replicaset_master_call(replicaset, func, args, opts) >>>>        assert(opts == nil or type(opts) == 'table') >>>>        assert(type(func) == 'string', 'function name') >>>>        assert(args == nil or type(args) == 'table', 'function arguments') >>>> -    local conn, err = replicaset_connect_master(replicaset) >>>> -    if not conn then >>>> -        return nil, err >>>> -    end >>>> -    if not opts then >>>> -        opts = {timeout = replicaset.master.net_timeout} >>>> -    elseif not opts.timeout then >>>> -        opts = table.copy(opts) >>>> -        opts.timeout = replicaset.master.net_timeout >>>> +    local master = replicaset.master >>>> +    if not master then >>>> +        opts = opts and table.copy(opts) or {} >>>> +        if opts.is_async then >>>> +            return nil, lerror.vshard(lerror.code.MISSING_MASTER, >>>> +                                      replicaset.uuid) >>>> +        end >>> Could / should we here wakeup master discovery if "auto" is specified? >> We can. But I am afraid that under considerable RW requests load we >> might wakeup it too often. And if there are many routers, they might >> create a storm of `vshard.storage._call('info')` requests putting a >> notable load on the storages. So I decided it is not safe until the >> subscriptions are implemented. > > But we can't simply fail async requests when master is unknown. There is not much of a choice for now. You will fail them even if the master is here, but disconnected. Because netbox fails is_async immediately if the connection is not in active nor in fetch_schema states. If it must be solved, I would better do it in a generic way for both situations, separately. But it is not trivial. Because you start getting a concept of is_async + timeout so as the requests wouldn't hang there for too long time overflowing the queue of them. This means you would need a heap of requests sorted by their deadlines, and you would flush them in on_connect trigger, which AFAIR not suitable for making calls somewhy which is a next issue (but I might be wrong). I wouldn't want to do that all in scope of master discovery ticket. > And we even don't have exposed replicaset_wait_connected/replicaset_wait_master > > to call it before master will be known. As result several seconds we will get failed requests. I didn't expose them yet because I am very reluctant towards exposing anything until there is an explicit request from a user. Don't want to overload the API. > Also I saw TODO in test that "RO requests should be able to go to replicas" and I expect questions > > about RTO 0 on read requests from our customers. What do you mean as "RTO 0 on read requests"? > I don't know will this feature be used by cartridge but it could create small inconvenience for our customers > > in test because they should inject timeouts until master will be discovered (it's too artifical example but there were > > some similar questions when discovery started to work by chunks). > >>>> +local function locate_masters(replicasets) >>>> +    local is_all_done = true >>>> +    local is_all_nop = true >>>> +    local last_err >>>> +    for _, replicaset in pairs(replicasets) do >>>> +        local is_done, is_nop, err = replicaset_locate_master(replicaset) >>> I think we should log result of master discovery. Especially if error occurred. >> It is logged by the caller. See master_search_f(), init.lua:1067. > > Hmm... Yes, but I think such approach with last_err could lose some errors. Yes, that is also done on purpose. I don't want to clog the logs with too many errors. For instance, imagine you have 100 replicasets with 3 nodes in each, each is polled with 0.5s. And there is no network. Then it can produce about 600 errors per second. I tried to make the clogging less severe by showing only the last error. And the idea is that you fix these errors one by one until the log has no errors and discovery works fine. For example, firstly you fix the network if none of the futures was created. Then you fix storage.call('info') being not existing due to the old version, and etc. In the log you will see the next error to fix. I also wanted not to log the same error too often. For example, check that the message didn't change in the master search fiber, and log it only on change or once per, say, 10 seconds. But decided to wait if it explodes anywhere before I complicate the code even more. Do you think it is not needed? I am not sure, and can switch to logging of everything. >>>> diff --git a/vshard/router/init.lua b/vshard/router/init.lua >>>> index 5e2a96b..9407ccd 100644 >>>> --- a/vshard/router/init.lua >>>> +++ b/vshard/router/init.lua >>>> @@ -1030,6 +1032,93 @@ local function failover_f(router) >> <...> >> >>>> +-- >>>> +-- Master discovery background function. It is supposed to notice master changes >>>> +-- and find new masters in the replicasets, which are configured for that. >>>> +-- >>>> +-- XXX: due to polling the search might notice master change not right when it >>> Is this TODO related tohttps://github.com/tarantool/vshard/issues/284  ? >> Yes, that is one of the "solutions", although I would rather call it a crutch. >> I prefer to fix it by doing fair subscriptions. And for that I at least need a >> way to get netbox call result asynchronously, without having to call wait_result() >> anywhere. > > Does it request some changes in netbox/iproto? I mean https://github.com/tarantool/tarantool/issues/6107 6107 is not about the protocol changes. It is only about the interface on the server and in netbox. For vshard it would be enough to patch netbox. I want to be able to do something like that: local subscribe_complete local function subscribe_push(conn, msg) log.info('Push: %s', msg) -- ... end local function subscribe_start(conn) conn:call('subscribe', { is_async = true, on_complete = subscribe_complete, on_complete_ctx = conn, on_push = subscribe_push, on_push_ctx = conn, }) end local function subscribe_complete(conn, res) log.info('Subscription is done: %s, retry', res) -- ... subscribe_start(conn) end subscribe_start(conn) 'Subscribe' in vshard would be a function on the storage which returns when the storage's state changes anyhow. I could manage these subscriptions on the client in a very cheap way without fibers. Then I could make multiple subscriptions or one big sub for all storage's details. Wouldn't matter. And wouldn't cost almost anything on the storage too if 6107 would be done.