Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions python/arcticdb/flattener.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -27,6 +27,19 @@
from arcticdb.preconditions import check
from arcticdb_ext import get_config_int

# 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)


class Flattener:
# Probably a bad idea given the dict key could have this, fine for now as write does not allow this symbol anyways.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)


Expand All @@ -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]}

Expand Down
Loading