bytedance / bytedance/videx

[Stats][Bug] Scope NDV methods and cache entries to each request

Open
#96 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
149
Forks
27
PR merge metrics
No merged PRs in 30d

Description

### Environment Setup

VIDEX Version: `8cad1b0f07236bc9067afcc36765b35d736e0a14` (current `main` as of 2026-08-22)
Deployment Mode: Standalone statistic server
OS: Windows 10 Pro N 22H2
Python: 3.9.13
Workload: deterministic in-memory `info_low()` requests

### Observed vs Expected Behavior

`VidexModelInnoDB.info_low()` stores the request's `ndv_method` in the shared model field `self.ndv_method`, but its NDV cache key contains only `(index_name, fields)`. A cached estimate produced by one method is therefore reused by later requests that explicitly select another method.

Minimal reproduction using the current class with a deterministic NDV stub:

```python
model.ndv_cache = TTLCache(maxsize=1000, ttl=1200)
model.ndv = MethodType(fake_ndv, model) # scale -> 10; GEE -> 20

first = model.info_low(request("scale"))
second = model.info_low(request("GEE"))
```

Actual result:

```text
calls: ['scale']
scale rec_per_key: 100.0
GEE rec_per_key: 100.0
cache key: ('idx_a', ('a',))
```

The second request never invokes `GEE`. With 1,000 table rows and the stubbed GEE NDV of 20, its expected `rec_per_key` is 50.0.

There is also a concurrency problem: `info_low()` assigns `self.ndv_method` per request while the Flask server runs with `threaded=True`, and table model instances are shared. Concurrent requests can overwrite the method while another request is calculating index prefixes.

Expected behavior:

- An explicit `ndv_method` must affect that request even when the same index prefix was previously cached with a different method.
- Concurrent requests must not communicate per-request method state through a mutable shared model field.

### Impact

The documented per-query estimator selection can be silently ignored based on request order. Under concurrent use, results can additionally depend on request interleaving, which makes cardinality and plan estimates non-deterministic.

### Related concurrency issue

#28 discusses concurrent hypothetical indexes interfering in the plugin layer. This report is narrower and independently reproducible in the statistic server's per-query NDV method/cache path; a maintainer comment on #28 requested a separate detailed reproduction for distinct concurrency bugs.

### Why this report has no PR

Adding `ndv_method` to the cache key would fix only the sequential stale-cache case. A complete fix should also decide how the method is passed through calculation without shared per-request state, and how access to the shared `TTLCache` is synchronized or otherwise scoped. That request-lifetime/concurrency design needs maintainer direction and a concurrent server-level regression test; this Windows audit environment cannot validate the database/plugin integration path.

Possible direction: keep `ndv_method` local to `info_low()`, pass it explicitly into `ndv()`, include the effective method in the cache key, and define locking or request-local cache semantics.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start at VidexModelInnoDB.info_low() and trace how ndv_method reaches ndv() and how TTLCache keys are formed. Reproduce the sequential scale/GEE case, then exercise concurrent requests through the threaded Flask server. Done means each request uses its selected estimator without stale cache reuse or cross-request method interference, with a concurrent regression test.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
databases
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.