Skip to content

Conversation

@pyramation
Copy link
Contributor

Add concurrent role creation test (reproduces unique_violation bug)

Summary

This PR adds a comprehensive test suite that intentionally demonstrates the race condition in concurrent PostgreSQL role creation on the main branch. The test should FAIL on main and PASS on the fix branches (PR #325 and PR #326).

Key Changes:

  • New test file: packages/pgsql-test/__tests__/postgres-test.concurrent-roles.test.ts
  • Four test cases covering different concurrency scenarios
  • Performance benchmarks measuring latency at concurrency levels 2, 4, and 8

The Bug Being Demonstrated:
When multiple processes attempt to create the same PostgreSQL role simultaneously, the current code only catches duplicate_object exceptions. However, under concurrency, the actual error is unique_violation (error code 23505) when both sessions race to insert into pg_authid and hit the pg_authid_rolname_index unique constraint.

Test Cases:

  1. Concurrent role creation - Reproduces the bug (should FAIL on main)
  2. Performance benchmarks - Measures latency with both exceptions caught (informational)
  3. Concurrent GRANT operations - Tests role membership grants under concurrency
  4. High concurrency stress test - Tests 10 roles × 3 concurrent attempts each with retries

Review & Testing Checklist for Human

CRITICAL - This PR is expected to FAIL on main branch:

Test Plan:

# On main branch (should fail):
cd packages/pgsql-test
pnpm test postgres-test.concurrent-roles.test.ts

# Expected: First test fails with unique_violation error
# Expected: Performance benchmark completes and prints latency stats

# On PR #325 or #326 branches (should pass):
git checkout devin/1763429451-fix-concurrent-role-creation  # or PR #326 branch
cd packages/pgsql-test
pnpm test postgres-test.concurrent-roles.test.ts

# Expected: All tests pass

Notes

Related PRs:

Why this test is valuable:

  • Provides reproducible demonstration of the concurrency bug
  • Establishes performance baseline for comparing lock vs no-lock approaches
  • Can be used for regression testing to ensure the fix works

Potential Issues:

  • Test may be flaky due to timing - concurrent operations might not always race
  • If test infrastructure doesn't support high concurrency, the bug might not reproduce consistently
  • Database cleanup could fail if tests are interrupted

Session Info:

This test demonstrates the race condition that occurs when multiple
processes attempt to create the same PostgreSQL role simultaneously.

On the main branch, this test FAILS with unique_violation error when
multiple concurrent CREATE ROLE statements hit the pg_authid_rolname_index.

The test includes:
- Concurrent role creation test (reproduces the bug)
- Performance benchmarks at concurrency levels 2, 4, 8
- Concurrent GRANT operations test
- High concurrency stress test with retries

This PR intentionally fails on main to demonstrate the problem.
Compare against PR #325 (with locks) and PR #326 (no locks) to see fixes.

Co-Authored-By: Dan Lynch <[email protected]>
@devin-ai-integration
Copy link
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@devin-ai-integration
Copy link
Contributor

Related PRs

This PR demonstrates the concurrent role creation race condition that is fixed by:

Note: This test is expected to FAIL on the main branch to demonstrate the bug. It should PASS when applied to either PR #325 or PR #326.

Performance Comparison

The test includes performance benchmarks at concurrency levels 2, 4, and 8 to measure the performance impact of the lock-based vs exceptions-only approaches.


Link to Devin run: https://app.devin.ai/sessions/2c33cab4511f4a7897e29001895487df
Requested by: Dan Lynch ([email protected]) / @pyramation

- Replace invalid 'CREATE ROLE IF NOT EXISTS' with proper DO block syntax
- PostgreSQL doesn't support IF NOT EXISTS for CREATE ROLE
- Use IF NOT EXISTS check inside DO block with exception handling
- Improve main test to loop 10 times with 6 concurrent attempts
- This increases reliability of reproducing the race condition on CI

The test intentionally only catches duplicate_object (not unique_violation)
in the reproduction test to demonstrate the bug on main branch.

Co-Authored-By: Dan Lynch <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants