Skip to content

Documentation update - #104

Merged
klmckeig merged 4 commits into
mainfrom
documentation-update-pre-release
Oct 28, 2026
Merged

klmckeig merged 4 commits into
mainfrom
documentation-update-pre-release

Conversation

@klmckeig

@klmckeig klmckeig commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Description

Documentation pass for the first release (v0.1.0), plus two message strings and one comment changed in src/. No new code and no behavior changes.

  • README:
    • Removed the AVX-512 feature bullet and the CMake row. SVS is prebuilt now.
    • The Building section links to the build guide, and Quick Start points to the required server settings.
    • Running Tests is restructured: build and install, start the server, installcheck, prove_installcheck, then a hardware dependent section (installcheck-hw, prove_installcheck-hw). The commands use SVS_INSTALL and PG_CONFIG.
    • CPU and platform wording: x86-64 only, compression only on Intel.
    • Added a Known Limitations link.
  • Build guide: Removed the CMake requirement and fixed the SVS step, which downloads a prebuilt release. Added "Required PostgreSQL settings": shared_preload_libraries, wal_level = logical, sender and slot settings, CREATE EXTENSION vector, and database enrollment.
  • User guide:
    • Removed the prototype disclaimer, the "Upgrading" note, version roadmap text and the unsupported performance estimates (replaced with a link to the wiki).
    • Fixed the CREATE EXTENSION prerequisite.
    • Numbered and prefixed the troubleshooting entries (9.1 to 9.17), with a Cause then Fix structure, and extended the TOC.
    • Converted the stacked notes in 4.1 to paragraphs.
    • Corrected statements against the code: memory savings table, search_window_size reloption, TRUNCATE behavior, restart and pause behavior, error messages, parameter ranges and others. The checkpoint trigger rows now say "Simple" instead of "Legacy".
  • Architecture doc:
    • Fixed the TOC anchors.
    • Added 8.4 Known Limitations, including "searches wait during writes", and a memory note in 8.3.
    • Corrected sections 4 to 9 against the code: the worker creates the replication slot, the save path, undo log location, VACUUM and checkpoint flows, standby behavior, and GUC tables.
  • BGW_LAUNCHER.md: Removed history wording about earlier designs.
  • Other: Removed the duplicate security.md, which differed from SECURITY.md only in link text and collided on case insensitive filesystems. Updated a stale comment in ci/build_and_test.sh.
  • src/ (message strings and a comment only):
    • The search_window_size reloption description now says it is not used and svs.search_window_size governs.
    • The build failure hint no longer suggests maintenance_work_mem, which has no effect on builds. It now points to the server log and svs.max_build_memory.
    • The comment above VamanaResolveSearchWindowSize in vamana.h now says the reloption is not currently used.

Related Issues

None.

Type of Change

  • Bug fix
  • New feature
  • Refactor / code cleanup
  • Documentation update
  • Test addition or update
  • Build / CI change

Pre-Merge Checklist

Build

  • make completes without errors or warnings
  • make install completes successfully

Tests

  • If this PR introduces no new behavior: existing regression tests (make installcheck) and TAP tests (test/t/) pass with no failures
  • If this PR introduces new behavior: test cases covering it were added to test/sql/ and/or test/t/
  • If this PR adds a standalone unit-test module under test/modules/: it builds and passes (make -C test/modules/<module> installcheck)

Documentation

  • Relevant docs under docs/ updated if architecture or usage changed

Testing Notes

This PR adds no new code and no new tests. The only changes under src/ are two message strings (a reloption description and an error hint) and one comment. No test file references the old text.

  • A script checked every relative link and heading anchor in the README and docs/**/*.md: no broken links.
  • Doc statements were checked against src/, the Makefile and svs.control.
  • The memory savings table (user guide 5.3) was recomputed from its formula, and the formula was checked against the SVS storage layout (a 9 byte header per level, with the LeanVec primary at leanvec_dims and the secondary at the full dimension). This is a static check of the layout, not a measured index size.
  • Not done: the build and test suites were not run on this branch, and the Quick Start was not executed.

🤖 Generated with Claude Code

Signed-off-by: klmckeig <kelly.l.mckeighan@intel.com>
@klmckeig
klmckeig requested a review from a team October 27, 2026 21:30
@matt-welch
matt-welch self-requested a review October 27, 2026 21:36
Signed-off-by: klmckeig <kelly.l.mckeighan@intel.com>
@klmckeig
klmckeig force-pushed the documentation-update-pre-release branch from b8f3002 to 6fd0291 Compare October 27, 2026 22:54

@matt-welch matt-welch left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks Kelly, this is a big improvement. I checked the changed statements against the source and most of them are accurate now. Links all resolve, and build plus the full TAP and regression suites (including the hardware tests) pass on this branch. A few things to fix before release:

Setup steps fail at enrollment. vamana_databases only exists in a database where CREATE EXTENSION svs has been run. docs/build_guide/README.md line 86 says to create the extension in each database that hosts indexes, then line 88 says to INSERT into vamana_databases in the launcher database (default postgres), but nothing says to create the extension there. On a fresh cluster that INSERT fails with ERROR: relation "vamana_databases" does not exist. README.md line 38 also says to enroll before the CREATE EXTENSION svs block. Please add CREATE EXTENSION vector; CREATE EXTENSION svs; in the launcher database as an explicit step, and put the README steps in order.

The new hint in src/vamanabuild.c line 848 doesn't show up for real build failures. SVS errors during the build are raised earlier by CheckSVSError in src/svs_wrapper.c as SVS operation failed: dynamic index build, with no hint. The failed to build SVS index path only runs when SVS returns no index without reporting an error, and then the logged error code is 0. I tried two out-of-range alpha builds and both showed the SVS operation failed error with no hint. If we want users to see this hint, it needs to go on the CheckSVSError path for the dynamic build.

Smaller doc corrections:

  • docs/dev/ARCHITECTURE.md line 338: hot_standby_feedback doesn't make the primary keep WAL. It pins the primary's catalog_xmin so the catalog rows the standby's logical slot reads aren't vacuumed (see the comment at src/vamanaworker.c line 1599). WAL retention comes from primary_slot_name.
  • README.md line 64: the all-in-one build doesn't set SVS_INSTALL or PG_CONFIG in the caller's shell (the config variable is SVS_INSTALL_DIR). The PostgreSQL it builds also has no --enable-tap-tests, so make prove_installcheck won't run against it.
  • docs/USER_GUIDE.md lines 1170 and 1174: writes aren't serialized per index. One worker serves every Vamana index in a database, so splitting into partitions in the same database won't spread write load. ARCHITECTURE 8.4 already says this correctly.
  • README.md line 27 and USER_GUIDE lines 67, 104 and 1110 to 1113 say LVQ/LeanVec need an Intel CPU, but the hint users get (src/svs_wrapper.c line 89) says "hardware with the required CPU instructions". Please make the docs and the hint say the same thing.
  • docs/USER_GUIDE.md line 1226 still describes the search_window_size reloption as the "Default query window". Section 4.2 now says it isn't used.
  • docs/USER_GUIDE.md line 662: inserting another database's name into vamana_databases from a non-launcher database succeeds with no error, and the row is ignored. Only enrolling the current database from a non-launcher database is rejected. Suggested wording: "Enrollment must run in the launcher database. Rows written to vamana_databases in any other database have no effect."

Nits:

  • docs/build_guide/README.md lines 78 and 82 list max_wal_senders as required, but nothing in the extension reads it.
  • ARCHITECTURE.md line 457 says the LOAD wait has no fixed timeout. It waits up to svs.worker_startup_timeout_ms.
  • BGW_LAUNCHER.md line 46 says the worker owns "that database's replication slot". Slots are per index.
  • docs/RESOURCE_SIZING.md lines 25 and 26 (not in this PR) still say CREATE EXTENSION svs installs vector automatically.

Signed-off-by: klmckeig <kelly.l.mckeighan@intel.com>

@matt-welch matt-welch left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks Kelly, this addresses everything from my last review. I ran the new setup steps on a fresh cluster with only the build guide's settings and they work, in both the README order and the build guide order. One README problem is left, plus a small doc fix and a few nits.

The Quick Start never creates mydb. README.md lines 41 to 46 enroll mydb from the launcher database, and line 49 then works in mydb, but nothing creates it. Run in order on a fresh cluster, the enroll step fails with ERROR: database "mydb" does not exist. Please add CREATE DATABASE mydb; before the launcher block, and the same before the enroll step in docs/build_guide/README.md (line 96).

docs/USER_GUIDE.md line 169 still says writes are serialized per index. Like the 9.15 fix, it should say per database, since one worker serves every Vamana index in a database.

Nits:

  • README.md line 101 says the hardware tests need "AVX-512 hardware". Since compression needs an Intel CPU (as line 27 says), "an Intel CPU with AVX-512" would be accurate.
  • docs/USER_GUIDE.md lines 1278 and 1279: simple mode turns on for a positive value, not whenever the GUC is set, so 0 also means off. ARCHITECTURE.md lines 726 and 727 already say "positive value".
  • docs/USER_GUIDE.md line 67 still has the "AVX-512 optimized" bullet that was removed from the README.

The two CI failures on this PR come from main, not from these changes. We're fixing them in a separate PR, and nothing here needs to change for them.

Signed-off-by: klmckeig <kelly.l.mckeighan@intel.com>

@matt-welch matt-welch left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks Kelly, this covers everything. I ran the README Quick Start exactly as written on a fresh cluster and it works end to end: it creates and enrolls mydb, builds an index and returns results. The other fixes all check out.

One optional thing for later, not blocking: in docs/build_guide/README.md, line 84 says to create the extensions in each database that hosts indexes, but mydb isn't created until line 96, and the section ends without creating the extensions in mydb. Moving that sentence after the enroll step, as "Then, in mydb, run ...", would make it read in order.

The CI failures here come from main, and we're fixing them separately. Approving.

@klmckeig
klmckeig merged commit 033cd2c into main Oct 28, 2026
7 of 8 checks passed
@klmckeig
klmckeig deleted the documentation-update-pre-release branch October 28, 2026 21:08
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