RobAndrewHurst commented 2 weeks ago

pg dbs connection management improvements & docs 🐘

Overview

This PR implements robust database connection handling with retry logic, better error management, and connection pooling optimizations for our PostgreSQL connections.

Key Changes

Implemented retry logic with exponential backoff
Enhanced connection pool configuration
Added comprehensive error handling
Improved resource management
Added detailed logging
Added JSDoc documentation

Detailed Implementation

1. Retry Logic

Maximum of 3 retry attempts
Exponential backoff delay between retries
Initial delay of 1 second, doubling with each retry

2. Connection Pool Configuration

const pool = new Pool({
  connectionString: process.env[key],
  keepAlive: true,
  connectionTimeoutMillis: 5000,    // 5 second timeout for connection attempts
  idleTimeoutMillis: 30000,         // 30 second timeout for idle connections
  max: 20                           // Maximum pool size
});

3. Error Handling

Pool-level error handler for idle client issues
Per-query error handling with retry information
Proper error classification for retry decisions
Error logging with context information

4. Resource Management

Guaranteed client release using finally block
Force release flag for error cases
Statement timeout handling

5. Logging Enhancements

Query context in error logs
Retry attempt counting
Pool identification
Error details and stack traces

Testing

Unit tests should be updated to include:
- Retry behavior verification
- Connection timeout scenarios
- Pool management

Performance Impact

Minimal impact on successful queries
Improved handling of transient failures
Better resource utilization through connection pooling
Controlled retry behavior prevents cascade failures

Documentation

JSDoc documentation has been added.

Testing Checklist

[ ] Verify retry behavior with simulated network failures
[ ] Test connection pool limits
[ ] Verify error logging format
[ ] Test statement timeout functionality
[ ] Verify resource cleanup
[ ] Load test under high concurrency
[ ] Test with different database configurations

Security Considerations

No new security implications
Maintains existing connection string security
No exposure of sensitive information in logs

dbauszus-glx commented 2 weeks ago

I have update the module documentation to include requires for the pg and logger. The documentation for pg.Pool is linked in the docs.

RobAndrewHurst commented 2 weeks ago

Putting back into draft to address some issues

dbauszus-glx commented 2 weeks ago

I removed the retry codes as these are ambiguous dependent on the dbs provider and version. It is also not clear which type of error should be retried.

I created a sleep function as this easier to understand than a const sleep that is a function.

I created the clientQuery function to receive the pool param which has the dbs as an option.

RobAndrewHurst commented 1 week ago

It should be possible to set the RETRY_LIMIT through the process configuration. Ideally by adding a property to the connection string.

The dbs_string now takes an additional optional parameter at the end of the connection string itself separated by a '|'. This will override the RETRY_LIMIT on the module.

@dbauszus-glx Where should we add in this documentation? Into the wiki or on the module itself explaining how connection string work?

dbauszus-glx commented 1 week ago

Having thought about this. It is probably better to use process.env.RETRY_LIMIT for documentation purposes.

This and the DBS need to be added to the global env type documentation.

RobAndrewHurst commented 1 week ago

Having thought about this. It is probably better to use process.env.RETRY_LIMIT for documentation purposes.

This and the DBS need to be added to the global env type documentation.

This has now been updated. The RETRY_LIMIT const will be assigned from the process.env.RETRY_LIMIT if it is present otherwise it will default to 3.

I have also included the DBS_ param and RETRY_LIMIT in the global .env docs