From f91e999b250221fd86749a29f46eb821ce03feb7 Mon Sep 17 00:00:00 2001 From: crystall-bitquill <97126568+crystall-bitquill@users.noreply.github.com> Date: Thu, 12 Dec 2024 00:01:52 +0000 Subject: [PATCH] feat: wrapper connect and query timeouts (#342) --- common/lib/driver_dialect/driver_dialect.ts | 4 ++++ common/lib/mysql_client_wrapper.ts | 7 +++++- common/lib/utils/client_utils.ts | 21 ++++++++++++---- common/lib/utils/locales/en.json | 1 + common/lib/wrapper_property.ts | 15 ++++++++++++ .../UsingTheNodejsWrapper.md | 24 ++++++++++--------- .../UsingTheHostMonitoringPlugin.md | 4 ++-- mysql/lib/dialect/mysql2_driver_dialect.ts | 19 ++++++++++++++- .../dialect/node_postgres_driver_dialect.ts | 18 ++++++++++++++ .../container/tests/utils/driver_helper.ts | 4 ++-- 10 files changed, 95 insertions(+), 22 deletions(-) diff --git a/common/lib/driver_dialect/driver_dialect.ts b/common/lib/driver_dialect/driver_dialect.ts index c824dece..8769775a 100644 --- a/common/lib/driver_dialect/driver_dialect.ts +++ b/common/lib/driver_dialect/driver_dialect.ts @@ -28,5 +28,9 @@ export interface DriverDialect { getAwsPoolClient(props: any): AwsPoolClient; + setConnectTimeout(props: Map, wrapperConnectTimeout?: any): void; + + setQueryTimeout(props: Map, sql?: any, wrapperConnectTimeout?: any): void; + setKeepAliveProperties(props: Map, keepAliveProps: any): void; } diff --git a/common/lib/mysql_client_wrapper.ts b/common/lib/mysql_client_wrapper.ts index d79d3907..0d94ee62 100644 --- a/common/lib/mysql_client_wrapper.ts +++ b/common/lib/mysql_client_wrapper.ts @@ -18,11 +18,13 @@ import { ClientWrapper } from "./client_wrapper"; import { HostInfo } from "./host_info"; import { ClientUtils } from "./utils/client_utils"; import { uniqueId } from "../logutils"; +import { DriverDialect } from "./driver_dialect/driver_dialect"; /* This is an internal wrapper class for the target community driver client created by the MySQL2DriverDialect. */ export class MySQLClientWrapper implements ClientWrapper { + private readonly driverDialect: DriverDialect; readonly client: any; readonly hostInfo: HostInfo; readonly properties: Map; @@ -34,15 +36,18 @@ export class MySQLClientWrapper implements ClientWrapper { * @param targetClient The community driver client created for an instance. * @param hostInfo Host information for the connected instance. * @param properties Connection properties for the target client. + * @param driverDialect The driver dialect to obtain driver-specific information. */ - constructor(targetClient: any, hostInfo: HostInfo, properties: Map) { + constructor(targetClient: any, hostInfo: HostInfo, properties: Map, driverDialect: DriverDialect) { this.client = targetClient; this.hostInfo = hostInfo; this.properties = properties; + this.driverDialect = driverDialect; this.id = uniqueId("MySQLClient_"); } query(sql: any): Promise { + this.driverDialect.setQueryTimeout(this.properties, sql); return this.client?.query(sql); } diff --git a/common/lib/utils/client_utils.ts b/common/lib/utils/client_utils.ts index 12dddff8..10abedf8 100644 --- a/common/lib/utils/client_utils.ts +++ b/common/lib/utils/client_utils.ts @@ -18,15 +18,26 @@ import { getTimeoutTask } from "./utils"; import { Messages } from "./messages"; import { AwsWrapperError, InternalQueryTimeoutError } from "./errors"; import { WrapperProperties } from "../wrapper_property"; +import { logger } from "../../logutils"; export class ClientUtils { + static hasWarnedDeprecation: boolean = false; static async queryWithTimeout(newPromise: Promise, props: Map, timeValue?: number): Promise { const timer: any = {}; - const timeoutTask = getTimeoutTask( - timer, - Messages.get("ClientUtils.queryTaskTimeout"), - timeValue ?? WrapperProperties.INTERNAL_QUERY_TIMEOUT.get(props) - ); + let timeout = timeValue; + if (props.has(WrapperProperties.INTERNAL_QUERY_TIMEOUT.name)) { + timeout = WrapperProperties.INTERNAL_QUERY_TIMEOUT.get(props); + if (!ClientUtils.hasWarnedDeprecation) { + logger.warn( + "The connection configuration property 'mysqlQueryTimeout' is deprecated since version 1.1.0. Please use 'wrapperQueryTimeout' instead." + ); + ClientUtils.hasWarnedDeprecation = true; + } + } + if (!timeout) { + timeout = WrapperProperties.WRAPPER_QUERY_TIMEOUT.get(props); + } + const timeoutTask = getTimeoutTask(timer, Messages.get("ClientUtils.queryTaskTimeout"), timeout); return await Promise.race([timeoutTask, newPromise]) .catch((error: any) => { if (error instanceof InternalQueryTimeoutError) { diff --git a/common/lib/utils/locales/en.json b/common/lib/utils/locales/en.json index 15fe223a..b7e25320 100644 --- a/common/lib/utils/locales/en.json +++ b/common/lib/utils/locales/en.json @@ -5,6 +5,7 @@ "ConnectionProvider.unsupportedHostSelectorStrategy": "Unsupported host selection strategy '%s' specified for this connection provider '%s'. Please visit the documentation for all supported strategies.", "ConnectionPluginChainBuilder.errorImportingPlugin": "The plugin could not be imported due to error '%s'. Please ensure the required dependencies have been installed. Plugin: '%s'", "ClientUtils.queryTaskTimeout": "Client query task timed out, if a network error did not occur, please review the usage of the 'mysqlQueryTimeout' connection parameter.", + "ClientUtils.connectTimeout": "Client connect timed out.", "DialectManager.unknownDialectCode": "Unknown dialect code: '%s'.", "DialectManager.getDialectError": "Was not able to get a database dialect.", "DefaultPlugin.executingMethod": "Executing method: %s", diff --git a/common/lib/wrapper_property.ts b/common/lib/wrapper_property.ts index bc3c4999..9cf54b87 100644 --- a/common/lib/wrapper_property.ts +++ b/common/lib/wrapper_property.ts @@ -71,12 +71,27 @@ export class WrapperProperties { null ); + /** + * @deprecated since version 1.1.0 and replaced by wrapper property WRAPPER_QUERY_TIMEOUT. + */ static readonly INTERNAL_QUERY_TIMEOUT = new WrapperProperty( "mysqlQueryTimeout", "Timeout in milliseconds for the wrapper to execute queries against MySQL database engines", 20000 ); + static readonly WRAPPER_CONNECT_TIMEOUT = new WrapperProperty( + "wrapperConnectTimeout", + "Timeout in milliseconds for the wrapper to create a connection.", + 10000 + ); + + static readonly WRAPPER_QUERY_TIMEOUT = new WrapperProperty( + "wrapperQueryTimeout", + "Timeout in milliseconds for the wrapper to execute queries.", + 20000 + ); + static readonly TRANSFER_SESSION_STATE_ON_SWITCH = new WrapperProperty( "transferSessionStateOnSwitch", "Enables session state transfer to a new connection.", diff --git a/docs/using-the-nodejs-wrapper/UsingTheNodejsWrapper.md b/docs/using-the-nodejs-wrapper/UsingTheNodejsWrapper.md index 1f83ebb1..607fa2e8 100644 --- a/docs/using-the-nodejs-wrapper/UsingTheNodejsWrapper.md +++ b/docs/using-the-nodejs-wrapper/UsingTheNodejsWrapper.md @@ -40,17 +40,19 @@ To enable logging when using the AWS Advanced NodeJS Wrapper, use the `LOG_LEVEL These parameters are applicable to any instance of the AWS Advanced NodeJS Wrapper. -| Parameter | Value | Required | Description | Default Value | -| ------------------------------ | ------------------ | -------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `host` | `string` | No | Database host. | `null` | -| `database` | `string` | No | Database name. | `null` | -| `user` | `string` | No | Database username. | `null` | -| `password` | `string` | No | Database password. | `null` | -| `transferSessionStateOnSwitch` | `boolean` | No | Enables transferring the session state to a new connection. | `true` | -| `resetSessionStateOnClose` | `boolean` | No | Enables resetting the session state before closing connection. | `true` | -| `enableGreenHostReplacement` | `boolean` | No | Enables replacing a green node host name with the original host name when the green host DNS doesn't exist anymore after a blue/green switchover. Refer to [Overview of Amazon RDS Blue/Green Deployments](https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/blue-green-deployments-overview.html) for more details about green and blue nodes. | `false` | -| `clusterInstanceHostPattern` | `string` | If connecting using an IP address or custom domain URL: Yes

Otherwise: No | This parameter is not required unless connecting to an AWS RDS cluster via an IP address or custom domain URL. In those cases, this parameter specifies the cluster instance DNS pattern that will be used to build a complete instance endpoint. A "?" character in this pattern should be used as a placeholder for the DB instance identifiers of the instances in the cluster. See [here](#host-pattern) for more information.

Example: `?.my-domain.com`, `any-subdomain.?.my-domain.com`

Use case Example: If your cluster instance endpoints follow this pattern:`instanceIdentifier1.customHost`, `instanceIdentifier2.customHost`, etc. and you want your initial connection to be to `customHost:1234`, then your client configuration should look like this: `{ host: "customHost", port: 1234, database: "test", clusterInstanceHostPattern: "?.customHost" }` | If the provided host is not an IP address or custom domain, the NodeJS Wrapper will automatically acquire the cluster instance host pattern from the customer-provided host. | -| `mysqlQueryTimeout` | `number` | No | Query timeout in milliseconds. This is only applicable when using the AwsMySQLClient. To set query timeout for the AwsPGClient, please use the built-in `query_timeout` parameter. See the `node-postgres` [documentation](https://node-postgres.com/apis/client) for more details.. | 20000 | +| Parameter | Value | Required | Description | Default Value | Version Supported | +| ------------------------------ | ------------------ | -------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------- | +| `host` | `string` | No | Database host. | `null` | `latest` | +| `database` | `string` | No | Database name. | `null` | `latest` | +| `user` | `string` | No | Database username. | `null` | `latest` | +| `password` | `string` | No | Database password. | `null` | `latest` | +| `transferSessionStateOnSwitch` | `boolean` | No | Enables transferring the session state to a new connection. | `true` | `latest` | +| `resetSessionStateOnClose` | `boolean` | No | Enables resetting the session state before closing connection. | `true` | `latest` | +| `enableGreenHostReplacement` | `boolean` | No | Enables replacing a green node host name with the original host name when the green host DNS doesn't exist anymore after a blue/green switchover. Refer to [Overview of Amazon RDS Blue/Green Deployments](https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/blue-green-deployments-overview.html) for more details about green and blue nodes. | `false` | `latest` | +| `clusterInstanceHostPattern` | `string` | If connecting using an IP address or custom domain URL: Yes

Otherwise: No | This parameter is not required unless connecting to an AWS RDS cluster via an IP address or custom domain URL. In those cases, this parameter specifies the cluster instance DNS pattern that will be used to build a complete instance endpoint. A "?" character in this pattern should be used as a placeholder for the DB instance identifiers of the instances in the cluster. See [here](#host-pattern) for more information.

Example: `?.my-domain.com`, `any-subdomain.?.my-domain.com`

Use case Example: If your cluster instance endpoints follow this pattern:`instanceIdentifier1.customHost`, `instanceIdentifier2.customHost`, etc. and you want your initial connection to be to `customHost:1234`, then your client configuration should look like this: `{ host: "customHost", port: 1234, database: "test", clusterInstanceHostPattern: "?.customHost" }` | If the provided host is not an IP address or custom domain, the NodeJS Wrapper will automatically acquire the cluster instance host pattern from the customer-provided host. | `latest` | +| ~~`mysqlQueryTimeout`~~ | `number` | No | This parameter has been deprecated since version 1.1.0, applications should use the `wrapperQueryTimeout` parameter instead.

Query timeout in milliseconds. This is only applicable when using the AwsMySQLClient. To set query timeout for the AwsPGClient, please use the built-in `query_timeout` parameter. See the `node-postgres` [documentation](https://node-postgres.com/apis/client) for more details. | 20000 | `1.0.0` | +| `wrapperConnectTimeout` | `number` | No | Connect timeout in milliseconds. This parameter will apply the provided timeout value to the underlying driver's built-in connect timeout parameter, if there is one available. | 20000 | `latest` | +| `wrapperQueryTimeout` | `number` | No | Query timeout in milliseconds. This parameter will apply the provided timeout value to the underlying driver's built-in query timeout parameter, if there is one available. The wrapper will also use this value for its own query timeout implementation. | 20000 | `latest` | | `wrapperKeepAliveProperties` | `Map` | No | If the underlying target driver has keepAlive properties available, properties within this map will be applied to the underlying target driver's client configuration. For example, the node-postgres driver's `keepAlive` and `keepAliveInitialDelayMillis` properties can be configured by setting this property in the client configuration: `{ wrapperKeepAliveProperties: new Map([["keepAlive", true], ["keepAliveInitialDelayMillis", 1234]]) }`.

Currently supported drivers: node-postgres | `null` | ## Host Pattern diff --git a/docs/using-the-nodejs-wrapper/using-plugins/UsingTheHostMonitoringPlugin.md b/docs/using-the-nodejs-wrapper/using-plugins/UsingTheHostMonitoringPlugin.md index 30d70f7f..34dec94e 100644 --- a/docs/using-the-nodejs-wrapper/using-plugins/UsingTheHostMonitoringPlugin.md +++ b/docs/using-the-nodejs-wrapper/using-plugins/UsingTheHostMonitoringPlugin.md @@ -51,8 +51,8 @@ The Host Monitoring Connection Plugin may create new monitoring connections to c params = { plugins: "efm", connectTimeout: 1000, - mysqlQueryTimeout: 120000, - monitoring_mysqlQueryTimeout: 1000 + wrapperQueryTimeout: 120000, + monitoring_wrapperQueryTimeout: 1000 }; const client = new AwsMySQLClient(params); diff --git a/mysql/lib/dialect/mysql2_driver_dialect.ts b/mysql/lib/dialect/mysql2_driver_dialect.ts index c82225e1..302094f6 100644 --- a/mysql/lib/dialect/mysql2_driver_dialect.ts +++ b/mysql/lib/dialect/mysql2_driver_dialect.ts @@ -26,6 +26,8 @@ import { HostInfo } from "../../../common/lib/host_info"; import { UnsupportedMethodError } from "../../../common/lib/utils/errors"; export class MySQL2DriverDialect implements DriverDialect { + static readonly connectTimeoutPropertyName = "connectTimeout"; + static readonly queryTimeoutPropertyName = "timeout"; protected dialectName: string = this.constructor.name; getDialectName(): string { @@ -36,8 +38,9 @@ export class MySQL2DriverDialect implements DriverDialect { const driverProperties = WrapperProperties.removeWrapperProperties(props); // MySQL2 does not support keep alive, explicitly check and throw an exception if this value is set. this.setKeepAliveProperties(driverProperties, props.get(WrapperProperties.KEEPALIVE_PROPERTIES.name)); + this.setConnectTimeout(driverProperties, props.get(WrapperProperties.WRAPPER_CONNECT_TIMEOUT.name)); const targetClient = await createConnection(driverProperties); - return Promise.resolve(new MySQLClientWrapper(targetClient, hostInfo, props)); + return Promise.resolve(new MySQLClientWrapper(targetClient, hostInfo, props, this)); } preparePoolClientProperties(props: Map, poolConfig: AwsPoolConfig | undefined): any { @@ -57,6 +60,20 @@ export class MySQL2DriverDialect implements DriverDialect { return new AwsMysqlPoolClient(props); } + setConnectTimeout(props: Map, wrapperConnectTimeout?: any) { + const timeout = wrapperConnectTimeout ?? props.get(WrapperProperties.WRAPPER_CONNECT_TIMEOUT.name); + if (timeout) { + props.set(MySQL2DriverDialect.connectTimeoutPropertyName, timeout); + } + } + + setQueryTimeout(props: Map, sql?: any, wrapperConnectTimeout?: any) { + const timeout = wrapperConnectTimeout ?? props.get(WrapperProperties.WRAPPER_QUERY_TIMEOUT.name); + if (timeout && !sql[MySQL2DriverDialect.queryTimeoutPropertyName]) { + sql[MySQL2DriverDialect.queryTimeoutPropertyName] = timeout; + } + } + setKeepAliveProperties(props: Map, keepAliveProps: any) { if (keepAliveProps) { throw new UnsupportedMethodError("Keep alive configuration is not supported for MySQL2."); diff --git a/pg/lib/dialect/node_postgres_driver_dialect.ts b/pg/lib/dialect/node_postgres_driver_dialect.ts index d604b45b..6f10f1e5 100644 --- a/pg/lib/dialect/node_postgres_driver_dialect.ts +++ b/pg/lib/dialect/node_postgres_driver_dialect.ts @@ -27,6 +27,8 @@ import { PgClientWrapper } from "../../../common/lib/pg_client_wrapper"; import { HostInfo } from "../../../common/lib/host_info"; export class NodePostgresDriverDialect implements DriverDialect { + static readonly connectTimeoutPropertyName = "connectionTimeoutMillis"; + static readonly queryTimeoutPropertyName = "query_timeout"; protected dialectName: string = this.constructor.name; private static keepAlivePropertyName = "keepAlive"; private static keepAliveInitialDelayMillisPropertyName = "keepAliveInitialDelayMillis"; @@ -38,6 +40,8 @@ export class NodePostgresDriverDialect implements DriverDialect { async connect(hostInfo: HostInfo, props: Map): Promise { const driverProperties = WrapperProperties.removeWrapperProperties(props); this.setKeepAliveProperties(driverProperties, props.get(WrapperProperties.KEEPALIVE_PROPERTIES.name)); + this.setConnectTimeout(driverProperties, props.get(WrapperProperties.WRAPPER_CONNECT_TIMEOUT.name)); + this.setQueryTimeout(driverProperties, props.get(WrapperProperties.WRAPPER_QUERY_TIMEOUT.name)); const targetClient = new pkgPg.Client(driverProperties); await targetClient.connect(); return Promise.resolve(new PgClientWrapper(targetClient, hostInfo, props)); @@ -60,6 +64,20 @@ export class NodePostgresDriverDialect implements DriverDialect { return new AwsPgPoolClient(props); } + setConnectTimeout(props: Map, wrapperConnectTimeout?: any) { + const timeout = wrapperConnectTimeout ?? props.get(WrapperProperties.WRAPPER_CONNECT_TIMEOUT.name); + if (timeout) { + props.set(NodePostgresDriverDialect.connectTimeoutPropertyName, timeout); + } + } + + setQueryTimeout(props: Map, sql?: any, wrapperQueryTimeout?: any) { + const timeout = wrapperQueryTimeout ?? props.get(WrapperProperties.WRAPPER_QUERY_TIMEOUT.name); + if (timeout) { + props.set(NodePostgresDriverDialect.queryTimeoutPropertyName, timeout); + } + } + setKeepAliveProperties(props: Map, keepAliveProps: any) { if (!keepAliveProps) { return; diff --git a/tests/integration/container/tests/utils/driver_helper.ts b/tests/integration/container/tests/utils/driver_helper.ts index 0eda2edf..aa02e773 100644 --- a/tests/integration/container/tests/utils/driver_helper.ts +++ b/tests/integration/container/tests/utils/driver_helper.ts @@ -117,8 +117,8 @@ export class DriverHelper { props["monitoring_query_timeout"] = 3000; } else if (engine === DatabaseEngine.MYSQL && performance) { props["connectTimeout"] = 3000; - props["monitoring_mysqlQueryTimeout"] = 3000; - props["mysqlQueryTimeout"] = 120000; + props["monitoring_wrapperQueryTimeout"] = 3000; + props["wrapperQueryTimeout"] = 120000; } return props; }