[Tarantool-patches] [PATCH vshard 2/5] storage: auto enable/disable
Oleg Babin
olegrok at tarantool.org
Fri Dec 17 14:09:32 MSK 2021
Thanks for your patch. See my two nits below.
On 17.12.2021 03:25, Vladislav Shpilevoy wrote:
> +--------------------------------------------------------------------------------
> +-- Public API protection
> +--------------------------------------------------------------------------------
> +
> +--
> +-- Arguments are listed explicitly instead of '...' because the latter does not
> +-- jit.
> +--
> +local function storage_api_call_safe(func, arg1, arg2, arg3, arg4)
> + return func(arg1, arg2, arg3, arg4)
> +end
> +
> +--
> +-- Unsafe proxy is loaded with protections. But it is used rarely and only in
> +-- the beginning of instance's lifetime.
> +--
> +local function storage_api_call_unsafe(func, arg1, arg2, arg3, arg4)
> + -- box.info is quite expensive. Avoid calling it again when the instance
> + -- is finally loaded.
> + if not M.is_loaded then
> + -- box.info raises an error until box.cfg() is started.
> + local ok, status = pcall(function()
> + return box.info.status
> + end)
nit: It could be changed to type(box.cfg) == 'function'. I'd call it
"common" pattern to check that box is not yet configured.
> + if not ok then
> + local msg = 'box seem not to be configured'
nit: seem -> seems?
> + return error(lerror.vshard(lerror.code.STORAGE_IS_DISABLED, msg))
> + end
> + -- 'Orphan' is allowed because even if a replica is an orphan, it still
> + -- could be up to date. Just not all other replicas are connected.
> + if status ~= 'running' and status ~= 'orphan' then
> + local msg = ('instance status is "%s"'):format(status)
> + return error(lerror.vshard(lerror.code.STORAGE_IS_DISABLED, msg))
> + end
> + M.is_loaded = true
> + end
> + if not M.is_configured then
> + local msg = 'storage is not configured'
> + return error(lerror.vshard(lerror.code.STORAGE_IS_DISABLED, msg))
> + end
> + M.api_call_cache = storage_api_call_safe
> + return func(arg1, arg2, arg3, arg4)
> +end
> +
> +M.api_call_cache = storage_api_call_unsafe
> +
> +local function storage_make_api(func)
> + return function(arg1, arg2, arg3, arg4)
> + return M.api_call_cache(func, arg1, arg2, arg3, arg4)
> + end
> +end
> +
> --------------------------------------------------------------------------------
> -- Module definition
> --------------------------------------------------------------------------------
> @@ -3021,44 +3091,67 @@ M.bucket_are_all_rw = bucket_are_all_rw_public
> M.bucket_generation_wait = bucket_generation_wait
> lregistry.storage = M
>
> +--
> +-- Not all methods are public here. Private methods should not be exposed if
> +-- possible. At least not without notable difference in naming.
> +--
> return {
> - sync = sync,
> - bucket_force_create = bucket_force_create,
> - bucket_force_drop = bucket_force_drop,
> - bucket_collect = bucket_collect,
> - bucket_recv = bucket_recv,
> - bucket_send = bucket_send,
> - bucket_stat = bucket_stat,
> - bucket_pin = bucket_pin,
> - bucket_unpin = bucket_unpin,
> - bucket_ref = bucket_ref,
> - bucket_unref = bucket_unref,
> - bucket_refro = bucket_refro,
> - bucket_refrw = bucket_refrw,
> - bucket_unrefro = bucket_unrefro,
> - bucket_unrefrw = bucket_unrefrw,
> - bucket_delete_garbage = bucket_delete_garbage,
> - garbage_collector_wakeup = garbage_collector_wakeup,
> - rebalancer_wakeup = rebalancer_wakeup,
> - rebalancer_apply_routes = rebalancer_apply_routes,
> - rebalancer_disable = rebalancer_disable,
> - rebalancer_enable = rebalancer_enable,
> - is_locked = is_this_replicaset_locked,
> - rebalancing_is_in_progress = rebalancing_is_in_progress,
> - recovery_wakeup = recovery_wakeup,
> - call = storage_call,
> - _call = service_call,
> + --
> + -- Bucket methods.
> + --
> + bucket_force_create = storage_make_api(bucket_force_create),
> + bucket_force_drop = storage_make_api(bucket_force_drop),
> + bucket_collect = storage_make_api(bucket_collect),
> + bucket_recv = storage_make_api(bucket_recv),
> + bucket_send = storage_make_api(bucket_send),
> + bucket_stat = storage_make_api(bucket_stat),
> + bucket_pin = storage_make_api(bucket_pin),
> + bucket_unpin = storage_make_api(bucket_unpin),
> + bucket_ref = storage_make_api(bucket_ref),
> + bucket_unref = storage_make_api(bucket_unref),
> + bucket_refro = storage_make_api(bucket_refro),
> + bucket_refrw = storage_make_api(bucket_refrw),
> + bucket_unrefro = storage_make_api(bucket_unrefro),
> + bucket_unrefrw = storage_make_api(bucket_unrefrw),
> + bucket_delete_garbage = storage_make_api(bucket_delete_garbage),
> + _bucket_delete_garbage = bucket_delete_garbage,
> + buckets_info = storage_make_api(storage_buckets_info),
> + buckets_count = storage_make_api(bucket_count_public),
> + buckets_discovery = storage_make_api(buckets_discovery),
> + --
> + -- Garbage collector.
> + --
> + garbage_collector_wakeup = storage_make_api(garbage_collector_wakeup),
> + --
> + -- Rebalancer.
> + --
> + rebalancer_wakeup = storage_make_api(rebalancer_wakeup),
> + rebalancer_apply_routes = storage_make_api(rebalancer_apply_routes),
> + rebalancer_disable = storage_make_api(rebalancer_disable),
> + rebalancer_enable = storage_make_api(rebalancer_enable),
> + rebalancing_is_in_progress = storage_make_api(rebalancing_is_in_progress),
> + rebalancer_request_state = storage_make_api(rebalancer_request_state),
> + _rebalancer_request_state = rebalancer_request_state,
> + --
> + -- Recovery.
> + --
> + recovery_wakeup = storage_make_api(recovery_wakeup),
> + --
> + -- Instance info.
> + --
> + is_locked = storage_make_api(is_this_replicaset_locked),
> + info = storage_make_api(storage_info),
> + sharded_spaces = storage_make_api(storage_sharded_spaces),
> + _sharded_spaces = storage_sharded_spaces,
> + module_version = function() return M.module_version end,
> + --
> + -- Miscellaneous.
> + --
> + call = storage_make_api(storage_call),
> + _call = storage_make_api(service_call),
> + sync = storage_make_api(sync),
> cfg = function(cfg, uuid) return storage_cfg(cfg, uuid, false) end,
> - info = storage_info,
> - buckets_info = storage_buckets_info,
> - buckets_count = bucket_count_public,
> - buckets_discovery = buckets_discovery,
> - rebalancer_request_state = rebalancer_request_state,
> + on_master_enable = storage_make_api(on_master_enable),
> + on_master_disable = storage_make_api(on_master_disable),
> internal = M,
> - on_master_enable = on_master_enable,
> - on_master_disable = on_master_disable,
> - sharded_spaces = function()
> - return table.deepcopy(find_sharded_spaces())
> - end,
> - module_version = function() return M.module_version end,
> }
More information about the Tarantool-patches
mailing list