From ad918ea2211e8f84a78827e9b0aa25398fca65ea Mon Sep 17 00:00:00 2001 From: Phoebus Mak Date: Thu, 11 Jun 2026 14:34:17 +0100 Subject: [PATCH 1/3] Hardpin to 511 --- python/arcticdb/flattener.py | 13 +++++++++++-- .../version_store/test_recursive_normalizers.py | 14 ++++++++++---- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/python/arcticdb/flattener.py b/python/arcticdb/flattener.py index cfe6efaa8ce..9dd113adcbb 100644 --- a/python/arcticdb/flattener.py +++ b/python/arcticdb/flattener.py @@ -14,11 +14,11 @@ from arcticdb.exceptions import DataTooNestedException, UnsupportedKeyInDictionary try: - from msgpack.fallback import DEFAULT_RECURSE_LIMIT + from msgpack.fallback import DEFAULT_RECURSE_LIMIT as _DEFAULT_RECURSE_LIMIT except ImportError: # The default as of msgpack 1.1.0 - handle the import error in case the msgpack wheel stops exporting this constant. # Want to keep compatibility with a wide range of msgpack versions. - DEFAULT_RECURSE_LIMIT = 511 + _DEFAULT_RECURSE_LIMIT = 511 from arcticdb import _msgpack_compat from arcticdb.log import version as log @@ -27,6 +27,15 @@ from arcticdb.preconditions import check from arcticdb_ext import get_config_int +# The binding constraint is the read path: reconstructing the data recurses ~3 Python stack frames +# per nesting level (see create_original_obj_from_metastruct), so with the default recursion limit of +# 1000 reads fail above ~330 levels. We pin the cap at 255 - the value it was effectively fixed at for +# years, when msgpack's DEFAULT_RECURSE_LIMIT was 511 (511 // 2 == 255) - which both packs on write and +# round-trips on read. The min() keeps msgpack's packer as a ceiling too. Tying the cap directly to +# DEFAULT_RECURSE_LIMIT broke when msgpack 1.2.0 raised it to 1024, letting writes through at depths the +# reader could not reconstruct. +DEFAULT_RECURSE_LIMIT = min(511, _DEFAULT_RECURSE_LIMIT) + class Flattener: # Probably a bad idea given the dict key could have this, fine for now as write does not allow this symbol anyways. diff --git a/python/tests/unit/arcticdb/version_store/test_recursive_normalizers.py b/python/tests/unit/arcticdb/version_store/test_recursive_normalizers.py index ffe55218090..e89514e89df 100644 --- a/python/tests/unit/arcticdb/version_store/test_recursive_normalizers.py +++ b/python/tests/unit/arcticdb/version_store/test_recursive_normalizers.py @@ -8,7 +8,7 @@ import numpy as np import arcticdb from arcticdb import QueryBuilder, LibraryOptions -from arcticdb.flattener import Flattener +from arcticdb.flattener import Flattener, DEFAULT_RECURSE_LIMIT from arcticdb.version_store._custom_normalizers import ( CustomNormalizer, register_normalizer, @@ -392,12 +392,18 @@ def test_deep_nesting_metastruct_size_over_limit(lmdb_version_store_v1, all_recu key = "reasonable_length_key" data = {key: pd.DataFrame({"col": [0]})} - nesting_levels = 256 + # Mirror the guard in flattener._create_meta_structure (raises above DEFAULT_RECURSE_LIMIT // 2). + # flattener clamps DEFAULT_RECURSE_LIMIT to 511, so this is 255 - see the note there for why. + nesting_limit = DEFAULT_RECURSE_LIMIT // 2 + nesting_levels = nesting_limit + 1 for i in range(nesting_levels - 1): data[key] = {key: data[key]} # When & Then - with pytest.raises(DataTooNestedException, match=r"^Symbol sym cannot be recursively normalized.*255 levels.*"): + with pytest.raises( + DataTooNestedException, + match=rf"^Symbol sym cannot be recursively normalized.*{nesting_limit} levels.*", + ): lib.write(sym, data, recursive_normalizers=True) @@ -408,7 +414,7 @@ def test_deep_nesting_metastruct_size_under_limit(lmdb_version_store_v1, all_rec key = "reasonable_length_key" data = {key: pd.DataFrame({"col": [0]})} - nesting_levels = 255 + nesting_levels = (DEFAULT_RECURSE_LIMIT - 1) // 2 for i in range(nesting_levels - 1): data[key] = {key: data[key]} From 94e7a9f44c5dc708899c30163dba4d0b47a40984 Mon Sep 17 00:00:00 2001 From: Phoebus Mak Date: Thu, 11 Jun 2026 18:12:26 +0100 Subject: [PATCH 2/3] Add task link --- python/arcticdb/flattener.py | 1 + 1 file changed, 1 insertion(+) diff --git a/python/arcticdb/flattener.py b/python/arcticdb/flattener.py index 9dd113adcbb..56326c08af0 100644 --- a/python/arcticdb/flattener.py +++ b/python/arcticdb/flattener.py @@ -34,6 +34,7 @@ # round-trips on read. The min() keeps msgpack's packer as a ceiling too. Tying the cap directly to # DEFAULT_RECURSE_LIMIT broke when msgpack 1.2.0 raised it to 1024, letting writes through at depths the # reader could not reconstruct. +# https://man312219.monday.com/boards/7852509418/pulses/12254825163 DEFAULT_RECURSE_LIMIT = min(511, _DEFAULT_RECURSE_LIMIT) From 0b6a5732f311e9e96071771bd03b8634a7ccb629 Mon Sep 17 00:00:00 2001 From: Phoebus Mak Date: Fri, 12 Jun 2026 14:56:41 +0100 Subject: [PATCH 3/3] Update comment --- python/arcticdb/flattener.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/python/arcticdb/flattener.py b/python/arcticdb/flattener.py index 56326c08af0..bf83fd91197 100644 --- a/python/arcticdb/flattener.py +++ b/python/arcticdb/flattener.py @@ -27,13 +27,16 @@ from arcticdb.preconditions import check from arcticdb_ext import get_config_int -# The binding constraint is the read path: reconstructing the data recurses ~3 Python stack frames -# per nesting level (see create_original_obj_from_metastruct), so with the default recursion limit of -# 1000 reads fail above ~330 levels. We pin the cap at 255 - the value it was effectively fixed at for -# years, when msgpack's DEFAULT_RECURSE_LIMIT was 511 (511 // 2 == 255) - which both packs on write and -# round-trips on read. The min() keeps msgpack's packer as a ceiling too. Tying the cap directly to -# DEFAULT_RECURSE_LIMIT broke when msgpack 1.2.0 raised it to 1024, letting writes through at depths the -# reader could not reconstruct. +# Two separate limits govern maximum nesting depth: +# Write (msgpack packer): uses ~2 Python stack frames per level; capped at DEFAULT_RECURSE_LIMIT // 2. +# Read (create_original_obj_from_metastruct): uses ~3 Python stack frames per level, so reads +# fail above sys.getrecursionlimit() // 3 ≈ 333 with Python's default limit of 1000. +# +# The read path is tighter, so we pin msgpack's DEFAULT_RECURSE_LIMIT at 511, giving a write +# cap of 511 // 2 == 255 — safely below the ~333 read ceiling. msgpack 1.2.0 raised its own +# constant DEFAULT_RECURSE_LIMIT to 1024, which silently allowed writes that the reader could not +# reconstruct. We keep 511 regardless of what msgpack exports. +# TODO: once the metastruct reader is non-recursive, raise to (sys.getrecursionlimit() - 10) // 2. # https://man312219.monday.com/boards/7852509418/pulses/12254825163 DEFAULT_RECURSE_LIMIT = min(511, _DEFAULT_RECURSE_LIMIT)