Improvements/v1.7 security memory performance#89
Merged
Conversation
Minor cosmetic fix to update comment that still referenced version 1.6. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
HIGH PRIORITY FIXES: - Add explicit ResourceOwner cleanup on normal worker exit - Fix integer overflow in timestamp diff with LONG_MAX cap - Fix race condition in list_v2() by snapshotting PIDs before iteration MEDIUM PRIORITY FIXES: - Add PGBG_QUEUE_SIZE_MAX (256MB) to prevent DSM exhaustion - UTF-8 aware string truncation using pg_mbcliplen() - Add PGBG_TIMEOUT_MS_MAX (24h) bounds checking - Consolidate memory context switches in launch_internal LOW PRIORITY POLISH: - Add golden ratio constant documentation - Document pid_t vs int32 type handling - Update Windows header with all v2 API functions NEW FEATURES: - GUC: pg_background.max_workers (default 16, limit concurrent workers) - GUC: pg_background.default_queue_size (default 64KB) - GUC: pg_background.worker_timeout (default 0 = no limit) - pg_background_stats_v2(): Session statistics (launched/completed/failed/active) - pg_background_progress(pct, msg): Worker progress reporting - pg_background_get_progress_v2(pid, cookie): Get worker progress SQL FILES: - pg_background--1.8.sql: Fresh install - pg_background--1.7--1.8.sql: Upgrade from 1.7 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Update version badge to 1.8 - Add v1.8 enhancements to Key Features section - Add GUC configuration documentation (max_workers, worker_timeout, default_queue_size) - Add new API functions to reference (stats_v2, progress, get_progress_v2) - Add new composite types (pg_background_stats, pg_background_progress) - Add usage examples for statistics and progress reporting - Update migration guide with v1.7->1.8 and v1.6->1.7 paths - Update PostgreSQL compatibility table (minimum version 14) - Update version info footer (v1.8, 2025-02-12) - Update per-session worker limits documentation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Change license from GPL-3.0 to PostgreSQL License - Remove Support section (community, professional support) - Remove Acknowledgments section - Keep only Author information - Update Table of Contents Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
New test coverage: - GUC settings: max_workers, default_queue_size, worker_timeout - pg_background_stats_v2(): Session statistics verification - pg_background_progress(): Worker progress reporting - pg_background_get_progress_v2(): Progress retrieval - max_workers enforcement: Verify limit prevents excess workers Tests verify: - GUC defaults and SET/RESET behavior - Stats track launched/completed/failed/active workers - Progress can be reported from worker and read by launcher - Worker limit correctly rejects new workers when at capacity Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ory ordering - Add 'canceled' flag to worker_info struct to track canceled workers separately - Add workers_canceled counter to pg_background_stats_v2() output - Apply pg_background.default_queue_size GUC when queue_size parameter is 0 - Change SQL function defaults from 65536 to 0 to respect GUC setting - Fix DSM segment leak when RegisterDynamicBackgroundWorker() fails - Add pg_write_barrier() in progress reporting for proper memory ordering - Add pg_read_barrier() in progress retrieval for consistent reads - Use pg_mbcliplen() for UTF-8 aware progress message truncation - Update function comments to reflect new default queue_size behavior - Update regression tests for new stats columns Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add #if PG_VERSION_NUM >= 150000 guard for MarkGUCPrefixReserved() (function was added in PostgreSQL 15) - Add PGDLLEXPORT prototype for _PG_init() to fix missing-prototypes warning - Replace strlcpy with memcpy in pg_background_get_progress_v2() to avoid volatile qualifier warning (memory is synchronized after pg_read_barrier) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove --load-extension from Makefile (test SQL creates extension itself) - Update expected output: 65536 -> 64kB for default_queue_size GUC display - Add test-local.sh for running regression tests locally via Docker Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Update final stats to show correct counts: 14 launched, 9 completed, 0 failed, 5 canceled (was 8/3) - Add trailing spaces to header lines in pg_background_1.out to match psql output format differences across PostgreSQL versions Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
CI workflow improvements: - Add concurrency control to cancel in-progress runs on same branch - Add APT package caching for faster builds - Add job timeouts (15/10/20 min) to prevent runaway jobs - Add test-summary job to aggregate matrix results - Extract clang/llvm symlink setup into dedicated step - Update branch triggers to include improvements/* and v1.* patterns - Add tag triggers for releases (v*) - Limit security scan to main branches and PRs - Add explicit permissions for CodeQL - Use DEFAULT_PG_VERSION env var for lint/security jobs - Add retention-days for artifacts - Improve error handling with ::error:: annotations Documentation updates: - Add Quick Start section referencing test-local.sh - Document all v1.8 extension features tested - Add job summary table with timeouts - Add ASCII diagram of build/test flow - Update environment variables table - Streamline troubleshooting section - Remove duplicate test script (reference actual test-local.sh) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add PGDG repository setup to lint job (postgresql-server-dev-17 not available in default Ubuntu repos) - Fix pg_background_1.out expected output: change INSERT to INSERT 0 1 to match actual PostgreSQL command tag format Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Update expected output to handle psql formatting differences: - Add trailing spaces to 'result' column header - Add extra trailing space to 'progress_status' header - Add space character to empty result value lines (void functions) These formatting differences occur between psql versions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The security job also needs the PGDG repository to install postgresql-server-dev-17 (not available in default Ubuntu repos). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
GitHub Actions runners have pre-configured Microsoft Azure CLI repos that sometimes return 403 Forbidden errors. Remove these repo files before running apt-get update to avoid CI failures. Applied to all three jobs: test, lint, and security. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove branch restriction so CodeQL runs on all branches, enabling proper security vulnerability tracking and baseline comparison for pull requests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request upgrades pg_background from version 1.7 to 1.8, introducing significant improvements in security, memory management, and performance monitoring capabilities. The extension enables running SQL commands in background worker processes with asynchronous execution and result retrieval via shared memory queues.
Changes:
- Added comprehensive session statistics tracking (
pg_background_stats_v2()) with workers_canceled counter - Implemented progress reporting API for long-running workers with UTF-8 aware truncation and memory barriers
- Introduced three new GUC parameters:
max_workers,worker_timeout, anddefault_queue_sizefor resource control - Fixed DSM segment resource leak on worker registration failure
- Enhanced robustness with bounds checking, overflow protection, and race condition fix in
list_v2()
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pg_background.c | Core implementation: GUC system, statistics tracking, progress reporting with memory barriers, DSM leak fix, UTF-8 truncation, worker timeout logic |
| windows/pg_background_win.h | Added PGDLLEXPORT declarations for new v1.8 functions (stats_v2, progress, get_progress_v2) |
| pg_background.control | Updated default_version from 1.7 to 1.8 |
| pg_background--1.8.sql | Full extension definition for fresh installations with new types and functions |
| pg_background--1.7--1.8.sql | Upgrade script adding new types, functions, and privilege grants |
| sql/pg_background.sql | Added regression tests for GUC settings, statistics, progress reporting, max_workers enforcement |
| expected/pg_background*.out | Expected test output showing new functionality working correctly |
| test-local.sh | New Docker-based local testing script supporting PostgreSQL 14-18 |
| README.md | Updated documentation with v1.8 features, GUC configuration, progress reporting examples |
| CI.md | Enhanced CI documentation with local testing instructions |
| Makefile | Added 1.8 SQL files, removed --load-extension option |
| .github/workflows/ci.yml | Improved CI with concurrency control, APT caching, better error handling |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1. pgbg_timestamp_diff_ms: Check diff_us before division to avoid potential overflow when comparing against LONG_MAX. Changed from (diff_us / 1000 > LONG_MAX) to (diff_us > (int64) LONG_MAX * 1000) 2. shm_mq_minimum_size cast: Add StaticAssertStmt to verify at compile time that shm_mq_minimum_size fits in an int for the GUC definition 3. total_execution_us: Add overflow protection when accumulating execution times to prevent wraparound on extremely long sessions Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1. shm_mq_minimum_size: Change from StaticAssertStmt to runtime Assert since shm_mq_minimum_size is not a compile-time constant 2. pgbg_timestamp_diff_ms: Revert to original overflow check using division (diff_us / 1000 > LONG_MAX). The multiplication approach overflows on 64-bit systems where LONG_MAX is already INT64_MAX. Division cannot overflow and the comparison works correctly because LONG_MAX is promoted to int64. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.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
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.
Summary of changes pushed: