SPOC-527: add 024_node_id_collision for duplicate node_id behaviour

[danolivo]

Summary

Add a TAP test that exercises Spock's behaviour when two independently-created nodes end up sharing a node_id. The id is currently generated locally as hash_any(name) & 0xffff at node_create time, with no cluster-wide coordination — so this collision is reachable in practice (cluster splits, geo-migrations, DR rehydration). The test documents what happens today, before any negotiation protocol lands.

What's covered

The test creates a 2-node cluster (node_create only, no cross-wire), then tampers with n2's catalog to give it n1's id. Tampering navigates two FK constraints: node_interface.if_nodeid and replication_set.set_nodeid follow via ON UPDATE CASCADE; local_node.node_id has no cascade and is updated manually under SET session_replication_role = replica to bypass the FK trigger. The PRIMARY KEY on spock.node(node_id) is enforced by the unique index, so this works only because we tamper pre-attach when each catalog still holds a single row.
With the colliding state in place, sub_create is attempted from both directions. The test asserts:

• both attempts fail with duplicate key value violates unique constraint "node_pkey" (the existing PK catches the collision when create_node(origin) at src/spock_functions.c:503 tries to insert the remote-rep row);
• both spock.node catalogs still hold exactly one row (the failed transactions rolled back atomically);
• spock.subscription is empty on both sides (no orphan subscription rows pointing at a colliding id);
• n1's seed table was not synced to n2 (sub_create was invoked with sync_structure=true, sync_data=true — the PK trip has to happen before the copy phase or n2 would be left with an orphaned populated table).

Out of scope

The cluster-merge case where a colliding id sits on a third-party peer (one the joining node hasn't subscribed to directly) produces silent misattribution rather than a PK trip. That requires a 3-node forwarding scenario and is left for the upcoming negotiation-protocol work.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5cc6e6ed-d336-4136-9f0a-b36d5cc92bec

📥 Commits

Reviewing files that changed from the base of the PR and between b7fc702 and e8ed8eb.

📒 Files selected for processing (2)

• tests/tap/schedule
• tests/tap/t/024_node_id_collision.pl

---

📝 Walkthrough

Walkthrough

A new integration test for node ID collision scenarios is added to the test suite. The test validates that subscriptions fail when duplicate identifiers are engineered on different nodes and confirms proper transactional rollback behavior.

Changes

Cohort / File(s)	Summary
Node ID Collision Integration Test tests/tap/schedule, tests/tap/t/024_node_id_collision.pl	Adds a TAP test that engineers a spock.node_id collision by matching node identifiers between two cluster nodes, validates subscription creation failures due to duplicate key constraints, and verifies transactional rollback preserves cluster state integrity.

Poem

Two nodes walk into a database bar,
Their IDs collide like a shooting star! ✨
"Duplicate key?" cried the subscription queue,
Rolling back transactions, like rabbits do. 🐰
Collision detected, the safety check's true!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly references the issue (SPOC-527) and accurately describes the main change: adding a test for node_id collision behavior.
Description check	✅ Passed	The description provides a detailed, coherent explanation of the test's purpose, implementation, and assertions, all directly related to the changeset.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch spoc-527

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}