From: Oleg Babin via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>,
tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH vshard 1/2] Drop Tarantool < 1.10.1 support
Date: Fri, 28 Jan 2022 08:47:22 +0300 [thread overview]
Message-ID: <640e1bef-d7a1-4e8e-ccd4-bde0d24d2904@tarantool.org> (raw)
In-Reply-To: <4027b571e5f7b9edbf9e153f127e2d1172cc2647.1643329358.git.v.shpilevoy@tarantool.org>
Hi! Thanks for your patch. LGTM.
On 28.01.2022 03:23, Vladislav Shpilevoy wrote:
> The reasons are:
>
> - Rebalancer won't work. It uses fiber.join().
>
> - Bucket discovery on router is extra expensive - it downloads the
> entire _bucket from each storage. That can make the latter and
> the router unresponsive for notable time if bucket count is big
> enough.
>
> - There are other minor hacks which complicate development. For
> instance, box.error.new() couldn't be used.
>
> - Tests don't pass. Thus can't tell what else is broken. Makes no
> sense to support it if it can't be even tested properly
>
> The reason why it is done now is because the code will get more
> complicated soon and will use more new features which even 1.10
> won't support. This patch should reduce the level of version
> dependency complications before they get even worse.
>
> Closes #256
> Closes #267
>
> @TarantoolBot document
> Title: VShard now supports only Tarantool >= 1.10.1
>
> It used to support 1.9 too, but now it is dropped. To use VShard
>> 0.1.19 a user needs Tarantool >= 1.10.1.
> ---
> vshard/cfg.lua | 5 ----
> vshard/error.lua | 9 ++----
> vshard/router/init.lua | 65 ++---------------------------------------
> vshard/storage/init.lua | 21 ++-----------
> vshard/util.lua | 4 +++
> 5 files changed, 13 insertions(+), 91 deletions(-)
>
> diff --git a/vshard/cfg.lua b/vshard/cfg.lua
> index a059d44..1903967 100644
> --- a/vshard/cfg.lua
> +++ b/vshard/cfg.lua
> @@ -2,7 +2,6 @@
>
> local log = require('log')
> local luri = require('uri')
> -local lutil = require('vshard.util')
> local consts = require('vshard.consts')
>
> local function check_uri(uri)
> @@ -22,10 +21,6 @@ local function check_replica_master(master, ctx)
> end
>
> local function check_replicaset_master(master, ctx)
> - -- Limit the version due to extensive usage of netbox is_async feature.
> - if not lutil.version_is_at_least(1, 10, 1) then
> - error('Only versions >= 1.10.1 support master discovery')
> - end
> if master ~= 'auto' then
> error('Only "auto" master is supported')
> end
> diff --git a/vshard/error.lua b/vshard/error.lua
> index c673827..d36803a 100644
> --- a/vshard/error.lua
> +++ b/vshard/error.lua
> @@ -235,8 +235,7 @@ local function make_error(e)
> -- box.error, return unpacked
> return box_error(e)
> elseif type(e) == 'string' then
> - local ok, err = pcall(box.error, box.error.PROC_LUA, e)
> - return box_error(err)
> + return box_error(box.error.new(box.error.PROC_LUA, e))
> elseif type(e) == 'table' then
> return setmetatable(e, {__tostring = json.encode})
> else
> @@ -271,12 +270,10 @@ local function make_alert(code, ...)
> end
>
> --
> --- Create a timeout error object. Box.error.new() can't be used because is
> --- present only since 1.10.
> +-- Create a timeout error object.
> --
> local function make_timeout()
> - local _, err = pcall(box.error, box.error.TIMEOUT)
> - return make_error(err)
> + return make_error(box.error.new(box.error.TIMEOUT))
> end
>
> return {
> diff --git a/vshard/router/init.lua b/vshard/router/init.lua
> index 5a94a2f..ebeaac9 100644
> --- a/vshard/router/init.lua
> +++ b/vshard/router/init.lua
> @@ -249,15 +249,10 @@ local function discovery_handle_buckets(router, replicaset, buckets)
> end
> end
>
> -local discovery_f
> -
> -if util.version_is_at_least(1, 10, 0) then
> --
> --- >= 1.10 version of discovery fiber does parallel discovery of
> --- all replicasets at once. It uses is_async feature of netbox
> --- for that.
> +-- Bucket discovery main loop.
> --
> -discovery_f = function(router)
> +local function discovery_f(router)
> local module_version = M.module_version
> assert(router.discovery_mode == 'on' or router.discovery_mode == 'once')
> local iterators = {}
> @@ -394,47 +389,6 @@ discovery_f = function(router)
> end
> end
>
> --- Version >= 1.10.
> -else
> --- Version < 1.10.
> -
> ---
> --- Background fiber to perform discovery. It periodically scans
> --- replicasets one by one and updates route_map.
> ---
> -discovery_f = function(router)
> - local module_version = M.module_version
> - while module_version == M.module_version do
> - while not next(router.replicasets) do
> - lfiber.sleep(consts.DISCOVERY_IDLE_INTERVAL)
> - end
> - local old_replicasets = router.replicasets
> - for rs_uuid, replicaset in pairs(router.replicasets) do
> - local active_buckets, err =
> - replicaset:callro('vshard.storage.buckets_discovery', {},
> - {timeout = 2})
> - while M.errinj.ERRINJ_LONG_DISCOVERY do
> - M.errinj.ERRINJ_LONG_DISCOVERY = 'waiting'
> - lfiber.sleep(0.01)
> - end
> - -- Renew replicasets object captured by the for loop
> - -- in case of reconfigure and reload events.
> - if router.replicasets ~= old_replicasets then
> - break
> - end
> - if not active_buckets then
> - log.error('Error during discovery %s: %s', replicaset, err)
> - else
> - discovery_handle_buckets(router, replicaset, active_buckets)
> - end
> - lfiber.sleep(consts.DISCOVERY_IDLE_INTERVAL)
> - end
> - end
> -end
> -
> --- Version < 1.10.
> -end
> -
> --
> -- Immediately wakeup discovery fiber if exists.
> --
> @@ -758,9 +712,6 @@ local function router_call(router, bucket_id, opts, ...)
> ...)
> end
>
> -local router_map_callrw
> -
> -if util.version_is_at_least(1, 10, 0) then
> --
> -- Consistent Map-Reduce. The given function is called on all masters in the
> -- cluster with a guarantee that in case of success it was executed with all
> @@ -800,7 +751,7 @@ if util.version_is_at_least(1, 10, 0) then
> -- about concrete replicaset. For example, not all buckets were found even
> -- though all replicasets were scanned.
> --
> -router_map_callrw = function(router, func, args, opts)
> +local function router_map_callrw(router, func, args, opts)
> local replicasets = router.replicasets
> local timeout = opts and opts.timeout or consts.CALL_TIMEOUT_MIN
> local deadline = fiber_clock() + timeout
> @@ -919,16 +870,6 @@ router_map_callrw = function(router, func, args, opts)
> return nil, err, err_uuid
> end
>
> --- Version >= 1.10.
> -else
> --- Version < 1.10.
> -
> -router_map_callrw = function()
> - error('Supported for Tarantool >= 1.10')
> -end
> -
> -end
> -
> --
> -- Get replicaset object by bucket identifier.
> -- @param bucket_id Bucket identifier.
> diff --git a/vshard/storage/init.lua b/vshard/storage/init.lua
> index 3eaad9c..78103cf 100644
> --- a/vshard/storage/init.lua
> +++ b/vshard/storage/init.lua
> @@ -1288,11 +1288,8 @@ local function bucket_recv_xc(bucket_id, from, data, opts)
> local space_name, space_data = row[1], row[2]
> local space = box.space[space_name]
> if space == nil then
> - -- Tarantool doesn't provide API to create box.error
> - -- objects before 1.10.
> - local _, boxerror = pcall(box.error, box.error.NO_SUCH_SPACE,
> - space_name)
> - return nil, lerror.box(boxerror)
> + local err = box.error.new(box.error.NO_SUCH_SPACE, space_name)
> + return nil, lerror.box(err)
> end
> box.begin()
> for _, tuple in ipairs(space_data) do
> @@ -2209,13 +2206,6 @@ end
> --
> local function rebalancer_worker_f(worker_id, dispenser, quit_cond)
> lfiber.name(string.format('vshard.rebalancer_worker_%d', worker_id))
> - if not util.version_is_at_least(1, 10, 0) then
> - -- Return control to the caller immediately to allow it
> - -- to finish preparations. In 1.9 a caller couldn't create
> - -- a fiber without switching to it.
> - lfiber.yield()
> - end
> -
> local _status = box.space._bucket.index.status
> local opts = {timeout = consts.REBALANCER_CHUNK_TIMEOUT}
> local active_key = {consts.BUCKET.ACTIVE}
> @@ -2299,12 +2289,7 @@ local function rebalancer_apply_routes_f(routes)
> local quit_cond = lfiber.cond()
> local workers = table.new(worker_count, 0)
> for i = 1, worker_count do
> - local f
> - if util.version_is_at_least(1, 10, 0) then
> - f = lfiber.new(rebalancer_worker_f, i, dispenser, quit_cond)
> - else
> - f = lfiber.create(rebalancer_worker_f, i, dispenser, quit_cond)
> - end
> + local f = lfiber.new(rebalancer_worker_f, i, dispenser, quit_cond)
> f:set_joinable(true)
> workers[i] = f
> end
> diff --git a/vshard/util.lua b/vshard/util.lua
> index afa658b..366fdea 100644
> --- a/vshard/util.lua
> +++ b/vshard/util.lua
> @@ -154,6 +154,10 @@ local function version_is_at_least(major_need, middle_need, minor_need)
> return minor >= minor_need
> end
>
> +if not version_is_at_least(1, 10, 1) then
> + error("VShard supports only Tarantool >= 1.10.1")
> +end
> +
> --
> -- Copy @a src table. Fiber yields every @a interval keys copied. Does not give
> -- any guarantees on what is the result when the source table is changed during
next prev parent reply other threads:[~2022-01-28 5:47 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-28 0:23 [Tarantool-patches] [PATCH vshard 0/2] Deprecations Vladislav Shpilevoy via Tarantool-patches
2022-01-28 0:23 ` [Tarantool-patches] [PATCH vshard 1/2] Drop Tarantool < 1.10.1 support Vladislav Shpilevoy via Tarantool-patches
2022-01-28 5:47 ` Oleg Babin via Tarantool-patches [this message]
2022-01-28 0:23 ` [Tarantool-patches] [PATCH vshard 2/2] Drop periodic Lua GC explicit call Vladislav Shpilevoy via Tarantool-patches
2022-01-28 5:47 ` Oleg Babin via Tarantool-patches
2022-01-28 22:19 ` Vladislav Shpilevoy via Tarantool-patches
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=640e1bef-d7a1-4e8e-ccd4-bde0d24d2904@tarantool.org \
--to=tarantool-patches@dev.tarantool.org \
--cc=olegrok@tarantool.org \
--cc=v.shpilevoy@tarantool.org \
--subject='Re: [Tarantool-patches] [PATCH vshard 1/2] Drop Tarantool < 1.10.1 support' \
/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