Perf issue 1641 select for update

[rakdutta]

closes issue ##1641
This PR addresses performance and data consistency issues under high concurrency by introducing and testing row-level locking using get_for_update across critical service methods. The changes ensure that updates, toggles, and conflict checks on key resources (tools, servers, resources, prompts, gateways, A2A agents) are safe from race conditions, especially when running on PostgreSQL

Key Changes

• Implements and tests the use of get_for_update for row-level locking in all major service classes.

• Adds a comprehensive new test suite: test_row_level_locking.pyto verify correct locking behavior and prevent race conditions.

• Refactors and improves mocking in existing service tests to accurately simulate locking and database behavior.

• Updates tool, server, and agent service tests to ensure atomic operations and correct handling of concurrent modifications.

• Enhances test coverage for edge cases, including conflict detection and concurrent toggles.

---

Run the Concurrency Row Locking Integration Test

Option 1: Using Docker (Recommended)

# Start a PostgreSQL container
docker run -d \
  --name postgres-test \
  -e POSTGRES_PASSWORD=test \
  -e POSTGRES_DB=test \
  -p 5432:5432 \
  postgres:15

Option 2: Using Local PostgreSQL

Ensure PostgreSQL is running locally, then create a test database and user:

CREATE DATABASE test;
CREATE USER postgres WITH PASSWORD 'test';
GRANT ALL PRIVILEGES ON DATABASE test TO postgres;

Run the Test

# Set the database URL
export DATABASE_URL=postgresql+psycopg://postgres:mysecretpassword@localhost:5432/test

# Execute the integration test
pytest tests/integration/test_concurrency_row_locking.py -v --with-integration

[crivetimihai]

Code Review Fixes Applied

After reviewing this PR, I've applied several fixes to address issues found during review:

Critical Fixes

Issue	Fix
skip_locked=True default could miss conflicts	Changed default to skip_locked=False - now waits for locked rows instead of skipping them
get_server using FOR UPDATE for read-only operation	Reverted to regular db.execute(select(...)) - no locking needed for reads
toggle_gateway_status holding lock during network I/O	Reverted to regular select - holding DB locks during _initialize_gateway network calls would cause timeouts/deadlocks
DELETE...RETURNING not supported on MySQL/MariaDB	Changed to rowcount check which works across all databases

Test Improvements

Issue	Fix
Test gating only checked DB=postgres	Added is_postgresql() helper that checks both DB env var and DATABASE_URL
Unused imports (F401) in test files	Removed with autoflake
Trailing whitespace (W293) in test files	Fixed with black
No-op test bodies	Added @pytest.mark.skip decorators with reasons
Test mock for delete operations	Updated to use side_effect for different rowcount values

Commits Added

6d513f9c chore: Fix lint issues in PR test files
ebf03e89 fix: Use rowcount instead of RETURNING for MySQL compatibility
dd25b908 fix: Improve PostgreSQL detection in concurrency tests
ce55fa07 fix: Address row-level locking issues in PR #1990
7a0f9485 chore: Remove commented out db.get() calls

Verification

• ✅ make flake8 passes
• ✅ make ruff passes
• ✅ All 1611 unit tests pass
• ✅ No F401/W293 lint errors in PR files

Design Note

toggle_gateway_status intentionally does NOT use row locking because it calls _initialize_gateway which performs network I/O. Holding a database lock during potentially slow network operations would block other database operations and risk timeouts. The code includes a comment explaining this tradeoff.