Make installcheck segfaults (SPOC-383) #292
Merged
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
Some utility commands (e.g., CLUSTER without table name) execute outside transaction blocks, committing after each table operation. When the AutoDDL hook fires after such commands complete, catalog operations and queue insertions fail without transaction context. This commit adds transaction and snapshot guards at the AutoDDL entry point in spock_autoddl_process(). Test case added: CLUSTER without table name after ALTER TABLE CLUSTER ON verifies that AutoDDL works correctly when invoked outside transaction context. co-author: Claude.
There are some evidence exists that we have been cavalier with database lookups. It seems sometimes Spock does it outside a transaction. And the reason why we didn't see any issues before is that relation we touch is frequently in the syscache. It is unstable situation and we must fix it. It is too expensive to run tests with the '--discard-caches' enabled. So, to make thing simpler, add assertions to check we touch the database inside a transaction.
Add Assert(IsTransactionState()) to all functions that perform catalog access in spock_queue.c, spock_sync.c, and spock_exception_handler.c. These assertions complement the existing assertions in spock_node.c and spock_repset.c, providing comprehensive protection against calling catalog functions outside transaction context. Functions updated: - spock_queue.c: queue_message(), queued_message_from_tuple() - spock_sync.c: create_local_sync_status(), drop_subscription_sync_status(), get_subscription_sync_status(), drop_table_sync_status(), drop_table_sync_status_for_sub(), get_table_sync_status(), get_unsynced_tables(), get_subscription_tables() - spock_exception_handler.c: add_entry_to_exception_log() These assertions will help catch transaction state violations during development and testing, particularly in edge cases like CLUSTER without table name or other utility commands that run outside transaction blocks.
danolivo
changed the title
danolivo
force-pushed
the
make-installcheck-segfaults
branch
from
61cc7e2 to
f622562
Compare
Previous commit added a test on this feature and opened that Spock has troubles
on subscriber's side with commands which requires no transaction.
Having no clean idea what to do here we may just filter problematic commands.
DESCRIPTION:
===========
Some PostgreSQL utility commands cannot be executed inside transaction
blocks. When these commands are replicated to subscribers, the apply
worker executes them inside transactions (via begin_replication_step()),
causing failures or crashes.
This commit implements publisher-side filtering to prevent problematic
commands from being queued for replication:
1. CLUSTER command filtering:
- Skip CLUSTER without table name (clusters all tables)
- Skip CLUSTER on partitioned tables
- Allow CLUSTER on regular tables (safe to replicate)
2. Additional transaction-incompatible commands:
- VACUUM
- CREATE/DROP TABLESPACE
- REINDEX CONCURRENTLY
- CREATE/DROP DATABASE (already filtered)
- Concurrent index operations (already filtered)
In addition:
3. Optimization in autoddl_can_proceed():
- Remove early get_local_node() check for better performance
- The check is still performed later in spock_auto_replicate_ddl()
after initial filtering reduces unnecessary catalog lookups
The filtering approach is simpler and more maintainable than attempting
to handle these commands specially on the subscriber side. Users receive
a clear warning when DDL statements cannot be replicated.
danolivo
force-pushed
the
make-installcheck-segfaults
branch
from
f622562 to
b0e8677
Compare
mason-sharp
reviewed
Dec 20, 2025
mason-sharp
reviewed
Dec 20, 2025
Member
|
Looks good, just need to address that PG 15 and 16 are failing. |
Contributor
Author
Done. |
mason-sharp
approved these changes
Dec 21, 2025
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.
No description provided.