Skip to content

The #151-4 collision guard's sanitize-collision path is untested for every engine (only the _EXACT path is covered) #294

Description

@fcostaoliveira

experiment::run's #151-4 startup collision guard has two ways to fire. Only one of them is tested, and the untested one is the one that fires on ordinary config names.

The two paths

derive_index_name(base_env, "idx", &config.name) collides when either:

  1. <ENGINE>_INDEX_NAME_EXACT=1 is set with >1 config for that engine — every config resolves to the same verbatim base. This one is covered (PR fix(vectorsets): derive the vector-set key per config so concurrent runs stop clobbering each other #278 added test_vectorsets_exact_pin_with_two_configs_is_rejected_at_startup, for vectorsets).

  2. Two distinct config names sanitise to the same token. sanitize_token maps every char outside [A-Za-z0-9_-] to _, so it is non-injective:

    vectorsets-coll:a  ->  vectorsets-coll_a
    vectorsets-coll_a  ->  vectorsets-coll_a
    

    Also a*b, a b, a?b, a:b all collapse to a_b. Two such configs derive one index namespace and would silently overwrite each other — exactly the bug Vertex benchmark reliability: 401 handling, token refresh, --engines-file no-op, Redis M×EFC index collision, search watchdog #151-4 exists to prevent.

Path 2 has no test for any engine. grep -rn "derive the same index namespace" tests/ returns nothing.

Why it is worth a test rather than a shrug

The production guard does handle it — verified live, the run refuses to start and names both configs. So this is a coverage gap, not a live bug. But path 2 is the one reachable without any env var, on nothing more exotic than a config named with a colon or a space, and it is the path whose failure mode is silent data loss rather than a wrong-looking name. A guard that is only exercised through its env-var branch could lose the ordinary branch to a refactor without CI noticing.

Worth noting the guard is also per-invocation: it catches two colliding configs in one sweep, but two sequential single-config runs whose names sanitise together still clobber, and nothing detects that. Same for _EXACT plus two sequential runs. Probably out of scope for a test, but worth recording as the limit of what the guard promises.

Suggested

One integration test per Redis-wire engine family (or one shared one), asserting that a config set containing two names that sanitise to the same token is rejected at startup with the derive the same index namespace error — the same shape as the _EXACT test in PR #278, which can be copied.

Context: found during the mutation-testing review of PR #278 (#236). All 19 shipped vectorsets-* configs are sanitize-clean with zero collisions, so nothing in-tree is currently affected.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions