Install spock changes #316
Closed
Conversation
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
DESTDIR is used in package builds to install Postgres and extensions to a build root. We already use this prefix elsewhere but it was missing from these two lines. PLAT-173
* Create README.md
Add to list of those to run.
…nctions.md, spockctrl.md, Removed callout in features.md, Formatting changes in creating_subscriber_nodes.md, spock_info.md (#101) Co-authored-by: Susan Douglas <susan@pgedge.local>
SPOC-125 : Added new sample files for ZODAN. After merging, I'll move this into a folder.
….md (#104) Co-authored-by: Susan Douglas <susan@pgedge.local>
) Replaced secrets.READ_SPOCKBENCH_PAT with secrets.GITHUB_TOKEN for (#110) Checking out the pgedge/spockbench repository. This allows the workflow to run in pull requests from forks now that spockbench is public.
* Bump Spock extension version from 5.0.0 to 6.0.0.devel. - Updated SPOCK_VERSION in spock.h to 6.0.0.devel - Blank SQL file create to upgrade from 5.0.0 to 6.0.0.devel * SPOC-156: Use spock-6.0.0-devel.sql instead of spock--6.0.0.devel. * SPOC-156: Added missing files. * SPOC-156: Update expected output file init.out. --------- Co-authored-by: Ibrar Ahmed <ibrar@pgedge.com>
…pdating spockctrl README to remove mention of crosswiring workflows (#108) Co-authored-by: Susan Douglas <susan@pgedge.local>
* Fix replicate_ddl default search_path with updated behavior
This was inconsistent with the overloaded version of the function,
which correctly defaulted to current_setting('search_path'). This
patch updates the original version to match, resolving the issue.
Updated triggers.out expected file to reflect schema-less NOTICE
messages (due to search_path being set).
* Updated regression test expected files to reflect changes in NOTICE
and ERROR output, which no longer include schema-qualified names. This
is due to the updated search_path default in the spock.replicate_ddl
function.
…tion Conflict detection now evaluates the index predicate on both the remote and local tuples. This prevents false positives when a partial unique index applies only to a subset of rows. If the remote tuple does not match the index predicate, we skip scanning the index altogether, as no conflict is possible in that case.
Put change in spock--5.0.0--5.0.1.sql
These two changes to the Spock initial script are intended to make ‘make installcheck’ clearer by avoiding false-positive errors. Adding parallel safety clause to these functions we state we actual behaviour.
1. Bug. In the spock_repset.c module functions replication_set_add_table and replication_set_add_seq should check locking on the table before calling another lock-acquiring routine. It prevents unnecessary calls and reduces the lock level. 2. Improvement. In the spock_queue.c module function queue_message insert a tuple into the spock.queue table. The pattern of this table usage is quite trivial and doesn’t include any concurrent updates. So, we may release the table lock before commit without harm, and let ‘make installcheck’ be more conventional. author: Andrei Lepikhov co-author: Claude
In case of IF EXISTS / NOT EXISTS clause Postgres core utility call produces an INFO message and lets utility hooks do their job as usual. In the Spock case, we need to do the same and carefully process the clause so it doesn't produce an ERROR that breaks the convention.
Now, each apply progress HTAB entry contains tangled values of remote_commit_lsn and remote_commit_ts columns (see handle_commit). So, when extracting data from the progress view, we can use any of these numbers, and using LSN streamlines the check_commit_timestamp_and_advance_slot procedure: we can directly point to the correct WAL record, avoiding the expensive get_lsn_from_commit_ts() call. So, replace the expensive search for the proper LSN with direct picking of a specific WAL record. TODO: Further commits should also change the Python version. And consequently, remove the function get_lsn_from_commit_ts() from the extension altogether.
Add health check from zodan.py to zodan.sql Add verification from zodan.sql to zodan.py
Remove the following unused routines: - get_commit_timestamp - advance_replication_slot - monitor_replication_lag(text, boolean) - monitor_replication_lag_wait - monitor_multiple_replication_lags - wait_for_n3_sync - check_node_lag In the procedure monitor_lag_with_dblink, use the function clock_timestamp instead of now(). The now() function is stable and doesn't change the value within the monitoring loop. So, the volatile clock_timestamp looks more correct to obtain fresh value of time.
The SpockGroupEntry structure contained a redundant key field that duplicated the key already embedded in its progress member (SpockGroupEntry.key vs SpockGroupEntry.progress.key). This redundancy violated the DRY principle and created potential for inconsistency if the two keys ever diverged. This commit removes the duplicate key field from SpockGroupEntry, relying solely on the key embedded in SpockApplyProgress. The hash table already uses SpockProgressKey (the first field of SpockApplyProgress) as the hash key, making the separate SpockGroupEntry.key field unnecessary. Changes: 1. Structural cleanup: - Removed SpockGroupEntry.key field (the duplicate) - Kept SpockApplyProgress.key as the single source of truth - Renamed SpockGroupKey to SpockProgressKey for clarity 2. Code quality improvements: - Created init_progress_fields() helper for safe initialization - Replaced fragile pointer arithmetic with explicit field initialization - Follows PostgreSQL conventions (similar to CommitTsShmemInit) - Added comprehensive documentation explaining hash table design 3. Documentation and comments: - Added header explaining Group Registry architecture - Clarified that SpockApplyProgress.key MUST be first field - Documented persistence strategy (WAL + file snapshot) - Improved TODO comments to be actionable The functional behavior is unchanged; this is a refactoring that improves code quality and reduces redundancy.
Being loaded via shared_preload_libraries, Spock always initializes its
shared memory segment and hash tables in the postmaster process during
shmem_startup_hook. The shmem_startup_hook is called exactly once per
postmaster lifetime while holding AddinShmemInitLock.
There is no case where the postmaster does not call shmem_startup_hook()
after allocating shared memory. Also, there is no case where the postmaster
performs any operations requiring Spock's shared memory before initialization.
During recovery, the evidence that shared memory is already initialized and
operable is the fact that redo operations can acquire locks and access the
hash tables.
This commit removes the redundant spock_shmem_attach() routine and its calls
from checkpoint hook, WAL redo startup, and bgworker initialization. This
simplifies the code by eliminating the attach/detach complexity.
Changes:
1. Removed spock_shmem_attach() function entirely
- Startup process, checkpointer, and bgworkers no longer need to "attach"
- All processes can directly access shared memory after postmaster init
- Removed the static 'attached' flag tracking per-process attachment
2. Simplified spock_group_shmem_startup()
- Removed 'found' parameter (always creating new structures)
- Removed conditional file loading (always load on initialization)
- The function is only called once via shmem_startup_hook, so checks
for existing structures are unnecessary
3. Simplified spock_shmem_init_internal()
- Removed 'found' return value and tracking
- Removed conditional initialization (if (!SpockCtx), etc.)
- ShmemInit* functions are safely called once under AddinShmemInitLock
- Changed return type from bool to void
4. Code quality improvements
- Updated comments to reflect simplified initialization model
- Improved DEBUG log message clarity
- Removed outdated references to 'all_found' parameter
Benefits:
- Eliminates ~70 lines of unnecessary attachment logic
- Makes initialization flow clearer and easier to understand
- Removes per-process attachment tracking overhead
- Prevents potential bugs from missed spock_shmem_attach() calls
The functional behavior is unchanged; all processes still have access to
Spock's shared memory structures after postmaster startup.
The name SpockGroupKey maintains consistency with SpockGroupEntry. Partial revert of the renaming in commit 2a8607d. No functional changes.
Shape the spock_group module a little: * Remove unused lookup function. * Make spock_group_foreach static, forasmuch as no one uses it outside the module. * Add forgotten ‘extern’ declarations.
There are plenty of places where Spock touches shared memory structures that may cause races. It is always an issue, even considering these structures are rarely accessed for writing. But the progress state is a high-frequency updated structure, and guarding it with a lock is critical.
After moving to HTAB implementation, we actually don't have a target connection. So, remove it. While here, fix forgotten 'extern' declaration in the 'spock_group.h' module.
In Spock 5, we used to employ a progress table and a transactional snapshot to ensure that the progress corresponds to the replication slot (snapshot, LSN), and we could request this state at any time until the transaction ends. With an HTAB, it's no longer true, and we need to invent extra hacks. With this commit, we take the progress state right before LR slot creation, which is as close as possible to the real state. It is still not entirely consistent in the case of Z0DAN, but while we use it just for information, it is ok. XXX: The code related to table syncing stays as is. Right now, we aren't sure if the progress state needs to be corrected if we re-sync a single table from the database. So, just keep the code untouched.
Use direct structure assignment instead of memcpy for copying SpockApplyProgress. This is more idiomatic, type-safe, and resolves static analyzer warnings about buffer size validation. The compiler generates identical code while providing compile-time type checking and better code clarity.
- Fix assertion in spock_worker.c Changed 'Assert(MySpockWorker->proc = MyProc)' to use '==' - Fix memory leak in spock_apply_heap.c Free remotetuple after reporting conflict
When adding a third node in a Z0DAN sync scenario, the origin advancement
logic in spock.check_commit_timestamp_and_advance_slot was using
spock.lag_tracker to retrieve commit timestamps and convert them back to LSNs.
This approach no longer works because spock.progress is now an in-memory HTAB
instead of a catalog table, so lag_tracker doesn't retain historical
data after the SYNC process COPY operation.
Root Cause:
The procedure spock.create_disable_subscriptions_and_slots creates logical
slots on existing nodes (e.g., n2) when adding a new node (n3). In v5,
the commit LSN/timestamp from the source node (n1) was copied to n3 via
lag_tracker during SYNC, and spock.check_commit_timestamp_and_advance_slot
would use this to advance the origin. With the HTAB-based progress table, this
data is no longer available after COPY.
The Fix:
1. Capture the LSN returned by pg_create_logical_replication_slot in
create_disable_subscriptions_and_slots and store it in temp_sync_lsns
2. Use this LSN directly in check_commit_timestamp_and_advance_slot to
advance the origin, eliminating the dependency on lag_tracker and the
timestamp→LSN conversion
This approach is more reliable because it uses the authoritative LSN from
slot creation - the exact point where the apply/sync process will begin
decoding when the subscription is enabled.
Apply the same fix from commit 86acd7b to zodan.py: - Use LSN from pg_create_logical_replication_slot() - Advance slot to stored commit LSN instead of querying lag_tracker - Store both sync_lsn and commit_lsn for later use
Dockerfile improvements: - Add build dependencies (git, zstd-devel, libxml2-devel) - Fix Rocky Linux package compatibility (dnsutils → bind-utils) - Add readline-devel for enhanced psql interactive experience - Remove redundant packages (keep only -devel variants) - Remove duplicate perl-IPC-Run installation (use system package) - Fix cpanm cache cleanup to target correct directory - Remove unnecessary ownership change operations - Improve code consistency and documentation The base image now provides complete PostgreSQL build dependencies for all configure flags (zstd, lz4, ICU, libxml, libxslt, GSSAPI, LDAP, PAM, LLVM, OpenSSL, systemd, Python). Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Embed comprehensive build metadata for exact reproduction: - Capture build timestamp, git commit, branch, and Rocky Linux version - Store metadata in /etc/pgedge/build-info.txt (printed during build) - Add OCI standard labels to image - Tag images with both :latest and :commit-sha - Print all metadata to workflow output Documentation updated with build reproducibility section. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Enable native builds for both Intel/AMD and Apple Silicon systems: - Add QEMU setup for cross-platform emulation - Configure Docker Buildx for multiplatform builds - Build and push linux/amd64 and linux/arm64 variants - Create multiplatform manifest for automatic architecture selection - Update documentation to reflect platform support Docker automatically selects the correct architecture when pulling, providing optimal native performance on Apple Silicon Macs. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Fix hash table removal during iteration to prevent corruption. Use PostgreSQL-style single-pass removal for full flushes and two-pass approach for partial removals. Add error checking for hash_search(HASH_REMOVE) operations. Add NULL checks for HASH_FIXED_SIZE hash tables that can return NULL when full to prevent dereferencing NULL pointers. Add spinlock protection for reading counter values to prevent torn reads of 64-bit counters.
… add warning about changing items in the spock schema
…dded to v5_STABLE as well
Member
|
Looks like it has other commits from main, so closing in favor of a cherry-pick based commit: |
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.
These are the same changes I committed to main (accidentally) yesterday...