Preserve stats config and values between reloads

After this patch, statistics module state (enabled/disabled, driver)
preserved between package reloads and Tarantool Cartridge role
reloads [1]. Since `metrics` package do not fully support
preserving registry between role reloads [2] now, this feature doesn't
work for `metrics` driver. Corresponding tests are marked with xfail
until issue is resolved for metrics.

1. https://www.tarantool.io/en/doc/latest/book/cartridge/cartridge_api/modules/cartridge.roles/#reload
2. tarantool/metrics#334

Follows up #224

Author: DifferentialOrange

README.md

@@ -722,6 +722,13 @@ registry they are stored as `tnt_crud_map_reduces`,
 `tnt_crud_tuples_fetched` and `tnt_crud_tuples_lookup` metrics
 with `{ operation = 'select', name = space_name }` labels.
 
+Statistics are preserved between package reloads or [Tarantool Cartridge
+role reloads](https://www.tarantool.io/en/doc/latest/book/cartridge/cartridge_api/modules/cartridge.roles/#reload).
+Beware that metrics 0.12.0 and below do not support
+preverving stats between role reload
+(see [tarantool/metrics#334](https://github.com/tarantool/metrics/issues/334)),
+thus this feature will be unsupported for `metrics` driver.
+
 ## Cartridge roles
 
 `cartridge.roles.crud-storage` is a Tarantool Cartridge role that depends on the

crud/stats/local_registry.lua

@@ -1,9 +1,10 @@
 local dev_checks = require('crud.common.dev_checks')
 local op_module = require('crud.stats.operation')
 local registry_common = require('crud.stats.registry_common')
+local stash = require('crud.stats.stash')
 
 local registry = {}
-local internal_registry = {}
+local internal_registry = stash.get('local_registry')
 
 --- Initialize local metrics registry
 --
@@ -31,7 +32,7 @@ end
 -- @treturn boolean Returns true.
 --
 function registry.destroy()
-    internal_registry = {}
+    internal_registry = stash.reset('local_registry')
 
     return true
 end

crud/stats/metrics_registry.lua

@@ -3,9 +3,11 @@ local is_package, metrics = pcall(require, 'metrics')
 local dev_checks = require('crud.common.dev_checks')
 local op_module = require('crud.stats.operation')
 local registry_common = require('crud.stats.registry_common')
+local stash = require('crud.stats.stash')
 
 local registry = {}
-local internal_registry = {}
+-- Used to cache collectors.
+local internal_registry = stash.get('metrics_registry')
 
 local metric_name = {
     -- Summary collector for all operations.
@@ -114,7 +116,7 @@ function registry.destroy()
         metrics.registry:unregister(c)
     end
 
-    internal_registry = {}
+    internal_registry = stash.reset('metrics_registry')
     return true
 end
 
@@ -321,4 +323,28 @@ function registry.observe_map_reduces(count, space_name)
     return true
 end
 
+-- Workaround for https://github.com/tarantool/metrics/issues/334 .
+-- This workaround does not prevent observations reset between role reloads,
+-- but it fixes collector unlink from registry. Without this workaround,
+-- we will continue to use cached collectors that are already cleaned up
+-- from registry and changes will not appear in metrics export output.
+local function workaround_role_reload()
+    if not registry.is_supported() then
+        return
+    end
+
+    -- Check if this registry was enabled before reload.
+    if next(internal_registry) == nil then
+        return
+    end
+
+    -- Check if base collector is in metrics package registry.
+    -- If it's not, then registry has beed cleaned up on role reload.
+    if metrics.registry:find('summary', metric_name.stats) == nil then
+        registry.init()
+    end
+end
+
+workaround_role_reload()
+
 return registry

crud/stats/module.lua

@@ -6,15 +6,12 @@ local vshard = require('vshard')
 local dev_checks = require('crud.common.dev_checks')
 local utils = require('crud.common.utils')
 local op_module = require('crud.stats.operation')
+local stash = require('crud.stats.stash')
 
 local StatsError = errors.new_class('StatsError', {capture_stack = false})
 
 local stats = {}
-local internal = {
-    registry = nil,
-    driver = nil,
-}
-stats.internal = internal
+local internal = stash.get('internal')
 
 local local_registry = require('crud.stats.local_registry')
 local metrics_registry = require('crud.stats.metrics_registry')
@@ -337,4 +334,10 @@ end
 --
 stats.op = op_module
 
+--- Stats module internal state (for debug/test)
+--
+-- @table internal
+--
+stats.internal = internal
+
 return stats

crud/stats/stash.lua

@@ -0,0 +1,54 @@
+local dev_checks = require('crud.common.dev_checks')
+
+local stash = {}
+
+local function get_G_name(name)
+    return ('__crud_stats_%s'):format(name)
+end
+
+--- Get a stash instance, initialize if needed
+--  Stashes are persistent to package reload and cartridge roles reload.
+--
+-- @function get
+--
+-- @tparam string name
+--  Stash identifier.
+--
+-- @treturn table A stash instance.
+--
+function stash.get(name)
+    dev_checks('string')
+
+    local _G_name = get_G_name(name)
+    local instance = rawget(_G, _G_name) or {}
+    rawset(_G, _G_name, instance)
+
+    local hotreload = package.loaded['cartridge.hotreload']
+    if hotreload ~= nil then
+        hotreload.whitelist_globals({_G_name})
+    end
+
+    return instance
+end
+
+--- Reset stash instance
+--  Stashes are persistent to package reload and cartridge roles reload.
+--
+-- @function get
+--
+-- @tparam string name
+--  Stash identifier.
+--
+-- @treturn table A stash instance.
+--
+function stash.reset(name)
+    dev_checks('string')
+
+    local _G_name = get_G_name(name)
+    local instance = {}
+    rawset(_G, _G_name, instance)
+
+    return instance
+end
+
+return stash

deps.sh

@@ -4,7 +4,7 @@
 set -e
 
 # Test dependencies:
-tarantoolctl rocks install luatest 0.5.5
+tarantoolctl rocks install luatest 0.5.6
 tarantoolctl rocks install luacheck 0.25.0
 tarantoolctl rocks install luacov 0.13.0
 

test/entrypoint/srv_select.lua

@@ -24,7 +24,6 @@ package.preload['customers-storage'] = function()
                 },
                 if_not_exists = true,
                 engine = engine,
-                id = 542,
             })
             -- primary index
             customers_space:create_index('id_index', {

test/entrypoint/srv_stats.lua

@@ -0,0 +1,64 @@
+#!/usr/bin/env tarantool
+
+require('strict').on()
+_G.is_initialized = function() return false end
+
+local log = require('log')
+local errors = require('errors')
+local cartridge = require('cartridge')
+
+package.preload['customers-storage'] = function()
+    local engine = os.getenv('ENGINE') or 'memtx'
+    return {
+        role_name = 'customers-storage',
+        init = function()
+            local customers_space = box.schema.space.create('customers', {
+                format = {
+                    {name = 'id', type = 'unsigned'},
+                    {name = 'bucket_id', type = 'unsigned'},
+                    {name = 'name', type = 'string'},
+                    {name = 'last_name', type = 'string'},
+                    {name = 'age', type = 'number'},
+                    {name = 'city', type = 'string'},
+                },
+                if_not_exists = true,
+                engine = engine,
+                id = 542,
+            })
+            -- primary index
+            customers_space:create_index('id_index', {
+                parts = { {field = 'id'} },
+                if_not_exists = true,
+            })
+            customers_space:create_index('bucket_id', {
+                parts = { {field = 'bucket_id'} },
+                unique = false,
+                if_not_exists = true,
+            })
+            customers_space:create_index('age_index', {
+                parts = { {field = 'age'} },
+                unique = false,
+                if_not_exists = true,
+            })
+        end,
+    }
+end
+
+local ok, err = errors.pcall('CartridgeCfgError', cartridge.cfg, {
+    advertise_uri = 'localhost:3301',
+    http_port = 8081,
+    bucket_count = 3000,
+    roles = {
+        'cartridge.roles.crud-router',
+        'cartridge.roles.crud-storage',
+        'customers-storage',
+    },
+    roles_reload_allowed = true,
+})
+
+if not ok then
+    log.error('%s', err)
+    os.exit(1)
+end
+
+_G.is_initialized = cartridge.is_healthy

test/helper.lua

@@ -411,4 +411,12 @@ function helpers.get_map_reduces_stat(router, space_name)
     ]], { space_name })
 end
 
+function helpers.reload_roles(srv)
+    local ok, err = srv.net_box:eval([[
+        return require("cartridge.roles").reload()
+    ]])
+
+    t.assert_equals({ok, err}, {true, nil})
+end
+
 return helpers

test/integration/reload_test.lua

@@ -8,14 +8,6 @@ local g = t.group()
 
 local helpers = require('test.helper')
 
-local function reload(srv)
-    local ok, err = srv.net_box:eval([[
-        return require("cartridge.roles").reload()
-    ]])
-
-    t.assert_equals({ok, err}, {true, nil})
-end
-
 g.before_all(function()
     g.cluster = helpers.Cluster:new({
         datadir = fio.tempdir(),
@@ -92,7 +84,7 @@ function g.test_router()
         t.assert_equals(last_insert[3], 'A', 'No workload for label A')
     end)
 
-    reload(g.router)
+    helpers.reload_roles(g.router)
 
     local cnt = #g.insertions_passed
     g.cluster:retrying({}, function()
@@ -117,7 +109,7 @@ function g.test_storage()
     -- snapshot with a signal
     g.s1_master.process:kill('USR1')
 
-    reload(g.s1_master)
+    helpers.reload_roles(g.s1_master)
 
     g.cluster:retrying({}, function()
         g.s1_master.net_box:call('box.snapshot')