Skip to content

Guard the SVS code paths so USE_SVS=OFF builds - #1006

Open
plusky wants to merge 1 commit into
RedisAI:mainfrom
plusky:guard-svs-include-in-tiered-factory-header
Open

Guard the SVS code paths so USE_SVS=OFF builds#1006
plusky wants to merge 1 commit into
RedisAI:mainfrom
plusky:guard-svs-include-in-tiered-factory-header

Conversation

@plusky

@plusky plusky commented Jul 30, 2026

Copy link
Copy Markdown

cmake/svs.cmake offers USE_SVS as a real option (cmake_dependent_option(USE_SVS "Build with SVS library support" ON "SVS_SUPPORTED" OFF)), but building with -DUSE_SVS=OFF does not compile.

With SVS off, HAVE_SVS is 0 and the ScalableVectorSearch sources are never fetched — but two translation units still reach for SVS unconditionally:

src/VecSim/algorithms/svs/svs.h:21:10: fatal error: svs/index/vamana/dynamic_index.h: No such file or directory
    (via tiered_factory.h -> svs_tiered.h -> svs.h)

src/VecSim/algorithms/svs/svs_utils.h:16:10: fatal error: svs/core/distance.h: No such file or directory
    (via vec_sim.cpp)

tiered_factory.h needs nothing from svs_tiered.h — none of its declarations mention an SVS type — so that include is simply guarded, the way tiered_factory.cpp and svs_factory.cpp already guard the same headers.

vec_sim.cpp does use VecSimSVSThreadPool, in two functions that are part of the public C API and so have to keep existing either way:

  • VecSim_UpdateThreadPoolSize() sets the write mode and then resizes the shared SVS pool. With no SVS there is no pool, so setting the write mode is all the call has to do.
  • VecSim_GetSharedMemory() reports the process-wide shared allocation. That is an SVS construct; without it nothing is held outside the individual indexes, which already report their own, so it returns 0.

No behaviour change when SVS is on — every added branch is under #if HAVE_SVS.

Why this matters outside your CI

I hit it packaging Redis 8.10.0 for openSUSE. Distribution builds have no network access at build time, and USE_SVS=ON wants eve, fmt, spdlog, tomlplusplus and robin-map cloned from git during cmake configure — plus the SVS_SHARED_LIB path, which downloads a pre-compiled shared library. -DUSE_SVS=OFF is exactly the escape hatch for that case, and these guards are all that stand between it and a working build. VectorSimilarity then uses its own HNSW and flat indexes, a perfectly good default for a distribution package.

Happy to add a USE_SVS=OFF configuration to CI in a follow-up so this stays fixed.

@CLAassistant

CLAassistant commented Jul 30, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@plusky
plusky force-pushed the guard-svs-include-in-tiered-factory-header branch from 2d18483 to 8d545f8 Compare July 30, 2026 12:54
@plusky plusky changed the title Guard the SVS include in tiered_factory.h so USE_SVS=OFF builds Guard the SVS includes so USE_SVS=OFF builds Jul 30, 2026
Building with -DUSE_SVS=OFF does not compile. HAVE_SVS is then 0 and the
ScalableVectorSearch sources are never fetched, but two translation units still
reach for SVS unconditionally:

  src/VecSim/algorithms/svs/svs.h:21:10: fatal error:
      svs/index/vamana/dynamic_index.h: No such file or directory
    (via tiered_factory.h -> svs_tiered.h -> svs.h)

  src/VecSim/algorithms/svs/svs_utils.h:16:10: fatal error:
      svs/core/distance.h: No such file or directory
    (via vec_sim.cpp)

tiered_factory.h needs nothing from svs_tiered.h -- none of its declarations
mention an SVS type -- so that include is simply guarded, the way
tiered_factory.cpp and svs_factory.cpp already guard the same headers.

vec_sim.cpp does use VecSimSVSThreadPool, in two functions that are part of the
public C API and so have to keep existing either way:

  - VecSim_UpdateThreadPoolSize() sets the write mode and then resizes the
    shared SVS pool. With no SVS there is no pool, and setting the write mode is
    all the call has to do.
  - VecSim_GetSharedMemory() reports the process-wide shared allocation. That is
    an SVS construct; without it nothing is held outside the individual indexes,
    which already report their own, so it returns 0.
@plusky
plusky force-pushed the guard-svs-include-in-tiered-factory-header branch from 8d545f8 to cbfed38 Compare July 30, 2026 13:05
@plusky plusky changed the title Guard the SVS includes so USE_SVS=OFF builds Guard the SVS code paths so USE_SVS=OFF builds Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants