Skip to content

Commit 3281caa

Browse files
committed
Improved handling of DB connection acquisition timeouts
* syncVersions now atomic - if listing either S3 or COMS versions fails, throw an error and don't continue * Increase DB acquire connection timeout to 1.5 mins (configurable via new envar: db.acquireConnectionTimeout) * Separate out getBucket() errors instead of throwing everything as a 404 (including DB connection-related errors)
1 parent 8f0d340 commit 3281caa

File tree

7 files changed

+26
-13
lines changed

7 files changed

+26
-13
lines changed

app/README.md

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -48,15 +48,17 @@ The following variables enable and enforce the use of Basic Authentication for r
4848

4949
The following variables configure the use of a backend database to support user-based access control, tagging and other advanced features
5050

51-
| Config Var | Env Var | Default | Notes |
52-
| ---------- | ------------- | --------- | --------------------------- |
53-
| `database` | `DB_DATABASE` | coms | COMS database name |
54-
| `host` | `DB_HOST` | localhost | Database conection hostname |
55-
| `username` | `DB_USERNAME` | app | Database account username |
56-
| `password` | `DB_PASSWORD` | | Database account password |
57-
| `port` | `DB_PORT` | 5432 | Database connection port |
58-
| `poolMin` | `DB_POOL_MIN` | 2 | avalable connections |
59-
| `poolMax` | `DB_POOL_MAX` | 10 | available connections |
51+
| Config Var | Env Var | Default | Notes |
52+
|----------------------------|---------------------------------|-----------|---------------------------------------------------------------------|
53+
| `acquireConnectionTimeout` | `DB_ACQUIRE_CONNECTION_TIMEOUT` | 90000 | Timeout length on acquiring a database connection (in milliseconds) |
54+
| `database` | `DB_DATABASE` | coms | COMS database name |
55+
| `host` | `DB_HOST` | localhost | Database conection hostname |
56+
| `username` | `DB_USERNAME` | app | Database account username |
57+
| `password` | `DB_PASSWORD` | | Database account password |
58+
| `port` | `DB_PORT` | 5432 | Database connection port |
59+
| `poolMin` | `DB_POOL_MIN` | 2 | avalable connections |
60+
| `poolMax` | `DB_POOL_MAX` | 10 | available connections |
61+
6062

6163
### Keycloak Variables
6264

app/config/custom-environment-variables.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
"username": "BASICAUTH_USERNAME"
77
},
88
"db": {
9+
"acquireConnectionTimeout": "DB_ACQUIRE_CONNECTION_TIMEOUT",
910
"database": "DB_DATABASE",
1011
"host": "DB_HOST",
1112
"password": "DB_PASSWORD",

app/config/default.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
{
22
"db": {
3+
"acquireConnectionTimeout": 90000,
34
"database": "coms",
45
"host": "localhost",
56
"port": "5432",

app/knexfile.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ types.setTypeParser(1184, (value) => new Date(value).toISOString());
3232
const logWrapper = (level, msg) => log.log(level, msg);
3333

3434
module.exports = {
35+
acquireConnectionTimeout: config.get('db.acquireConnectionTimeout'),
3536
client: 'pg',
3637
connection: {
3738
host: config.get('db.host'),

app/src/components/utils.js

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,15 @@ const utils = {
115115
return data;
116116
} catch (err) {
117117
log.error(err.message, { function: 'getBucket' });
118-
throw new Problem(404, { detail: err.message });
118+
if (err.name === 'NotFoundError') {
119+
throw new Problem(404, { detail: `bucketId ${bucketId} not found` });
120+
}
121+
else if (err.name == 'KnexTimeoutError') {
122+
throw new Problem(504, { detail: 'Database timeout' });
123+
}
124+
else {
125+
throw new Problem(500, { detail: err.message });
126+
}
119127
}
120128
},
121129

app/src/services/objectQueue.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ const { NIL: SYSTEM_USER } = require('uuid');
33
const { ObjectQueue } = require('../db/models');
44

55
/**
6-
* Max number of parameters in a prepared statement (this is a Postrgres hard-coded limit).
6+
* Max number of parameters in a prepared statement (this is a Postgres hard-coded limit).
77
* https://www.postgresql.org/docs/current/limits.html ("query parameters")
88
*/
99
const POSTGRES_QUERY_MAX_PARAM_LIMIT = 65535;

app/src/services/sync.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -238,10 +238,10 @@ const service = {
238238
const comsObject = typeof object === 'object' ? object : await objectService.read(object, trx);
239239

240240
// Check for COMS and S3 Version statuses
241-
const [comsVersions, s3VersionsRaw] = await Promise.allSettled([
241+
const [comsVersions, s3VersionsRaw] = await Promise.all([
242242
versionService.list(comsObject.id, trx),
243243
storageService.listAllObjectVersions({ filePath: comsObject.path, bucketId: comsObject.bucketId })
244-
]).then(settled => settled.map(promise => promise.value));
244+
]);
245245

246246
// Combine S3 DeleteMarkers and Versions into one array
247247
const s3Versions = s3VersionsRaw.DeleteMarkers

0 commit comments

Comments
 (0)