diff --git a/simplyblock_core/models/storage_node.py b/simplyblock_core/models/storage_node.py index 836ff375a5..572fa1683c 100644 --- a/simplyblock_core/models/storage_node.py +++ b/simplyblock_core/models/storage_node.py @@ -186,6 +186,25 @@ def get_hublvol_port(self, lvs_name=None): return self.hublvol.nvmf_port return 0 + def lvstore_cluster_size(self, lvs_name: str) -> Optional[int]: + """The ``cluster_sz`` this node's lvstore ``lvs_name`` was created with, + in bytes, or None when the persisted stack has no such lvstore. + + ``lvstore_stack`` is written once by create_lvstore with the exact + parameters sent to SPDK and replayed verbatim on every restart, so this + is the running lvstore's real cluster size whatever release created it. + Anything that turns ``num_allocated_clusters`` into bytes must use this + rather than recompute the size from the current cluster settings: the + formula has changed between releases (page size x1 before 26.3.2, x ndcs + since) and an lvstore keeps the size it was born with. + """ + for entry in self.lvstore_stack: + if entry.get("type") == "bdev_lvstore" and entry.get("name") == lvs_name: + cluster_sz = (entry.get("params") or {}).get("cluster_sz") + if cluster_sz: + return int(cluster_sz) + return None + def client(self, **kwargs): """Return API client to this node """ diff --git a/simplyblock_core/services/lvol_stat_collector.py b/simplyblock_core/services/lvol_stat_collector.py index 5736e0ffdd..c1058f8492 100644 --- a/simplyblock_core/services/lvol_stat_collector.py +++ b/simplyblock_core/services/lvol_stat_collector.py @@ -34,7 +34,37 @@ def sum_stats(stats_list): return ret -def add_lvol_stats(cluster, lvol, stats_list, capacity_dict=None): +def lvstore_cluster_size(cluster, lvol, snode) -> int: + """Bytes per lvstore cluster for ``lvol``, i.e. what one unit of + ``num_allocated_clusters`` is worth. + + Taken from the lvstore's persisted creation parameters on its node + (``StorageNode.lvstore_cluster_size``): that is the size SPDK was given + and replays on every restart, whatever release created the lvstore. + Recomputing it from the current cluster settings is wrong for any lvstore + created under a different formula -- lvstores built before 26.3.2 use one + page per cluster, those since use ``ndcs`` pages, and the 26.3.2 collector + assumed the new size for all of them, doubling reported usage on a 2x2 + cluster (customer report 2026-10-21). + + The settings-based formula remains only as a fallback for a node record + with no lvstore entry, and is logged when used so a mis-sized sample is + traceable rather than silent. + """ + cluster_size = snode.lvstore_cluster_size(lvol.lvs_name) if snode is not None else None + if cluster_size: + return cluster_size + # distr_ndcs defaults to 0, and multiplying by it would report every + # volume as using nothing at all -- a worse answer than one page. + cluster_size = cluster.page_size_in_blocks * max(cluster.distr_ndcs, 1) + logger.warning( + "lvstore %s of lvol %s has no persisted cluster_sz on node %s; " + "assuming %d bytes from the cluster settings, reported usage may be off", + lvol.lvs_name, lvol.get_id(), snode.get_id() if snode is not None else None, cluster_size) + return cluster_size + + +def add_lvol_stats(cluster, lvol, stats_list, capacity_dict=None, snode=None): now = int(time.time()) data = { "pool_id": lvol.pool_uuid, @@ -45,15 +75,7 @@ def add_lvol_stats(cluster, lvol, stats_list, capacity_dict=None): size_used = 0 lvol_dict = capacity_dict size_total = int(lvol_dict['num_blocks']*lvol_dict['block_size']) - # Must match what create_lvstore sized the lvstore with, or every - # allocated cluster is converted back into the wrong number of bytes - # and reported usage is off by the data-chunk count. - # - # Falls back to one page when the cluster records no data-chunk count: - # distr_ndcs defaults to 0, and multiplying by it would report every - # volume as using nothing at all -- a worse answer than the single-page - # size this used to assume. - cluster_size = cluster.page_size_in_blocks * max(cluster.distr_ndcs, 1) + cluster_size = lvstore_cluster_size(cluster, lvol, snode) if "driver_specific" in lvol_dict and "lvol" in lvol_dict["driver_specific"]: num_allocated_clusters = lvol_dict["driver_specific"]["lvol"]["num_allocated_clusters"] size_used = int(num_allocated_clusters*cluster_size) @@ -268,7 +290,7 @@ def main(): if ret: capacity_dict = ret[0] - record = add_lvol_stats(cluster, lvol, stats, capacity_dict) + record = add_lvol_stats(cluster, lvol, stats, capacity_dict, snode=snode) if record: if lvol.pool_uuid in pools_lvols_stats and pools_lvols_stats[lvol.pool_uuid]: pools_lvols_stats[lvol.pool_uuid].append(record) diff --git a/tests/unit/services/test_lvol_used_size_cluster_size.py b/tests/unit/services/test_lvol_used_size_cluster_size.py new file mode 100644 index 0000000000..e4a4ea0ce4 --- /dev/null +++ b/tests/unit/services/test_lvol_used_size_cluster_size.py @@ -0,0 +1,92 @@ +"""The lvol used-size sample converts allocated lvstore clusters into bytes with +the cluster size the lvstore was actually created with, not one recomputed +from the current cluster settings. + +Customer report 2026-10-21 (26.3.2.1, 2x2 cluster): ``sbctl lvol list`` showed +39.7 TiB used on a 21 TiB volume. 26.3.2 changed ``create_lvstore`` to size a +cluster at ``page_size x ndcs`` and the collector to assume that size for every +lvstore -- but the customer's lvstores were created on 26.3.1 with one page per +cluster, so every allocated cluster was counted twice. The node record persists +the creation parameters (``lvstore_stack``), and that is the source of truth. +""" +from unittest.mock import MagicMock + +from simplyblock_core.models.storage_node import StorageNode +from simplyblock_core.services import lvol_stat_collector as collector + +PAGE = 2097152 # page_size_in_blocks default +LVS = "LVS_1234" + + +def _node(cluster_sz): + node = StorageNode() + node.uuid = "node-1" + node.lvstore = LVS + node.lvstore_stack = [ + {"type": "bdev_raid", "name": "raid0_1234", "params": {}}, + {"type": "bdev_lvstore", "name": LVS, + "params": {"name": LVS, "bdev_name": "raid0_1234", "cluster_sz": cluster_sz}}, + ] + return node + + +def _cluster(ndcs=2): + cluster = MagicMock() + cluster.page_size_in_blocks = PAGE + cluster.distr_ndcs = ndcs + return cluster + + +def _lvol(): + lvol = MagicMock() + lvol.uuid = "lvol-uuid" + lvol.lvs_name = LVS + lvol.pool_uuid = "pool-uuid" + lvol.get_id.return_value = "lvol-uuid" + return lvol + + +def test_node_reports_the_persisted_cluster_size(): + assert _node(PAGE).lvstore_cluster_size(LVS) == PAGE + assert _node(2 * PAGE).lvstore_cluster_size(LVS) == 2 * PAGE + + +def test_node_without_that_lvstore_reports_none(): + assert _node(PAGE).lvstore_cluster_size("LVS_other") is None + assert StorageNode().lvstore_cluster_size(LVS) is None + + +def test_pre_26_3_2_lvstore_counts_one_page_per_cluster(): + # Created with cluster_sz = page_size (x1) on a cluster that is 2x2 today: + # the persisted size wins over the settings-derived 2 x page. + assert collector.lvstore_cluster_size(_cluster(ndcs=2), _lvol(), _node(PAGE)) == PAGE + + +def test_post_26_3_2_lvstore_counts_ndcs_pages_per_cluster(): + assert collector.lvstore_cluster_size(_cluster(ndcs=2), _lvol(), _node(2 * PAGE)) == 2 * PAGE + + +def test_used_never_exceeds_size_on_a_full_pre_26_3_2_volume(): + # 21 TiB volume, every cluster allocated, 2x2 cluster: the 26.3.2 formula + # reported 2x the volume size here. + size = 21 << 40 + allocated_clusters = size // PAGE + cluster_size = collector.lvstore_cluster_size(_cluster(ndcs=2), _lvol(), _node(PAGE)) + + assert allocated_clusters * cluster_size == size + + +def test_missing_stack_entry_falls_back_to_cluster_settings_and_warns(caplog): + node = _node(PAGE) + node.lvstore_stack = [] + + with caplog.at_level("WARNING"): + cluster_size = collector.lvstore_cluster_size(_cluster(ndcs=2), _lvol(), node) + + assert cluster_size == 2 * PAGE + assert any("no persisted cluster_sz" in r.getMessage() for r in caplog.records) + + +def test_no_node_falls_back_to_cluster_settings(): + # ndcs 0 is clamped to one page, as before. + assert collector.lvstore_cluster_size(_cluster(ndcs=0), _lvol(), None) == PAGE