Conversation
github-actions
Bot
force-pushed
the
master
branch
from
14db4ac to
bad9884
Compare
github-actions
Bot
force-pushed
the
master
branch
4 times, most recently
from
b481005 to
ac73e0f
Compare
github-actions
Bot
force-pushed
the
master
branch
2 times, most recently
from
3b1d505 to
7dda555
Compare
github-actions
Bot
force-pushed
the
master
branch
2 times, most recently
from
44349f6 to
7e77e41
Compare
- Hourly upstream sync from postgres/postgres (24x daily) - AI-powered PR reviews using AWS Bedrock Claude Sonnet 4.5 - Multi-platform CI via existing Cirrus CI configuration - Cost tracking and comprehensive documentation Features: - Automatic issue creation on sync conflicts - PostgreSQL-specific code review prompts (C, SQL, docs, build) - Cost limits: $15/PR, $200/month - Inline PR comments with security/performance labels - Skip draft PRs to save costs Documentation: - .github/SETUP_SUMMARY.md - Quick setup overview - .github/QUICKSTART.md - 15-minute setup guide - .github/PRE_COMMIT_CHECKLIST.md - Verification checklist - .github/docs/ - Detailed guides for sync, AI review, Bedrock See .github/README.md for complete overview Complete Phase 3: Windows builds + fix sync for CI/CD commits Phase 3: Windows Dependency Build System - Implement full build workflow (OpenSSL, zlib, libxml2) - Smart caching by version hash (80% cost reduction) - Dependency bundling with manifest generation - Weekly auto-refresh + manual triggers - PowerShell download helper script - Comprehensive usage documentation Sync Workflow Fix: - Allow .github/ commits (CI/CD config) on master - Detect and reject code commits outside .github/ - Merge upstream while preserving .github/ changes - Create issues only for actual pristine violations Documentation: - Complete Windows build usage guide - Update all status docs to 100% complete - Phase 3 completion summary All three CI/CD phases complete (100%): ✅ Hourly upstream sync with .github/ preservation ✅ AI-powered PR reviews via Bedrock Claude 4.5 ✅ Windows dependency builds with smart caching Cost: $40-60/month total See .github/PHASE3_COMPLETE.md for details Fix sync to allow 'dev setup' commits on master The sync workflow was failing because the 'dev setup v19' commit modifies files outside .github/. Updated workflows to recognize commits with messages starting with 'dev setup' as allowed on master. Changes: - Detect 'dev setup' commits by message pattern (case-insensitive) - Allow merge if commits are .github/ OR dev setup OR both - Update merge messages to reflect preserved changes - Document pristine master policy with examples This allows personal development environment commits (IDE configs, debugging tools, shell aliases, Nix configs, etc.) on master without violating the pristine mirror policy. Future dev environment updates should start with 'dev setup' in the commit message to be automatically recognized and preserved. See .github/docs/pristine-master-policy.md for complete policy See .github/DEV_SETUP_FIX.md for fix summary Optimize CI/CD costs by skipping builds for pristine commits Add cost optimization to Windows dependency builds to avoid expensive builds when only pristine commits are pushed (dev setup commits or .github/ configuration changes). Changes: - Add check-changes job to detect pristine-only pushes - Skip Windows builds when all commits are dev setup or .github/ only - Add comprehensive cost optimization documentation - Update README with cost savings (~40% reduction) Expected savings: ~$3-5/month on Windows builds, ~$40-47/month total through combined optimizations. Manual dispatch and scheduled builds always run regardless.
Review every PR (including drafts) with two jobs that authenticate to AWS Bedrock (Claude Opus 4.8) via GitHub OIDC (vars.AWS_ROLE_ARN); no static AWS credentials are stored in the repo. - ocr-review: runs Alibaba Open Code Review through an ephemeral LiteLLM proxy bridging OCR's OpenAI protocol to Bedrock, and posts inline review comments. Uses output_config.effort=xhigh (Opus 4.8 adaptive thinking). Path-scoped rules (.github/ocr/rule.json) encode PostgreSQL community review standards plus reviewer discipline (verify against the diff, don't hallucinate, state confidence, be blunt, accuracy over approval). - pg-history: OCR cannot call MCP, so a separate Bedrock tool-use agent (.github/ocr/pg-history.py) queries the Agora MCP server (pg.ddx.io) to tie the change to git + pgsql-hackers history, and upserts a comment linking threads as https://pg.ddx.io/m/pgsql-hackers/<message-id>.
The pg-history workflow job has been failing every run with 'Bedrock call failed: The read operation timed out' -- botocore's default 60s read timeout on bedrock-runtime is too short for a multi-round (MAX_ROUNDS=14) tool-use loop against a large PR diff on a reasoning-capable model; a single converse() call alone can take several minutes under load (the sibling ocr-review job's own LLM pass over a similarly large diff took 30-40 minutes). Confirmed via two consecutive live runs against PR #26. Set read_timeout=900s (15 min) explicitly via botocore.config.Config; leave connect_timeout short since a stuck TCP handshake is a different, cheaper-to-detect failure mode that shouldn't wait as long.
github-actions
Bot
force-pushed
the
master
branch
from
7e77e41 to
f9645af
Compare
StrategyGetBuffer() advances the shared clock hand, nextVictimBuffer, with a pg_atomic_fetch_add_u32(..., 1) on every tick. On a multi-socket system the cache line holding that counter has to travel over the interconnect on each operation, pushing a sweep tick from ~20ns (same-socket, line warm in L1/L2) into the ~100-200ns range. Under eviction pressure with hundreds of backends in StrategyGetBuffer() concurrently, that single cache line becomes the dominant cost of the sweep, visible as elevated bus-cycles and cache-misses in a perf profile. Have each backend claim a run of consecutive buffer IDs from the shared hand with a single fetch-add and then iterate through them privately. The sweep still advances through the pool in order, each buffer is still visited exactly once per complete pass, and the meaning of the clock state is unchanged; only the temporal ordering of visits within a pass changes, which the algorithm does not depend on. The contended atomic now fires roughly once per batch rather than once per buffer. The batch is one cache line's worth of clock-hand values -- PG_CACHE_LINE_SIZE / sizeof(uint32) -- capped at NBuffers so a claim can never wrap the pool more than once. Batching only helps when the counter's cache line actually bounces between sockets, so it is enabled only on multi-node NUMA hardware (pg_numa_get_max_node() >= 1); on a single socket, or where libnuma is unavailable, the batch size stays 1 and the code path is byte-identical to the stock clock sweep. Wraparound handling is adjusted: with batching, several backends can each see a fetch-add return a value past NBuffers within the same pass. Any such backend takes buffer_strategy_lock, re-reads the counter, and if it is still out of range wraps it with a single CAS and increments completePasses. StrategySyncStart() continues to see a consistent (nextVictimBuffer, completePasses) pair. This is the batched-clock-sweep idea from Jim Mlodgenski's pgsql-hackers thread, adapted to derive the batch size from the platform cache-line size. Co-authored-by: Jim Mlodgenski <mlodj@amazon.com>
Replace the 0..5 usage_count buffer-replacement policy with a cooling-stage
clock (the LeanStore / 2Q-A1 model): a buffer is simply HOT (recently used)
or COOL (an eviction candidate), with "pinned" being the existing refcount.
There is no per-buffer access counter.
- A demand-loaded page is admitted COOL (probationary), not HOT. A second
access via PinBuffer promotes it COOL -> HOT (the rescue). So a page
touched once -- a sequential scan -- fills and drains the COOL stage and
is evicted from it without ever displacing the HOT working set. Scan
resistance is intrinsic to the replacement algorithm, which is what lets a
later commit remove the BufferAccessStrategy ring buffers entirely.
- The foreground sweep in StrategyGetBuffer() reclaims an already-COOL,
unpinned buffer, pinning it with a CAS so a racing PinBuffer always wins.
Promotion (COOL -> HOT) and demotion (HOT -> COOL) are single-bit
transitions; only the eviction claim is a CAS.
- The background writer maintains the supply of COOL victims. As its LRU
scan runs ahead of the clock hand it demotes HOT buffers to COOL, so the
foreground finds a victim in a single pass rather than having to cool
buffers itself. The demotion is demand-driven -- bounded by the predicted
allocation for the next cycle -- so it stages just enough COOL buffers
without cooling the whole pool, and it is done under the buffer header
lock the scan already holds. A single second-chance reference bit
(set by PinBuffer, cleared on the bgwriter's first pass over a HOT buffer)
spares a recently-accessed buffer one cooling pass, keeping the genuinely
hot set out of the COOL stage under scan pressure. BgBufferSync also
tracks the reusable-buffer density it directly observes on a shorter
smoothing window, so a burst of probationary/scan COOL pages is followed
promptly rather than averaged away.
The 4-bit usage_count field is reinterpreted in place: bit 0 is the HOT/COOL
state (BUF_COOLSTATE_ONE), bit 1 the reference bit (BUF_REFBIT). The 64-bit
buffer-state layout -- refcount, flag and lock offsets and their StaticAsserts
-- is unchanged; only the meaning of the field and the instructions that touch
it change. A StaticAssert requires the field to be at least 2 bits wide so a
future width change cannot push the reference bit into the flag bits.
BM_MAX_USAGE_COUNT becomes BUF_COOLSTATE_HOT (1), so the pin fast path
saturates at HOT. Local (temp-table) buffers get the same two-state
treatment; being single-backend they need no background cooler.
Because the reference bit shares the field, the full 4-bit value can be 0..3;
readers that mean "the cooling state" must use BUF_STATE_GET_COOLSTATE(), which
masks to bit 0 and returns only 0 (COOL) or 1 (HOT), never the raw field.
contrib/pg_buffercache is updated accordingly: its usagecount column and the
pg_buffercache_summary average report the cooling state (0 = COOL, 1 = HOT),
and pg_buffercache_usage_counts() buckets on it. Using the raw 4-bit getter
there would index its BM_MAX_USAGE_COUNT+1 = 2-element arrays with values up
to 3 and overrun the stack; masking to the cooling bit keeps the index in
range. The reference bit is deliberately not exposed as usagecount.
Depends on the batched clock sweep from the previous commit.
The cooling-stage evictor admits demand-loaded pages COOL and promotes them to
HOT only on a second access, so a one-touch sequential scan fills and drains
the COOL stage without displacing the hot working set. Scan resistance is
therefore a property of the replacement algorithm itself, and the
BufferAccessStrategy ring buffers that previously provided it are dead weight.
Remove them end to end.
Deleted:
- the BufferAccessStrategy type and the BufferAccessStrategyType enum
(BAS_NORMAL/BULKREAD/BULKWRITE/VACUUM);
- the ring machinery in freelist.c (GetAccessStrategy[WithSize],
GetAccessStrategyBufferCount, GetAccessStrategyPinLimit,
FreeAccessStrategy, GetBufferFromRing, AddBufferToRing,
StrategyRejectBuffer, IOContextForStrategy);
- the strategy parameter from ReadBufferExtended, ReadBufferWithoutRelcache,
the ExtendBufferedRel* family, StrategyGetBuffer, read_stream_begin_*,
and every scan/vacuum/analyze/index-AM caller;
- the strategy fields on HeapScanDescData, IndexScanDescData,
BulkInsertStateData, ReadBuffersOperation, and ReadStream;
- _hash_getbuf_with_strategy (identical to _hash_getbuf without a strategy).
pg_stat_io's per-strategy IO contexts collapse: IOCONTEXT_BULKREAD,
IOCONTEXT_BULKWRITE and IOCONTEXT_VACUUM are removed, leaving IOCONTEXT_INIT
and IOCONTEXT_NORMAL. IOOP_REUSE only ever occurred while recycling a ring
buffer, so it is no longer tracked; GetVictimBuffer counts IOOP_EVICT only.
The vacuum_buffer_usage_limit GUC and the VACUUM/ANALYZE (BUFFER_USAGE_LIMIT
...) option are removed, along with the VacuumBufferUsageLimit global, the
ring-size plumbing through VacuumParams and parallel vacuum, and vacuumdb's
--buffer-usage-limit client option. read_stream's per-backend pin budget is
now enforced solely by GetPinLimit()/GetLocalPinLimit(), which already applied
and is unchanged for the (formerly universal) no-strategy case.
Documentation and the stats/amcheck regression tests are updated to drop the
removed contexts and options.
github-actions
Bot
force-pushed
the
master
branch
from
f9645af to
7ca55a1
Compare
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
1 participant
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.
Cooling-stage clock sweep for the buffer manager — CI check.
This PR is against gburd/postgres:master (my fork), not upstream, purely
to exercise CI on the three-patch series before posting to pgsql-hackers.
Net +510/-1642 over 83 files (almost all deletion is patch 3).
Each commit builds -Werror + cassert + injection_points and passes
regress/regress(245 tests) independently; verified on both the fork baseand a clean upstream/master rebase. See the draft cover letter for the
design reasoning and the m6i / r8i (6-node NUMA) / huge-pages / local-NVMe
real-IO benchmark data.
Not for merge — this is the review/CI vehicle for the -hackers submission.