Skip to content

Commit

Permalink
Handle when a Sub Millisecond Tx Timeout is specified (#1108)
Browse files Browse the repository at this point in the history
If a user sets a timeout with a fractional millisecond value it will be rounded up to the next integer value. e.g

> 0.1 → 1
> 1.4 → 2.0
> 1.8 → 2.0

A warning that a rounding has taken place should be logged under info level.
  • Loading branch information
bigmontz committed Jul 8, 2023
1 parent 33f587d commit dd187bf
Show file tree
Hide file tree
Showing 16 changed files with 453 additions and 40 deletions.
4 changes: 3 additions & 1 deletion packages/core/src/driver.ts
Expand Up @@ -98,6 +98,7 @@ type CreateSession = (args: {
bookmarkManager?: BookmarkManager
notificationFilter?: NotificationFilter
auth?: AuthToken
log: Logger
}) => Session

type CreateQueryExecutor = (createSession: (config: { database?: string, bookmarkManager?: BookmarkManager }) => Session) => QueryExecutor
Expand Down Expand Up @@ -827,7 +828,8 @@ class Driver {
fetchSize,
bookmarkManager,
notificationFilter,
auth
auth,
log: this._log
})
}

Expand Down
7 changes: 6 additions & 1 deletion packages/core/src/integer.ts
Expand Up @@ -908,14 +908,18 @@ class Integer {
* @param {!Integer|number|string|bigint|!{low: number, high: number}} val Value
* @param {Object} [opts={}] Configuration options
* @param {boolean} [opts.strictStringValidation=false] Enable strict validation generated Integer.
* @param {boolean} [opts.ceilFloat=false] Enable round up float to the nearest Integer.
* @returns {!Integer}
* @expose
*/
static fromValue (val: Integerable, opts: { strictStringValidation?: boolean} = {}): Integer {
static fromValue (val: Integerable, opts: { strictStringValidation?: boolean, ceilFloat?: boolean } = {}): Integer {
if (val /* is compatible */ instanceof Integer) {
return val
}
if (typeof val === 'number') {
if (opts.ceilFloat === true) {
val = Math.ceil(val)
}
return Integer.fromNumber(val)
}
if (typeof val === 'string') {
Expand Down Expand Up @@ -1066,6 +1070,7 @@ const TWO_PWR_24 = Integer.fromInt(TWO_PWR_24_DBL)
* @param {Mixed} value - The value to use.
* @param {Object} [opts={}] Configuration options
* @param {boolean} [opts.strictStringValidation=false] Enable strict validation generated Integer.
* @param {boolean} [opts.ceilFloat=false] Enable round up float to the nearest Integer.
* @return {Integer} - An object of type Integer.
*/
const int = Integer.fromValue
Expand Down
17 changes: 13 additions & 4 deletions packages/core/src/internal/tx-config.ts
Expand Up @@ -20,6 +20,7 @@
import * as util from './util'
import { newError } from '../error'
import Integer, { int } from '../integer'
import { Logger } from './logger'

/**
* Internal holder of the transaction configuration.
Expand All @@ -35,9 +36,9 @@ export class TxConfig {
* @constructor
* @param {Object} config the raw configuration object.
*/
constructor (config: any) {
constructor (config: any, log?: Logger) {
assertValidConfig(config)
this.timeout = extractTimeout(config)
this.timeout = extractTimeout(config, log)
this.metadata = extractMetadata(config)
}

Expand All @@ -63,10 +64,14 @@ const EMPTY_CONFIG = new TxConfig({})
/**
* @return {Integer|null}
*/
function extractTimeout (config: any): Integer | null {
function extractTimeout (config: any, log?: Logger): Integer | null {
if (util.isObject(config) && config.timeout != null) {
util.assertNumberOrInteger(config.timeout, 'Transaction timeout')
const timeout = int(config.timeout)
if (isTimeoutFloat(config) && log?.isInfoEnabled() === true) {
// eslint-disable-next-line @typescript-eslint/restrict-template-expressions
log?.info(`Transaction timeout expected to be an integer, got: ${config.timeout}. The value will be rounded up.`)
}
const timeout = int(config.timeout, { ceilFloat: true })
if (timeout.isNegative()) {
throw newError('Transaction timeout should not be negative')
}
Expand All @@ -75,6 +80,10 @@ function extractTimeout (config: any): Integer | null {
return null
}

function isTimeoutFloat (config: any): boolean {
return typeof config.timeout === 'number' && !Number.isInteger(config.timeout)
}

/**
* @return {object|null}
*/
Expand Down
19 changes: 12 additions & 7 deletions packages/core/src/session.ts
Expand Up @@ -38,6 +38,7 @@ import ManagedTransaction from './transaction-managed'
import BookmarkManager from './bookmark-manager'
import { Dict } from './record'
import NotificationFilter from './notification-filter'
import { Logger } from './internal/logger'

type ConnectionConsumer = (connection: Connection | null) => any | undefined | Promise<any> | Promise<undefined>
type TransactionWork<T> = (tx: Transaction) => Promise<T> | T
Expand Down Expand Up @@ -74,6 +75,7 @@ class Session {
private readonly _results: Result[]
private readonly _bookmarkManager?: BookmarkManager
private readonly _notificationFilter?: NotificationFilter
private readonly _log?: Logger
/**
* @constructor
* @protected
Expand All @@ -100,7 +102,8 @@ class Session {
impersonatedUser,
bookmarkManager,
notificationFilter,
auth
auth,
log
}: {
mode: SessionMode
connectionProvider: ConnectionProvider
Expand All @@ -113,6 +116,7 @@ class Session {
bookmarkManager?: BookmarkManager
notificationFilter?: NotificationFilter
auth?: AuthToken
log: Logger
}) {
this._mode = mode
this._database = database
Expand Down Expand Up @@ -153,6 +157,7 @@ class Session {
this._results = []
this._bookmarkManager = bookmarkManager
this._notificationFilter = notificationFilter
this._log = log
}

/**
Expand All @@ -176,7 +181,7 @@ class Session {
parameters
)
const autoCommitTxConfig = (transactionConfig != null)
? new TxConfig(transactionConfig)
? new TxConfig(transactionConfig, this._log)
: TxConfig.empty()

const result = this._run(validatedQuery, params, async connection => {
Expand Down Expand Up @@ -279,7 +284,7 @@ class Session {

let txConfig = TxConfig.empty()
if (arg != null) {
txConfig = new TxConfig(arg)
txConfig = new TxConfig(arg, this._log)
}

return this._beginTransaction(this._mode, txConfig)
Expand Down Expand Up @@ -385,7 +390,7 @@ class Session {
transactionWork: TransactionWork<T>,
transactionConfig?: TransactionConfig
): Promise<T> {
const config = new TxConfig(transactionConfig)
const config = new TxConfig(transactionConfig, this._log)
return this._runTransaction(ACCESS_MODE_READ, config, transactionWork)
}

Expand All @@ -410,7 +415,7 @@ class Session {
transactionWork: TransactionWork<T>,
transactionConfig?: TransactionConfig
): Promise<T> {
const config = new TxConfig(transactionConfig)
const config = new TxConfig(transactionConfig, this._log)
return this._runTransaction(ACCESS_MODE_WRITE, config, transactionWork)
}

Expand Down Expand Up @@ -443,7 +448,7 @@ class Session {
transactionWork: ManagedTransactionWork<T>,
transactionConfig?: TransactionConfig
): Promise<T> {
const config = new TxConfig(transactionConfig)
const config = new TxConfig(transactionConfig, this._log)
return this._executeInTransaction(ACCESS_MODE_READ, config, transactionWork)
}

Expand All @@ -465,7 +470,7 @@ class Session {
transactionWork: ManagedTransactionWork<T>,
transactionConfig?: TransactionConfig
): Promise<T> {
const config = new TxConfig(transactionConfig)
const config = new TxConfig(transactionConfig, this._log)
return this._executeInTransaction(ACCESS_MODE_WRITE, config, transactionWork)
}

Expand Down
2 changes: 2 additions & 0 deletions packages/core/test/driver.test.ts
Expand Up @@ -625,6 +625,8 @@ describe('Driver', () => {
mode: 'WRITE',
reactive: false,
impersonatedUser: undefined,
// @ts-expect-error
log: driver?._log,
...extra
}
}
Expand Down

0 comments on commit dd187bf

Please sign in to comment.