Skip to content

Commit bac3cd0

Browse files
authored
feat: Unify pool to make named timeout errors (#10375)
Centralizes generic-pool usage in @cubejs-backend/shared with a Pool wrapper that provides enhanced timeout error messages including the pool name. Drivers now import Pool from shared instead of using generic-pool directly.
1 parent 7cbafd5 commit bac3cd0

13 files changed

Lines changed: 125 additions & 22 deletions

File tree

packages/cubejs-backend-shared/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
},
4343
"dependencies": {
4444
"@oclif/color": "^0.1.2",
45+
"generic-pool": "^3.9.0",
4546
"bytes": "^3.1.2",
4647
"cli-progress": "^3.9.0",
4748
"cross-spawn": "^7.0.3",

packages/cubejs-backend-shared/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,3 +26,4 @@ export * from './decorators';
2626
export * from './PerfTracker';
2727
export * from './disposedProxy';
2828
export * from './logger';
29+
export * from './pool';
Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
/**
2+
* @copyright Cube Dev, Inc.
3+
* @license Apache-2.0
4+
* @fileoverview Named pool wrapper around generic-pool with enhanced error messages.
5+
*/
6+
7+
import genericPool, { Pool as GenericPool, Factory, Options } from 'generic-pool';
8+
9+
export { Factory, Options as PoolOptions } from 'generic-pool';
10+
11+
export class PoolTimeoutError extends Error {
12+
public readonly poolName: string;
13+
14+
public constructor(poolName: string) {
15+
super(`ResourceRequest timed out (pool: ${poolName})`);
16+
this.name = 'PoolTimeoutError';
17+
this.poolName = poolName;
18+
}
19+
}
20+
21+
/**
22+
* Uses composition instead of inheritance because generic-pool doesn't export
23+
* a Pool class, the Pool type is an interface, not an extendable class.
24+
*/
25+
export class Pool<T> {
26+
private readonly pool: GenericPool<T>;
27+
28+
private readonly name: string;
29+
30+
public constructor(name: string, factory: Factory<T>, options?: Options) {
31+
this.name = name;
32+
this.pool = genericPool.createPool<T>(factory, options);
33+
}
34+
35+
public async acquire(priority?: number): Promise<T> {
36+
try {
37+
return await this.pool.acquire(priority);
38+
} catch (error) {
39+
if (error instanceof Error && error.name === 'TimeoutError') {
40+
throw new PoolTimeoutError(this.name);
41+
}
42+
43+
throw error;
44+
}
45+
}
46+
47+
public async release(resource: T): Promise<void> {
48+
return this.pool.release(resource);
49+
}
50+
51+
public async destroy(resource: T): Promise<void> {
52+
return this.pool.destroy(resource);
53+
}
54+
55+
public async drain(): Promise<void> {
56+
return this.pool.drain();
57+
}
58+
59+
public async clear(): Promise<void> {
60+
return this.pool.clear();
61+
}
62+
63+
public async use<U>(cb: (resource: T) => U | Promise<U>): Promise<U> {
64+
try {
65+
return await this.pool.use(cb);
66+
} catch (error) {
67+
if (error instanceof Error && error.name === 'TimeoutError') {
68+
throw new PoolTimeoutError(this.name);
69+
}
70+
throw error;
71+
}
72+
}
73+
74+
// State accessors
75+
public get size(): number {
76+
return this.pool.size;
77+
}
78+
79+
public get available(): number {
80+
return this.pool.available;
81+
}
82+
83+
public get borrowed(): number {
84+
return this.pool.borrowed;
85+
}
86+
87+
public get pending(): number {
88+
return this.pool.pending;
89+
}
90+
91+
public get max(): number {
92+
return this.pool.max;
93+
}
94+
95+
public get min(): number {
96+
return this.pool.min;
97+
}
98+
99+
// Event handling
100+
public on(event: 'factoryCreateError' | 'factoryDestroyError', listener: (err: Error) => void): this {
101+
this.pool.on(event, listener);
102+
return this;
103+
}
104+
105+
// For backward compatibility (drivers use pool._factory for testConnection)
106+
public get _factory(): Factory<T> {
107+
// eslint-disable-next-line no-underscore-dangle
108+
return (this.pool as any)._factory;
109+
}
110+
}

packages/cubejs-cubestore-driver/package.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@
3333
"csv-write-stream": "^2.0.0",
3434
"flatbuffers": "23.3.3",
3535
"fs-extra": "^9.1.0",
36-
"generic-pool": "^3.8.2",
3736
"node-fetch": "^2.6.1",
3837
"sqlstring": "^2.3.3",
3938
"tempy": "^1.0.1",

packages/cubejs-hive-driver/package.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
"dependencies": {
2020
"@cubejs-backend/base-driver": "1.6.9",
2121
"@cubejs-backend/shared": "1.6.9",
22-
"generic-pool": "^3.8.2",
2322
"jshs2": "^0.4.4",
2423
"sasl-plain": "^0.1.0",
2524
"saslmechanisms": "^0.1.1",

packages/cubejs-hive-driver/src/HiveDriver.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,10 @@
77
const {
88
getEnv,
99
assertDataSource,
10+
Pool,
1011
} = require('@cubejs-backend/shared');
1112
const jshs2 = require('jshs2');
1213
const SqlString = require('sqlstring');
13-
const genericPool = require('generic-pool');
1414
const { BaseDriver } = require('@cubejs-backend/base-driver');
1515
const Connection = require('jshs2/lib/Connection');
1616
const IDLFactory = require('jshs2/lib/common/IDLFactory');
@@ -82,7 +82,7 @@ class HiveDriver extends BaseDriver {
8282

8383
const configuration = new Configuration(this.config);
8484

85-
this.pool = genericPool.createPool({
85+
this.pool = new Pool('hive', {
8686
create: async () => {
8787
const idl = new IDLContainer();
8888
await idl.initialize(configuration);

packages/cubejs-jdbc-driver/package.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
"@cubejs-backend/base-driver": "1.6.9",
2929
"@cubejs-backend/node-java-maven": "^0.1.3",
3030
"@cubejs-backend/shared": "1.6.9",
31-
"generic-pool": "^3.9.0",
3231
"sqlstring": "^2.3.0"
3332
},
3433
"optionalDependencies": {

packages/cubejs-jdbc-driver/src/JDBCDriver.ts

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
getEnv,
99
assertDataSource,
1010
CancelablePromise,
11+
Pool,
1112
} from '@cubejs-backend/shared';
1213
import {
1314
BaseDriver,
@@ -17,7 +18,6 @@ import {
1718
} from '@cubejs-backend/base-driver';
1819
import * as SqlString from 'sqlstring';
1920
import { promisify } from 'util';
20-
import genericPool, { Factory, Pool } from 'generic-pool';
2121
import path from 'path';
2222

2323
import { SupportedDrivers } from './supported-drivers';
@@ -72,14 +72,10 @@ Connection.prototype.getMetaDataAsync = promisify(Connection.prototype.getMetaDa
7272
DatabaseMetaData.prototype.getSchemasAsync = promisify(DatabaseMetaData.prototype.getSchemas);
7373
DatabaseMetaData.prototype.getTablesAsync = promisify(DatabaseMetaData.prototype.getTables);
7474

75-
interface ExtendedPool extends Pool<any> {
76-
_factory: Factory<any>;
77-
}
78-
7975
export class JDBCDriver extends BaseDriver {
8076
protected readonly config: JDBCDriverConfiguration;
8177

82-
protected pool: ExtendedPool;
78+
protected pool: Pool<any>;
8379

8480
protected jdbcProps: any;
8581

@@ -136,7 +132,7 @@ export class JDBCDriver extends BaseDriver {
136132
throw new Error('url is required property');
137133
}
138134

139-
this.pool = genericPool.createPool({
135+
this.pool = new Pool('jdbc', {
140136
create: async () => {
141137
await initMvn(await this.getCustomClassPath());
142138

@@ -182,7 +178,7 @@ export class JDBCDriver extends BaseDriver {
182178
testOnBorrow: true,
183179
acquireTimeoutMillis: 120000,
184180
...(poolOptions || {})
185-
}) as ExtendedPool;
181+
});
186182

187183
// https://github.com/coopernurse/node-pool/blob/ee5db9ddb54ce3a142fde3500116b393d4f2f755/README.md#L220-L226
188184
this.pool.on('factoryCreateError', (err) => {
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Options } from 'generic-pool';
1+
import { PoolOptions } from '@cubejs-backend/shared';
22

33
export type JDBCDriverConfiguration = {
44
database: string,
@@ -7,6 +7,6 @@ export type JDBCDriverConfiguration = {
77
drivername: string,
88
customClassPath?: string,
99
properties: Record<string, any>,
10-
poolOptions?: Options;
10+
poolOptions?: PoolOptions;
1111
prepareConnectionQueries?: string[];
1212
};

packages/cubejs-mongobi-driver/package.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
"@cubejs-backend/base-driver": "1.6.9",
3131
"@cubejs-backend/shared": "1.6.9",
3232
"@types/node": "^20",
33-
"generic-pool": "^3.9.0",
3433
"moment": "^2.29.1",
3534
"mysql2": "^3.11.5"
3635
},

0 commit comments

Comments
 (0)