Repository navigation
Fix svs.max_build_memory initializer mismatch - #57
Merged
Merged
Conversation
svs.max_build_memory's GUC registration sets a boot value of 4096 MB, but the C variable backing it initialized to 100. On a debug-assertion build, PostgreSQL's GUC-consistency check requires the two to match, so any such build crashes on startup as soon as the extension loads, before any GUC even has a chance to take effect. Release builds were never affected: GUC registration unconditionally overwrites the C variable with the boot value at startup, so the stale 100 was dead code there. The regression suite's expected output already asserts 4096 as the default, confirming the change brings the initializer in line with behavior the test suite, the GUC registration, and runtime output already agreed on. - Change the initializer in src/svs_memory.c from 100 to 4096 to match the boot_val already registered for this GUC - No behavior change on a normal build; this only matters for builds with assertions enabled Signed-off-by: Matt Welch <matt.welch@intel.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
svs.max_build_memory's GUC registration (src/vamana.c) sets aboot_valof4096MB, but the C variable backing it (src/svs_memory.c) initialized to100. On a build with--enable-cassert, PostgreSQL's GUC consistency check requires a custom variable's initial value to be either0or exactly equal to its registeredboot_val; since this variable started at100, neither held, and every backend that loadssvscrashed immediately on startup, before any GUC even took effect. Release builds were never affected:DefineCustomIntVariableunconditionally overwrites the C variable withboot_valat registration time regardless of build type, so the stale100was dead code there, never observed by anything at runtime.test/expected/runtime_params.outalready asserts4096as the correct default, and that test already passed before this change, confirming the fix brings the initializer in line with what the GUC registration, the regression suite, and runtime behavior already agreed on. This is a one-line change: the initializer insrc/svs_memory.cmoves from100to4096. No release-build behavior changes.Related Issues
None.
Type of Change
Pre-Merge Checklist
Build
makecompletes without errors or warningsmake installcompletes successfullyTests
make installcheck) and TAP tests (test/t/) pass with no failurestest/sql/and/ortest/t/test/modules/: it builds and passes (make -C test/modules/<module> installcheck)Documentation
docs/updated if architecture or usage changedTesting Notes
Full clean build (
make clean && make -j$(nproc) && make install) completed with no errors. One pre-existing compiler warning appears during the build (ISO C90 forbids mixed declarations and codein the SVS library's ownsvs/c/svs_c.hheader), but it is unrelated to this change, present in a third-party header this PR does not touch, and not newly introduced by this one-line edit. Full TAP suite (make prove_installcheck): 47 files, 1186 tests, all passing, with one expected skip (18_build_offset_overflow.pl, gated behindenable_slow_tests=yesand not run by default). Full SQL regression suite (test_svs_ext_regression.sh): 7 of 7 test files passing, includingruntime_params, which directly asserts thatSHOW svs.max_build_memoryreads4096. Also independently confirmed by reading PostgreSQL's own GUC implementation (src/backend/utils/misc/guc.c) that the consistency check this fixes is compiled out entirely without--enable-cassert, and that GUC registration always sets the backing C variable toboot_valregardless of build type, so this change has no observable effect on a normal (non-assert) build. The other four GUC initializers declared in the same file (vamana_max_residency_memory_mb,vamana_default_residency_memory_mb,vamana_max_search_work_mem_mb,vamana_default_search_work_mem_mb) were checked against their own registeredboot_vals and already match; none needed a change.