From e087a7fa7cf1e9299a6d2a6c50c01f1bc2f92a7e Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Fri, 1 Apr 2022 14:25:52 -0400 Subject: [PATCH 1/7] feat: enable channel pooling --- package.json | 1 + src/bigtable_grpc_config.json | 6 ++++++ src/index.ts | 11 ++++++++++- 3 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 src/bigtable_grpc_config.json diff --git a/package.json b/package.json index 298f6f6e6..bdca551e5 100644 --- a/package.json +++ b/package.json @@ -54,6 +54,7 @@ "escape-string-regexp": "^4.0.0", "extend": "^3.0.2", "google-gax": "^2.29.5", + "grpc-gcp": "^0.3.3", "is": "^3.0.1", "is-utf8": "^0.2.1", "lodash.snakecase": "^4.1.1", diff --git a/src/bigtable_grpc_config.json b/src/bigtable_grpc_config.json new file mode 100644 index 000000000..a55e17636 --- /dev/null +++ b/src/bigtable_grpc_config.json @@ -0,0 +1,6 @@ +{ + "channelPool": { + "maxSize": 10, + "maxConcurrentStreamsLowWatermark": 30 + } + } \ No newline at end of file diff --git a/src/index.ts b/src/index.ts index 506c072ea..25e844ceb 100644 --- a/src/index.ts +++ b/src/index.ts @@ -16,7 +16,7 @@ import {replaceProjectIdToken} from '@google-cloud/projectify'; import {promisifyAll} from '@google-cloud/promisify'; import arrify = require('arrify'); import * as extend from 'extend'; -import {GoogleAuth, CallOptions} from 'google-gax'; +import {GoogleAuth, CallOptions, grpc as gaxVendoredGrpc} from 'google-gax'; import * as gax from 'google-gax'; import * as protos from '../protos/protos'; @@ -33,6 +33,7 @@ import {google} from '../protos/protos'; import {ServiceError} from 'google-gax'; import * as v2 from './v2'; import {PassThrough, Duplex} from 'stream'; +import grpcGcpModule = require('grpc-gcp'); // eslint-disable-next-line @typescript-eslint/no-var-requires const streamEvents = require('stream-events'); @@ -42,6 +43,11 @@ const PKG = require('../../package.json'); const {grpc} = new gax.GrpcClient(); +// Enable channel pooling +const grpcGcp = grpcGcpModule(gaxVendoredGrpc); +// eslint-disable-next-line @typescript-eslint/no-var-requires +const gcpApiConfig = require('../../src/bigtable_grpc_config.json'); + export interface GetInstancesCallback { ( err: ServiceError | null, @@ -415,6 +421,9 @@ export class Bigtable { scopes, 'grpc.keepalive_time_ms': 30000, 'grpc.keepalive_timeout_ms': 10000, + 'grpc.callInvocationTransformer': grpcGcp.gcpCallInvocationTransformer, + 'grpc.channelFactoryOverride': grpcGcp.gcpChannelFactoryOverride, + 'grpc.gcpApiConfig': grpcGcp.createGcpApiConfig(gcpApiConfig), }, options ); From 08479ac733b9d0b1682b376f1ffcfe4fb863185a Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Fri, 1 Apr 2022 18:23:51 -0400 Subject: [PATCH 2/7] fix unit tests --- test/index.ts | 44 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 38 insertions(+), 6 deletions(-) diff --git a/test/index.ts b/test/index.ts index 147a76001..76d28e20b 100644 --- a/test/index.ts +++ b/test/index.ts @@ -190,6 +190,8 @@ describe('Bigtable', () => { assert.deepStrictEqual( options_, Object.assign( + {}, + options_, { libName: 'gccl', libVersion: PKG.version, @@ -233,18 +235,24 @@ describe('Bigtable', () => { assert.deepStrictEqual(bigtable.options, { BigtableClient: Object.assign( + {}, + bigtable.options['BigtableClient'], { servicePath: 'bigtable.googleapis.com', }, expectedOptions ), BigtableInstanceAdminClient: Object.assign( + {}, + bigtable.options['BigtableInstanceAdminClient'], { servicePath: 'bigtableadmin.googleapis.com', }, expectedOptions ), BigtableTableAdminClient: Object.assign( + {}, + bigtable.options['BigtableTableAdminClient'], { servicePath: 'bigtableadmin.googleapis.com', }, @@ -283,9 +291,21 @@ describe('Bigtable', () => { ); assert.deepStrictEqual(bigtable.options, { - BigtableClient: expectedOptions, - BigtableInstanceAdminClient: expectedOptions, - BigtableTableAdminClient: expectedOptions, + BigtableClient: Object.assign( + {}, + bigtable.options['BigtableClient'], + expectedOptions + ), + BigtableInstanceAdminClient: Object.assign( + {}, + bigtable.options['BigtableInstanceAdminClient'], + expectedOptions + ), + BigtableTableAdminClient: Object.assign( + {}, + bigtable.options['BigtableTableAdminClient'], + expectedOptions + ), }); }); @@ -315,9 +335,21 @@ describe('Bigtable', () => { assert.strictEqual(bigtable.customEndpoint, options.apiEndpoint); assert.deepStrictEqual(bigtable.options, { - BigtableClient: expectedOptions, - BigtableInstanceAdminClient: expectedOptions, - BigtableTableAdminClient: expectedOptions, + BigtableClient: Object.assign( + {}, + bigtable.options['BigtableClient'], + expectedOptions + ), + BigtableInstanceAdminClient: Object.assign( + {}, + bigtable.options['BigtableInstanceAdminClient'], + expectedOptions + ), + BigtableTableAdminClient: Object.assign( + {}, + bigtable.options['BigtableTableAdminClient'], + expectedOptions + ), }); }); From d445b2ba7e55b316bfe7ed778f5f7f9919cfb0fa Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Wed, 6 Apr 2022 13:45:54 -0400 Subject: [PATCH 3/7] bump grpc-gcp version and tweak the channel pool config --- package.json | 2 +- src/bigtable_grpc_config.json | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index bdca551e5..46485426e 100644 --- a/package.json +++ b/package.json @@ -54,7 +54,7 @@ "escape-string-regexp": "^4.0.0", "extend": "^3.0.2", "google-gax": "^2.29.5", - "grpc-gcp": "^0.3.3", + "grpc-gcp": "0.4.1", "is": "^3.0.1", "is-utf8": "^0.2.1", "lodash.snakecase": "^4.1.1", diff --git a/src/bigtable_grpc_config.json b/src/bigtable_grpc_config.json index a55e17636..6cd62379e 100644 --- a/src/bigtable_grpc_config.json +++ b/src/bigtable_grpc_config.json @@ -1,6 +1,8 @@ { "channelPool": { - "maxSize": 10, - "maxConcurrentStreamsLowWatermark": 30 + "minxSize": 2, + "maxSize": 4, + "maxConcurrentStreamsLowWatermark": 10, + "debugHeaderIntervalSecs": 600 } - } \ No newline at end of file + } From aae76c396e3c4420857629eb114c1894b65dbecb Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Wed, 6 Apr 2022 15:49:01 -0400 Subject: [PATCH 4/7] inline json to avoid compliants from pack-n-play. Also only install the channel pool on data connection. And improve error reporting for pack n play --- src/bigtable_grpc_config.json | 8 --- src/index.ts | 94 ++++++++++++++++------------------- system-test/install.ts | 10 +++- 3 files changed, 53 insertions(+), 59 deletions(-) delete mode 100644 src/bigtable_grpc_config.json diff --git a/src/bigtable_grpc_config.json b/src/bigtable_grpc_config.json deleted file mode 100644 index 6cd62379e..000000000 --- a/src/bigtable_grpc_config.json +++ /dev/null @@ -1,8 +0,0 @@ -{ - "channelPool": { - "minxSize": 2, - "maxSize": 4, - "maxConcurrentStreamsLowWatermark": 10, - "debugHeaderIntervalSecs": 600 - } - } diff --git a/src/index.ts b/src/index.ts index 25e844ceb..d0fa67171 100644 --- a/src/index.ts +++ b/src/index.ts @@ -34,6 +34,7 @@ import {ServiceError} from 'google-gax'; import * as v2 from './v2'; import {PassThrough, Duplex} from 'stream'; import grpcGcpModule = require('grpc-gcp'); +import base = Mocha.reporters.base; // eslint-disable-next-line @typescript-eslint/no-var-requires const streamEvents = require('stream-events'); @@ -45,8 +46,6 @@ const {grpc} = new gax.GrpcClient(); // Enable channel pooling const grpcGcp = grpcGcpModule(gaxVendoredGrpc); -// eslint-disable-next-line @typescript-eslint/no-var-requires -const gcpApiConfig = require('../../src/bigtable_grpc_config.json'); export interface GetInstancesCallback { ( @@ -414,20 +413,6 @@ export class Bigtable { } } - options = Object.assign( - { - libName: 'gccl', - libVersion: PKG.version, - scopes, - 'grpc.keepalive_time_ms': 30000, - 'grpc.keepalive_timeout_ms': 10000, - 'grpc.callInvocationTransformer': grpcGcp.gcpCallInvocationTransformer, - 'grpc.channelFactoryOverride': grpcGcp.gcpChannelFactoryOverride, - 'grpc.gcpApiConfig': grpcGcp.createGcpApiConfig(gcpApiConfig), - }, - options - ); - const defaultBaseUrl = 'bigtable.googleapis.com'; const defaultAdminBaseUrl = 'bigtableadmin.googleapis.com'; @@ -437,48 +422,57 @@ export class Bigtable { let customEndpointBaseUrl; let customEndpointPort; + let sslCreds; if (customEndpoint) { const customEndpointParts = customEndpoint.split(':'); customEndpointBaseUrl = customEndpointParts[0]; customEndpointPort = customEndpointParts[1]; + sslCreds = grpc.credentials.createInsecure() } + const baseOptions = Object.assign({ + libName: 'gccl', + libVersion: PKG.version, + port: customEndpointPort || 443, + sslCreds, + scopes, + 'grpc.keepalive_time_ms': 30000, + 'grpc.keepalive_timeout_ms': 10000, + }) as gax.ClientOptions; + + const dataOptions = Object.assign( + {}, + baseOptions, + { + servicePath: customEndpointBaseUrl || defaultBaseUrl, + 'grpc.callInvocationTransformer': grpcGcp.gcpCallInvocationTransformer, + 'grpc.channelFactoryOverride': grpcGcp.gcpChannelFactoryOverride, + 'grpc.gcpApiConfig': grpcGcp.createGcpApiConfig({ + channelPool: { + minxSize: 2, + maxSize: 4, + maxConcurrentStreamsLowWatermark: 10, + debugHeaderIntervalSecs: 600, + }, + }), + }, + options + ) as gax.ClientOptions; + + const adminOptions = Object.assign( + {}, + baseOptions, + { + servicePath: customEndpointBaseUrl || defaultAdminBaseUrl, + }, + options + ); + this.options = { - BigtableClient: Object.assign( - { - servicePath: customEndpoint ? customEndpointBaseUrl : defaultBaseUrl, - port: customEndpoint ? Number(customEndpointPort) : 443, - sslCreds: customEndpoint - ? grpc.credentials.createInsecure() - : undefined, - }, - options - ) as gax.ClientOptions, - BigtableInstanceAdminClient: Object.assign( - { - servicePath: customEndpoint - ? customEndpointBaseUrl - : defaultAdminBaseUrl, - port: customEndpoint ? Number(customEndpointPort) : 443, - sslCreds: customEndpoint - ? grpc.credentials.createInsecure() - : undefined, - }, - options - ) as gax.ClientOptions, - BigtableTableAdminClient: Object.assign( - { - servicePath: customEndpoint - ? customEndpointBaseUrl - : defaultAdminBaseUrl, - port: customEndpoint ? Number(customEndpointPort) : 443, - sslCreds: customEndpoint - ? grpc.credentials.createInsecure() - : undefined, - }, - options - ) as gax.ClientOptions, + BigtableClient: dataOptions, + BigtableInstanceAdminClient: adminOptions, + BigtableTableAdminClient: adminOptions, }; this.api = {}; diff --git a/system-test/install.ts b/system-test/install.ts index 6dd1eaada..6287e717a 100644 --- a/system-test/install.ts +++ b/system-test/install.ts @@ -46,6 +46,14 @@ describe('📦 pack-n-play test', () => { ).toString(), }, }; - await packNTest(options); + try { + await packNTest(options); + } catch (e) { + // all of the actionable information is on the output attribute + if (e.output) { + e.message += "output: " + e.output; + } + throw e; + } }); }); From adecd6d063fb8f0bfc161f88585e923ff04f46ef Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Wed, 6 Apr 2022 16:12:26 -0400 Subject: [PATCH 5/7] fixes --- src/index.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/index.ts b/src/index.ts index d0fa67171..8004eb572 100644 --- a/src/index.ts +++ b/src/index.ts @@ -427,8 +427,8 @@ export class Bigtable { if (customEndpoint) { const customEndpointParts = customEndpoint.split(':'); customEndpointBaseUrl = customEndpointParts[0]; - customEndpointPort = customEndpointParts[1]; - sslCreds = grpc.credentials.createInsecure() + customEndpointPort = Number(customEndpointParts[1]); + sslCreds = grpc.credentials.createInsecure(); } const baseOptions = Object.assign({ @@ -476,7 +476,7 @@ export class Bigtable { }; this.api = {}; - this.auth = new GoogleAuth(options); + this.auth = new GoogleAuth(Object.assign({}, baseOptions, options)); this.projectId = options.projectId || '{{projectId}}'; this.appProfileId = options.appProfileId; this.projectName = `projects/${this.projectId}`; From 1a65d80a550504710ddc03410101c9ec1cfe4a5c Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Wed, 6 Apr 2022 16:20:37 -0400 Subject: [PATCH 6/7] typo --- src/index.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/index.ts b/src/index.ts index 8004eb572..eb4b2b8d6 100644 --- a/src/index.ts +++ b/src/index.ts @@ -34,7 +34,6 @@ import {ServiceError} from 'google-gax'; import * as v2 from './v2'; import {PassThrough, Duplex} from 'stream'; import grpcGcpModule = require('grpc-gcp'); -import base = Mocha.reporters.base; // eslint-disable-next-line @typescript-eslint/no-var-requires const streamEvents = require('stream-events'); From abaaf94d13beeb1b241244b3c25d93c06b8a341f Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Wed, 6 Apr 2022 16:23:20 -0400 Subject: [PATCH 7/7] style --- src/index.ts | 2 +- system-test/install.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/index.ts b/src/index.ts index eb4b2b8d6..6dc3a892f 100644 --- a/src/index.ts +++ b/src/index.ts @@ -449,7 +449,7 @@ export class Bigtable { 'grpc.channelFactoryOverride': grpcGcp.gcpChannelFactoryOverride, 'grpc.gcpApiConfig': grpcGcp.createGcpApiConfig({ channelPool: { - minxSize: 2, + minSize: 2, maxSize: 4, maxConcurrentStreamsLowWatermark: 10, debugHeaderIntervalSecs: 600, diff --git a/system-test/install.ts b/system-test/install.ts index 6287e717a..5ff2e2ad9 100644 --- a/system-test/install.ts +++ b/system-test/install.ts @@ -51,7 +51,7 @@ describe('📦 pack-n-play test', () => { } catch (e) { // all of the actionable information is on the output attribute if (e.output) { - e.message += "output: " + e.output; + e.message += 'output: ' + e.output; } throw e; }