From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> To: Oleg Babin <olegrok@tarantool.org>, tarantool-patches@dev.tarantool.org Subject: Re: [Tarantool-patches] [PATCH vshard 6/7] router: make discovery smoother in a big cluster Date: Mon, 4 May 2020 23:09:53 +0200 [thread overview] Message-ID: <df14f457-a4fa-0dff-afe3-49dc9cbc8a23@tarantool.org> (raw) In-Reply-To: <2bb5c86a-04c4-96a9-ab3e-75b96d43b1d9@tarantool.org> Hi! Thanks for the review! >>>> diff --git a/vshard/router/init.lua b/vshard/router/init.lua >>>> index 6d88153..26ea85b 100644 >>>> --- a/vshard/router/init.lua >>>> +++ b/vshard/router/init.lua >>>> @@ -250,50 +250,127 @@ if util.version_is_at_least(1, 10, 0) then >>>> + -- Don't spam many requests at once. Give >>>> + -- storages time to handle them and other >>>> + -- requests. >>>> + lfiber.sleep(consts.DISCOVERY_WORK_STEP) >>>> + if module_version ~= M.module_version then >>>> + return >>>> end >>> >>> >>> Is it possible to place such checks to some "common" places. At start/end of each iteration or between steps. This looks strange and a bit unobvous to do such checks after each timeout. >> >> Purpose is to stop the fiber as soon as possible in case a reload >> happened. So moving it to a common place is not really possible. >> It would make it work longer with the old code after reload. But >> if we keep the checks, I still need to do then after/before any >> long yield. >> > > My main concern is that such things could be easily missed in future. But I agree that we should left this function early as possible. It can be, and it was a couple of times. Don't know what to do with that honestly, at this moment. This restart-on-reload machinery should be reworked completely. >> During working on your comments I found that during aggressive discovery I >> added sleep in a wrong place. Fixed below: >> >> diff --git a/vshard/consts.lua b/vshard/consts.lua >> index 5391c0f..a6a8c1b 100644 >> --- a/vshard/consts.lua >> +++ b/vshard/consts.lua >> @@ -40,5 +40,8 @@ return { >> DEFAULT_COLLECT_BUCKET_GARBAGE_INTERVAL = 0.5; >> RECOVERY_INTERVAL = 5; >> COLLECT_LUA_GARBAGE_INTERVAL = 100; >> - DISCOVERY_INTERVAL = 10; >> + DISCOVERY_IDLE_INTERVAL = 10, >> + DISCOVERY_WORK_INTERVAL = 1, >> + DISCOVERY_WORK_STEP = 0.01, >> + DISCOVERY_TIMEOUT = 10, >> } > > Nit: here commas and semicolons are mixed. I think we should support it in consistent state - only commas or only semicolons. I don't like these ';'. They are against our code style. They were added in the second commit in this repository, and since then kept growing. I deliberately stopped using them and decided to switch to ',' in all new code, and wash out the old lines gradually. I guess I can split DISCOVERY_* parameters with an empty line from the others who still use ';' though. To make it more clear. ==================== diff --git a/vshard/consts.lua b/vshard/consts.lua index a6a8c1b..8c2a8b0 100644 --- a/vshard/consts.lua +++ b/vshard/consts.lua @@ -40,6 +40,7 @@ return { DEFAULT_COLLECT_BUCKET_GARBAGE_INTERVAL = 0.5; RECOVERY_INTERVAL = 5; COLLECT_LUA_GARBAGE_INTERVAL = 100; + DISCOVERY_IDLE_INTERVAL = 10, DISCOVERY_WORK_INTERVAL = 1, DISCOVERY_WORK_STEP = 0.01, ====================
next prev parent reply other threads:[~2020-05-04 21:09 UTC|newest] Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-05-01 0:16 [Tarantool-patches] [PATCH vshard 0/7] Router extended discovery Vladislav Shpilevoy 2020-05-01 0:16 ` [Tarantool-patches] [PATCH vshard 1/7] test: print errors in a portable way Vladislav Shpilevoy 2020-05-01 16:58 ` Oleg Babin 2020-05-02 20:08 ` Vladislav Shpilevoy 2020-05-04 14:26 ` Oleg Babin 2020-05-01 0:16 ` [Tarantool-patches] [PATCH vshard 2/7] router: introduce discovery_mode Vladislav Shpilevoy 2020-05-01 16:59 ` Oleg Babin 2020-05-01 0:16 ` [Tarantool-patches] [PATCH vshard 3/7] test: disable router discovery for some tests Vladislav Shpilevoy 2020-05-01 17:00 ` Oleg Babin 2020-05-02 20:09 ` Vladislav Shpilevoy 2020-05-04 14:26 ` Oleg Babin 2020-05-01 0:16 ` [Tarantool-patches] [PATCH vshard 4/7] test: clear route map, respecting statistics Vladislav Shpilevoy 2020-05-01 17:00 ` Oleg Babin 2020-05-01 0:16 ` [Tarantool-patches] [PATCH vshard 5/7] router: keep known bucket count stat up to date Vladislav Shpilevoy 2020-05-01 17:01 ` Oleg Babin 2020-05-01 0:16 ` [Tarantool-patches] [PATCH vshard 6/7] router: make discovery smoother in a big cluster Vladislav Shpilevoy 2020-05-01 17:01 ` Oleg Babin 2020-05-02 20:12 ` Vladislav Shpilevoy 2020-05-04 14:26 ` Oleg Babin 2020-05-04 21:09 ` Vladislav Shpilevoy [this message] 2020-05-06 8:27 ` Oleg Babin 2020-05-07 22:45 ` Konstantin Osipov 2020-05-08 19:56 ` Vladislav Shpilevoy 2020-05-09 7:37 ` Konstantin Osipov 2020-05-01 0:16 ` [Tarantool-patches] [PATCH vshard 7/7] router: introduce discovery mode 'once' Vladislav Shpilevoy 2020-05-01 17:02 ` Oleg Babin 2020-05-02 20:12 ` Vladislav Shpilevoy 2020-05-04 14:27 ` Oleg Babin 2020-05-06 20:54 ` [Tarantool-patches] [PATCH vshard 0/7] Router extended discovery Vladislav Shpilevoy
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=df14f457-a4fa-0dff-afe3-49dc9cbc8a23@tarantool.org \ --to=v.shpilevoy@tarantool.org \ --cc=olegrok@tarantool.org \ --cc=tarantool-patches@dev.tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH vshard 6/7] router: make discovery smoother in a big cluster' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox