Skip to content

fix: vector count returns 0 due to account_id mismatch in observer#802

Closed
jackjin1997 wants to merge 1 commit intovolcengine:mainfrom
jackjin1997:fix/vector-count-account-mismatch
Closed

fix: vector count returns 0 due to account_id mismatch in observer#802
jackjin1997 wants to merge 1 commit intovolcengine:mainfrom
jackjin1997:fix/vector-count-account-mismatch

Conversation

@jackjin1997
Copy link
Contributor

Summary

Fixes #786

Root Cause

VikingDBObserver.count() calls VikingVectorIndexBackend.count(ctx=None), which defaults to _get_default_backend() — a backend bound to account_id="default".

When vectors are written with a different account_id (derived from the user's RequestContext in TextEmbeddingHandler), the count query filters them out, returning 0 even though vectors are properly persisted in LevelDB.

Code path:

Observer → VikingDBManager.count()
         → VikingVectorIndexBackend.count(ctx=None)
         → _get_default_backend()  ← bound to account_id="default"
         → _SingleAccountBackend.count()
         → Eq("account_id", "default")  ← misses non-default accounts

Fix

  • Add include_all_accounts: bool parameter to VikingVectorIndexBackend.count()
  • When True, uses _get_root_backend() (no account_id filter) to count all vectors
  • Update VikingDBObserver and get_collection_info() to use include_all_accounts=True
  • Backward compatible — existing callers with ctx or default behavior unchanged

Changes

  • openviking/storage/viking_vector_index_backend.py — add include_all_accounts param to count(), fix get_collection_info
  • openviking/storage/observers/vikingdb_observer.py — use include_all_accounts=True

Fixes volcengine#786

Root cause: VikingDBObserver.count() calls the backend without a
RequestContext, which defaults to filtering by account_id='default'.
When vectors are written with a different account_id (from the user's
actual context), the count query misses them entirely.

Fix: Add include_all_accounts parameter to count(). When True, uses
the root backend (no account_id filter) to count all vectors across
all tenants. The observer and get_collection_info now use this to
report accurate totals.
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link
Collaborator

@qin-ctx qin-ctx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the investigation — the root cause analysis of the account_id mismatch is correct. However, the fix direction needs reconsideration.

Multi-tenant isolation concern

Using include_all_accounts=True bypasses tenant isolation, which breaks the multi-tenant contract. The observer API is called with a user's API key (each tenant has their own), so it should only return that tenant's vector count — not a global total across all accounts.

The real issue is that RequestContext is available at the router level (observer.py:57, the _ctx parameter) but is never passed down the chain:

router (has _ctx but ignores it)
  → ObserverService.vikingdb (property, no params)
    → VikingDBObserver (no ctx)
      → vikingdb_manager.count() (no ctx → defaults to account_id="default")

The proper fix would be to propagate ctx through the observer chain so count(ctx=ctx) queries the correct tenant's data.

Runtime bug in _SingleAccountBackend.get_collection_info()

The diff modifies get_collection_info() at line 133, which belongs to the inner class _SingleAccountBackend. It changes self.count() to self.count(include_all_accounts=True), but _SingleAccountBackend.count() (line 352) has this signature:

async def count(self, filter: Optional[Dict[str, Any] | FilterExpr] = None) -> int:

It does not accept include_all_accounts. This will raise TypeError: count() got an unexpected keyword argument 'include_all_accounts' at runtime.

Suggested approach

  1. Pass ctx from the router → ObserverServiceVikingDBObservervikingdb_manager.count(ctx=ctx)
  2. Each tenant sees only their own vector count
  3. No need for the include_all_accounts parameter

@qin-ctx
Copy link
Collaborator

qin-ctx commented Mar 20, 2026

Hi @jackjin1997, thanks for digging into this — your root cause analysis of the account_id mismatch is spot on.

We've opened an alternative fix in #807 that takes a different approach: instead of include_all_accounts=True, it threads RequestContext from the HTTP router through the observer chain so count(ctx=ctx) queries the correct tenant's data. This preserves multi-tenant isolation — each tenant only sees their own vector count.

Also heads-up on a runtime issue in this PR: the diff modifies _SingleAccountBackend.get_collection_info() (line 133) to call self.count(include_all_accounts=True), but _SingleAccountBackend.count() doesn't accept that parameter — it would raise a TypeError at runtime.

Feel free to take a look at #807 and let us know if you have any thoughts!

@github-project-automation github-project-automation bot moved this from Backlog to Done in OpenViking project Mar 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[Bug]: Vector Storage Not Persisting to RocksDB - Embedding Succeeds but Vector Count Remains 0

4 participants