Skip to content

Conversation

devin-ai-integration[bot]
Copy link
Contributor

Add thread-safety to FireboltDriver with RWMutex

Summary

Added thread-safety to the FireboltDriver struct to enable safe concurrent access from multiple goroutines. The singleton driver instance registered via sql.Register("firebolt", &FireboltDriver{}) was previously not thread-safe, causing potential race conditions when accessed concurrently (e.g., in performance tests with multiple goroutines).

Key changes:

  • Added sync.RWMutex field to FireboltDriver struct
  • Implemented double-checked locking pattern in OpenConnector method for optimal performance
  • Protected all field accesses in option functions with appropriate locks
  • Used RLock/RUnlock for reads (allows concurrent reads) and Lock/Unlock for writes (exclusive access)

The implementation prioritizes performance by using a double-checked locking pattern - the common case where DSN hasn't changed uses a read lock for high concurrency, while the rare reinitialization case uses a write lock.

Review & Testing Checklist for Human

  • Verify double-checked locking correctness - Review the OpenConnector method implementation to ensure the pattern is correct and race-free
  • Performance impact testing - Run concurrent benchmarks to verify performance hasn't degraded significantly, especially for cached connection scenarios
  • Integration testing with real concurrent workloads - Test with multiple goroutines accessing the driver simultaneously using real Firebolt connections (not just unit tests)

Notes

  • All tests pass with Go race detector (go test -race) - no race conditions detected
  • Only short tests were run - full integration tests with real Firebolt instances should be executed
  • The performance optimization uses read locks for the common case (DSN unchanged) and write locks only for reinitialization

Link to Devin run: https://app.devin.ai/sessions/13544bc77c724e6fb35d3e5626975b17
Requested by: @ptiurin

- Add sync.RWMutex field to FireboltDriver struct
- Implement double-checked locking in OpenConnector for performance
- Protect all field accesses in option functions with Lock/Unlock
- Ensures safe concurrent access from multiple goroutines
- Verified with race detector - no race conditions detected

Co-Authored-By: [email protected] <[email protected]>
Copy link
Contributor Author

🤖 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

Copy link

github-actions bot commented Oct 9, 2025

Pull request title linting 🚨

In order to merge this pull request, the title of the pull request should be prefixed by one of the available types.

Available types:

  • feat: A new feature
  • fix: A bug fix
  • docs: Documentation only changes
  • style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
  • refactor: A code change that neither fixes a bug nor adds a feature
  • perf: A code change that improves performance
  • test: Adding missing tests or correcting existing tests
  • build: Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
  • ci: Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)
  • chore: Other changes that don't modify src or test files
  • revert: Reverts a previous commit

Examples
feat(grpc): add new endpoint
refactor: combine class A and class B
ci: update pull request linter
style: change format of strings

💁 For more examples, visit https://www.conventionalcommits.org/en/v1.0.0/#examples.

Copy link

sonarqubecloud bot commented Oct 9, 2025

@devin-ai-integration devin-ai-integration bot changed the title Add thread-safety to FireboltDriver with RWMutex refactor: add thread-safety to FireboltDriver with RWMutex Oct 9, 2025
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.

0 participants