-
Notifications
You must be signed in to change notification settings - Fork 1
Cooling-stage clock sweep (batched sweep + HOT/COOL evictor + strategy removal) #34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,43 @@ | ||||||
| Diagnostics: | ||||||
| MissingIncludes: None | ||||||
|
Comment on lines
+1
to
+2
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file should not be committed to the repository. The project's root Additionally, note the paths are developer-specific: |
||||||
| InlayHints: | ||||||
| Enabled: true | ||||||
| ParameterNames: true | ||||||
| DeducedTypes: true | ||||||
| CompileFlags: | ||||||
| CompilationDatabase: build/ # Search build/ directory for compile_commands.json | ||||||
| Remove: [ -Werror ] | ||||||
| Add: | ||||||
| - -DDEBUG | ||||||
| - -DLOCAL | ||||||
| - -DPGDLLIMPORT= | ||||||
| - -DPIC | ||||||
| - -O2 | ||||||
| - -Wall | ||||||
| - -Wcast-function-type | ||||||
| - -Wconversion | ||||||
| - -Wdeclaration-after-statement | ||||||
| - -Wendif-labels | ||||||
| - -Werror=vla | ||||||
| - -Wextra | ||||||
| - -Wfloat-equal | ||||||
| - -Wformat-security | ||||||
| - -Wimplicit-fallthrough=3 | ||||||
| - -Wmissing-format-attribute | ||||||
| - -Wmissing-prototypes | ||||||
| - -Wno-format-truncation | ||||||
| - -Wno-sign-conversion | ||||||
| - -Wno-stringop-truncation | ||||||
| - -Wno-unused-const-variable | ||||||
| - -Wpointer-arith | ||||||
| - -Wshadow | ||||||
| - -Wshadow=compatible-local | ||||||
| - -fPIC | ||||||
| - -fexcess-precision=standard | ||||||
| - -fno-strict-aliasing | ||||||
| - -fvisibility=hidden | ||||||
| - -fwrapv | ||||||
| - -g | ||||||
| - -std=c11 | ||||||
| - -I. | ||||||
| - -I../../../../src/include | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This include path is almost certainly wrong for a
Suggested change
Comment on lines
+42
to
+43
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These |
||||||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,156 @@ | ||||||||||
| # HOT Indexed Updates — GDB breakpoints for code review | ||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This entire file is a personal debugging aid and must be removed from the patch. Committing a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This entire file ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The header (and several other comment lines) use a non-ASCII em-dash character ('—'), which violates the project's ASCII-only rule for source and diffs. Use a plain ASCII hyphen/dash instead.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The em-dash (U+2014) violates the ASCII-only rule for source and diffs. If this file were to be kept at all, use an ASCII hyphen. (More fundamentally, the whole file should not be committed.) Confidence: high.
Suggested change
|
||||||||||
| # | ||||||||||
| # Usage: gdb -x .gdbinit <postgres-binary> | ||||||||||
|
Comment on lines
+1
to
+3
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This Worse, the file is orphaned from the feature it targets: the breakpoints reference a "HOT indexed updates" implementation that is NOT present in this tree. Symbols such as
Comment on lines
+1
to
+3
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This entire file is a personal debugging artifact and must not be committed to the PostgreSQL tree. GDB auto-loads Confidence: high. |
||||||||||
| # Or from gdb: source .gdbinit | ||||||||||
| # | ||||||||||
| # These breakpoints cover the major code paths introduced or modified by | ||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ASCII-only is required in PostgreSQL source and diffs. This comment block uses non-ASCII em-dashes (e.g., "HOT Indexed Updates —", "returns Bitmapset" lines and the "—" separators throughout). Another reason this file cannot be committed as-is. |
||||||||||
| # the HOT indexed updates patch series. They are organized by subsystem | ||||||||||
|
Comment on lines
+6
to
+7
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Topic mismatch: this file's stated purpose is a "HOT Indexed Updates" feature, but that feature is not present in this change set (the actual modified files are buffer-manager code: bufmgr.c, freelist.c, localbuf.c, buf_internals.h, pg_buffercache). A search of the heap access code found none of the symbols referenced below (heap_hot_indexed_create_tuple, heap_xlog_indexed_update, heap_hot_indexed_serialize_bitmap, HEAP_INDEXED_UPDATED). The commit therefore bundles unrelated content and is neither atomic nor bisectable. Confidence: high. |
||||||||||
| # to make it easy to enable/disable groups during debugging. | ||||||||||
| # | ||||||||||
| # Tip: to skip to a specific subsystem, disable all then enable selectively: | ||||||||||
| # disable breakpoints | ||||||||||
| # enable 1 2 3 # just the update-decision group | ||||||||||
|
|
||||||||||
| # ========================================================================= | ||||||||||
| # 1. UPDATE DECISION — heap_update() HOT/HOT-indexed/non-HOT choice | ||||||||||
| # src/backend/access/heap/heapam.c | ||||||||||
| # ========================================================================= | ||||||||||
|
|
||||||||||
| # Main entry: heap_update | ||||||||||
| break heapam.c:3210 | ||||||||||
|
Comment on lines
+19
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hardcoded file:line breakpoints are inaccurate and brittle. For example, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Breakpoints anchored to hardcoded source line numbers (heapam.c:3210/4019/4024/4033/4101/4147, heapam_indexscan.c:182/250/297, indexam.c:299, execIndexing.c:370, pruneheap.c:1287/1802/1836/1863/2936) are inherently fragile: they silently drift as surrounding code changes and will land on the wrong statements. Since this file should not be committed at all (see above), this is moot, but even as a private aid, function-name breakpoints are far more robust than line-number ones.
Comment on lines
+19
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Line-number breakpoints are inherently fragile: they silently drift the moment any adjacent code changes. In fact
Comment on lines
+19
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hardcoded file:line breakpoints are extremely fragile: they silently point at the wrong statement (or fail to resolve) after any edit to these files. Worse, these specific lines/symbols do not exist in the current tree - the referenced HOT-indexed functions and flag (heap_hot_indexed_*, heap_xlog_indexed_update, HEAP_INDEXED_UPDATED) were not found in src/backend/access/heap. These breakpoints reference code outside this change and are stale/meaningless here. Confidence: high. |
||||||||||
|
|
||||||||||
| # HOT decision block: pure HOT vs HOT indexed vs non-HOT | ||||||||||
| # Line 4019: pure HOT (no indexed columns changed) | ||||||||||
| # Line 4024: HOT indexed path (non-catalog, some indexed columns changed) | ||||||||||
| # Line 4031: predict augmented tuple size | ||||||||||
| # Line 4033: size+space check before creating augmented tuple | ||||||||||
| break heapam.c:4019 | ||||||||||
| break heapam.c:4024 | ||||||||||
| break heapam.c:4033 | ||||||||||
|
|
||||||||||
| # Set HEAP_INDEXED_UPDATED flag on new tuple before page insertion | ||||||||||
| break heapam.c:4101 | ||||||||||
|
|
||||||||||
| # Restore HEAP_INDEXED_UPDATED on old tuple (only if it previously had it) | ||||||||||
| break heapam.c:4147 | ||||||||||
|
|
||||||||||
| # ========================================================================= | ||||||||||
| # 2. TUPLE CREATION — building the augmented tuple with embedded bitmap | ||||||||||
| # src/backend/access/heap/heapam.c | ||||||||||
| # ========================================================================= | ||||||||||
|
|
||||||||||
| # Predict augmented tuple size (returns 0 if t_hoff would overflow) | ||||||||||
| break heap_hot_indexed_tuple_size | ||||||||||
|
|
||||||||||
| # Create augmented tuple with embedded modified-column bitmap | ||||||||||
| break heap_hot_indexed_create_tuple | ||||||||||
|
Comment on lines
+45
to
+46
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These breakpoints reference symbols that do not exist anywhere in this change: |
||||||||||
|
|
||||||||||
| # Serialize Bitmapset into raw bytes in tuple header | ||||||||||
| break heap_hot_indexed_serialize_bitmap | ||||||||||
|
|
||||||||||
| # ========================================================================= | ||||||||||
| # 3. BITMAP UTILITIES — raw bitmap operations for chain following | ||||||||||
| # src/backend/access/heap/heapam.c | ||||||||||
| # ========================================================================= | ||||||||||
|
|
||||||||||
| # Compute raw bitmap byte size from natts | ||||||||||
| break heap_hot_indexed_bitmap_raw_size | ||||||||||
|
|
||||||||||
| # Check if tuple header has room for bitmap between null bitmap and data | ||||||||||
| break heap_hot_indexed_has_bitmap_space | ||||||||||
|
|
||||||||||
| # Read HOT indexed bitmap from tuple header (returns Bitmapset) | ||||||||||
| break heap_hot_indexed_read_bitmap | ||||||||||
|
|
||||||||||
| # Fast overlap check: does tuple's raw bitmap overlap with indexed_attrs? | ||||||||||
| break heap_hot_indexed_bitmap_overlaps_raw | ||||||||||
|
|
||||||||||
| # OR a tuple's raw bitmap into an accumulator buffer | ||||||||||
| break heap_hot_indexed_bitmap_or_raw | ||||||||||
|
|
||||||||||
| # Check if accumulated raw bitmap overlaps with indexed_attrs | ||||||||||
| break heap_hot_indexed_accum_overlaps | ||||||||||
|
|
||||||||||
| # Merge bitmaps from dead tuples into a target tuple on the page | ||||||||||
| break heap_hot_indexed_merge_bitmaps_raw | ||||||||||
|
|
||||||||||
| # Deserialize raw bytes back to Bitmapset | ||||||||||
| break heap_hot_indexed_deserialize_bitmap | ||||||||||
|
|
||||||||||
| # ========================================================================= | ||||||||||
| # 4. INDEX SCAN — HOT chain following with stale-entry detection | ||||||||||
| # src/backend/access/heap/heapam_indexscan.c | ||||||||||
| # ========================================================================= | ||||||||||
|
|
||||||||||
| # Main HOT chain search with indexed update awareness | ||||||||||
| break heap_hot_search_buffer | ||||||||||
|
|
||||||||||
| # Redirect-with-data: initialize bitmap accumulator from collapsed redirect | ||||||||||
| break heapam_indexscan.c:182 | ||||||||||
|
|
||||||||||
| # Accumulate bitmap from INDEXED_UPDATED tuple in chain | ||||||||||
| break heapam_indexscan.c:250 | ||||||||||
|
|
||||||||||
| # Stale entry detection: accumulated bitmap overlaps this index's attrs | ||||||||||
| break heapam_indexscan.c:297 | ||||||||||
|
|
||||||||||
| # ========================================================================= | ||||||||||
| # 5. INDEX SCAN SETUP — indexed_attrs bitmap computation | ||||||||||
| # src/backend/access/index/indexam.c | ||||||||||
| # ========================================================================= | ||||||||||
|
|
||||||||||
| # Compute indexed_attrs for HOT indexed update chain following | ||||||||||
| break indexam.c:299 | ||||||||||
|
|
||||||||||
| # ========================================================================= | ||||||||||
| # 6. INDEX INSERTION — skip unchanged indexes for HOT indexed updates | ||||||||||
| # src/backend/executor/execIndexing.c | ||||||||||
| # ========================================================================= | ||||||||||
|
|
||||||||||
| # Entry: insert/update index tuples | ||||||||||
| break ExecInsertIndexTuples | ||||||||||
|
|
||||||||||
| # Index skip decision: skip indexes whose attrs don't overlap modified set | ||||||||||
| break execIndexing.c:370 | ||||||||||
|
|
||||||||||
| # ========================================================================= | ||||||||||
| # 7. PRUNING — chain collapsing and redirect-with-data | ||||||||||
| # src/backend/access/heap/pruneheap.c | ||||||||||
| # ========================================================================= | ||||||||||
|
|
||||||||||
| # Main prune function | ||||||||||
| break heap_page_prune_and_freeze | ||||||||||
|
|
||||||||||
| # Per-chain pruning entry | ||||||||||
| break heap_prune_chain | ||||||||||
|
|
||||||||||
| # Chain collapsing: collect bitmaps from dead INDEXED_UPDATED intermediates | ||||||||||
| break pruneheap.c:1802 | ||||||||||
|
|
||||||||||
| # OR dead tuple bitmaps into combined bitmap | ||||||||||
| break pruneheap.c:1836 | ||||||||||
|
|
||||||||||
| # Record redirect-with-data for execute phase | ||||||||||
| break pruneheap.c:1863 | ||||||||||
|
|
||||||||||
| # Execute phase: apply redirect-with-data entries on the page | ||||||||||
| break pruneheap.c:1287 | ||||||||||
|
|
||||||||||
| # ========================================================================= | ||||||||||
| # 8. WAL REPLAY — recovery of HOT indexed updates | ||||||||||
| # src/backend/access/heap/heapam_xlog.c | ||||||||||
| # ========================================================================= | ||||||||||
|
|
||||||||||
| # WAL replay for XLOG_HEAP2_INDEXED_UPDATE | ||||||||||
| break heap_xlog_indexed_update | ||||||||||
|
|
||||||||||
| # ========================================================================= | ||||||||||
| # 9. WAL LOGGING — writing HOT indexed update records | ||||||||||
| # src/backend/access/heap/heapam.c | ||||||||||
| # ========================================================================= | ||||||||||
|
|
||||||||||
| # WAL logging for heap updates (handles indexed_update flag) | ||||||||||
| break log_heap_update | ||||||||||
|
|
||||||||||
| # Serialize redirect-with-data into WAL record (pruneheap.c) | ||||||||||
| break pruneheap.c:2936 | ||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -161,7 +161,7 @@ pg_buffercache_pages(PG_FUNCTION_ARGS) | |
| reldatabase = bufHdr->tag.dbOid; | ||
| forknum = BufTagGetForkNum(&bufHdr->tag); | ||
| blocknum = bufHdr->tag.blockNum; | ||
| usagecount = BUF_STATE_GET_USAGECOUNT(buf_state); | ||
| usagecount = BUF_STATE_GET_COOLSTATE(buf_state); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This silently changes the user-visible semantics of the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change alters the user-visible semantics of the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This changes user-visible semantics: the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This silently changes the SQL-visible semantics of pg_buffercache. BUF_STATE_GET_COOLSTATE returns only 0 or 1 (bit 0 of the field: COOL/HOT), whereas the replaced BUF_STATE_GET_USAGECOUNT returned the full 0..N usage count. So the exposed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This silently repurposes the user-visible There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a user-visible SQL behavior change that is not documented in this patch. The |
||
| pinning_backends = BUF_STATE_GET_REFCOUNT(buf_state); | ||
|
|
||
| if (buf_state & BM_DIRTY) | ||
|
|
@@ -605,7 +605,7 @@ pg_buffercache_summary(PG_FUNCTION_ARGS) | |
| if (buf_state & BM_VALID) | ||
| { | ||
| buffers_used++; | ||
| usagecount_total += BUF_STATE_GET_USAGECOUNT(buf_state); | ||
| usagecount_total += BUF_STATE_GET_COOLSTATE(buf_state); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. usagecount_avg (usagecount_total / buffers_used) now averages a 0/1 HOT/COOL bit instead of the historic 0..BM_MAX_USAGE_COUNT usage count, so the reported average collapses to [0,1]. This is a user-visible change to pg_buffercache_summary()'s output and its documented meaning; update the docs ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same user-visible contract break here: |
||
|
|
||
| if (buf_state & BM_DIRTY) | ||
| buffers_dirty++; | ||
|
|
@@ -655,7 +655,7 @@ pg_buffercache_usage_counts(PG_FUNCTION_ARGS) | |
|
|
||
| CHECK_FOR_INTERRUPTS(); | ||
|
|
||
| usage_count = BUF_STATE_GET_USAGECOUNT(buf_state); | ||
| usage_count = BUF_STATE_GET_COOLSTATE(buf_state); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The local There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| usage_counts[usage_count]++; | ||
|
|
||
| if (buf_state & BM_DIRTY) | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| { | ||
| description = "PostgreSQL development environment"; | ||
|
|
||
| inputs = { | ||
| nixpkgs.url = "github:NixOS/nixpkgs/nixos-25.05"; | ||
| nixpkgs-unstable.url = "github:nixos/nixpkgs/nixpkgs-unstable"; | ||
| flake-utils.url = "github:numtide/flake-utils"; | ||
| }; | ||
|
|
||
| outputs = { | ||
| self, | ||
| nixpkgs, | ||
| nixpkgs-unstable, | ||
| flake-utils, | ||
| }: | ||
| flake-utils.lib.eachDefaultSystem ( | ||
| system: let | ||
| pkgs = import nixpkgs { | ||
| inherit system; | ||
| config.allowUnfree = true; | ||
| }; | ||
| pkgs-unstable = import nixpkgs-unstable { | ||
| inherit system; | ||
| config.allowUnfree = true; | ||
| }; | ||
|
|
||
| shellConfig = import ./shell.nix {inherit pkgs pkgs-unstable system;}; | ||
| in { | ||
| formatter = pkgs.alejandra; | ||
| devShells = { | ||
| default = shellConfig.devShell; | ||
| gcc = shellConfig.devShell; | ||
| clang = shellConfig.clangDevShell; | ||
| gcc-musl = shellConfig.muslDevShell; | ||
| clang-musl = shellConfig.clangMuslDevShell; | ||
| }; | ||
|
|
||
| packages = { | ||
| inherit (shellConfig) gdbConfig flameGraphScript pgbenchScript; | ||
| }; | ||
|
|
||
| environment.localBinInPath = true; | ||
| } | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| From 130c231020f97e5eb878cc9fdb2bd9b186a5aa04 Mon Sep 17 00:00:00 2001 | ||
| From: Greg Burd <greg@burd.me> | ||
| Date: Fri, 24 Oct 2025 11:58:24 -0400 | ||
| Subject: [PATCH] no warnings with -O0 and fortify source please | ||
|
|
||
| --- | ||
| include/features.h | 1 - | ||
| 1 file changed, 1 deletion(-) | ||
|
|
||
| diff --git a/include/features.h b/include/features.h | ||
| index 673c4036..a02c8a3f 100644 | ||
| --- a/include/features.h | ||
| +++ b/include/features.h | ||
| @@ -432,7 +432,6 @@ | ||
|
|
||
| #if defined _FORTIFY_SOURCE && _FORTIFY_SOURCE > 0 | ||
| # if !defined __OPTIMIZE__ || __OPTIMIZE__ <= 0 | ||
| -# warning _FORTIFY_SOURCE requires compiling with optimization (-O) | ||
| # elif !__GNUC_PREREQ (4, 1) | ||
| # warning _FORTIFY_SOURCE requires GCC 4.1 or later | ||
| # elif _FORTIFY_SOURCE > 2 && (__glibc_clang_prereq (9, 0) \ | ||
| -- | ||
| 2.50.1 | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This
.clangdfile is personal editor/LSP tooling configuration and should not be part of a patch destined for pgsql-hackers/commitfest. Per PostgreSQL patch hygiene, a patch must be a minimal diff containing only what the stated change requires; committing local IDE/tooling config (this file, plus.gdbinitandpg-aliases.shin this same change) is unrelated churn and a reliable rejection reason. The PostgreSQL tree does not carry.clangd. If you need it locally, put it in.git/info/excludeor your global gitignore rather than committing it. Remove this file from the patch.Also note the hardcoded relative include path
-I../../../../src/include(line 43) is brittle and workstation/subdirectory-specific, which further confirms this is a local artifact, not a tree-wide config.