Tarantool development patches archive
 help / color / mirror / Atom feed
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

  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