From 1cee4e3fe4f58a6c162c35872176f6f3568bce1f Mon Sep 17 00:00:00 2001 From: Dave Gramlich Date: Tue, 16 Oct 2018 09:40:25 -0400 Subject: [PATCH 1/2] feat(session): add label support --- src/database.js | 6 ++++++ src/session-pool.js | 10 +++++++--- src/session.js | 10 ++++++++-- test/database.js | 11 +++++++++++ test/session-pool.js | 15 ++++++++++++++- test/session.js | 35 +++++++++++++++++++++++++++++------ 6 files changed, 75 insertions(+), 12 deletions(-) diff --git a/src/database.js b/src/database.js index 73b8d7030..c361cd95d 100644 --- a/src/database.js +++ b/src/database.js @@ -348,6 +348,12 @@ class Database extends ServiceObject { const reqOpts = { database: this.formattedName_, }; + + if (options.labels) { + reqOpts.session = {labels: options.labels}; + delete options.labels; + } + this.request( { client: 'SpannerClient', diff --git a/src/session-pool.js b/src/session-pool.js index f9a4f363a..237c2254f 100644 --- a/src/session-pool.js +++ b/src/session-pool.js @@ -31,6 +31,7 @@ const DEFAULTS = { fail: false, idlesAfter: 10, keepAlive: 30, + labels: {}, max: 100, maxIdle: 1, min: 0, @@ -109,14 +110,16 @@ class WritePercentError extends TypeError { * there are no available sessions for a request. * @property {number} [idlesAfter=10] How long until a resource becomes idle, in * minutes. + * @property {number} [keepAlive=50] How often to ping idle sessions, in + * minutes. Must be less than 1 hour. + * @property {Object} [labels] Labels to apply to any session + * created by the pool. * @property {number} [max=100] Maximum number of resources to create at any * given time. * @property {number} [maxIdle=1] Maximum number of idle resources to keep in * the pool at any given time. * @property {number} [min=0] Minimum number of resources to keep in the pool at * any given time. - * @property {number} [keepAlive=50] How often to ping idle sessions, in - * minutes. Must be less than 1 hour. * @property {number} [writes=0.0] Percentage of sessions to be pre-allocated as * write sessions represented as a float. */ @@ -481,12 +484,13 @@ class SessionPool extends EventEmitter { */ _createSession(type) { const session = this.database.session(); + const labels = this.options.labels; this._inventory.borrowed.add(session); return this._requests .add(() => { - return session.create().then(() => { + return session.create({labels}).then(() => { if (type === READWRITE) { return this._prepareTransaction(session).catch( () => (type = READONLY) diff --git a/src/session.js b/src/session.js index f3fb3571f..40061e0d1 100644 --- a/src/session.js +++ b/src/session.js @@ -172,12 +172,18 @@ class Session extends ServiceObject { */ id: name, methods: methods, - createMethod: (options, callback) => { - database.createSession(options, (err, session, apiResponse) => { + createMethod: (_, options, callback) => { + if (is.fn(options)) { + callback = options; + options = {}; + } + + return database.createSession(options, (err, session, apiResponse) => { if (err) { callback(err, null, apiResponse); return; } + extend(this, session); callback(null, this, apiResponse); }); diff --git a/test/database.js b/test/database.js index 5d4f218f4..61607cc3e 100644 --- a/test/database.js +++ b/test/database.js @@ -1483,6 +1483,17 @@ describe('Database', () => { database.createSession(assert.ifError); }); + it('should send labels correctly', done => { + const labels = {a: 'b'}; + + database.request = function(config) { + assert.deepStrictEqual(config.reqOpts.session, {labels}); + done(); + }; + + database.createSession({labels}, assert.ifError); + }); + describe('error', () => { const ERROR = new Error('Error.'); const API_RESPONSE = {}; diff --git a/test/session-pool.js b/test/session-pool.js index 60a38c3f7..72e7da89c 100644 --- a/test/session-pool.js +++ b/test/session-pool.js @@ -49,8 +49,9 @@ class FakeSession { beginTransaction(options) { return new FakeTransaction(options).begin(); } - create() { + create(options) { this.created = true; + this.createOptions = options; return Promise.resolve(); } delete() { @@ -235,6 +236,7 @@ describe('SessionPool', () => { assert.strictEqual(sessionPool.options.fail, false); assert.strictEqual(sessionPool.options.idlesAfter, 10); assert.strictEqual(sessionPool.options.keepAlive, 30); + assert.deepStrictEqual(sessionPool.options.labels, {}); assert.strictEqual(sessionPool.options.max, 100); assert.strictEqual(sessionPool.options.maxIdle, 1); assert.strictEqual(sessionPool.options.min, 0); @@ -866,6 +868,17 @@ describe('SessionPool', () => { }); }); + it('should pass along the session labels', () => { + const labels = {a: 'b'}; + + sessionPool.options.labels = labels; + + return sessionPool._createSession('readonly').then(() => { + const session = sessionPool._inventory.readonly[0]; + assert.deepStrictEqual(session.createOptions, {labels}); + }); + }); + it('should discard the session if unable to create', () => { const error = new Error('err'); diff --git a/test/session.js b/test/session.js index 1b1a8964c..a13f2da76 100644 --- a/test/session.js +++ b/test/session.js @@ -135,15 +135,38 @@ describe('Session', () => { const session = new Session(databaseInstance, NAME); assert(session instanceof FakeGrpcServiceObject); - session.calledWith_[0].createMethod(options, (err, sess, resp) => { - assert.ifError(err); + session.calledWith_[0].createMethod( + null, + options, + (err, sess, resp) => { + assert.ifError(err); - assert.strictEqual(sess, session); + assert.strictEqual(sess, session); - assert.strictEqual(session.uniqueProperty, true); + assert.strictEqual(session.uniqueProperty, true); - assert.strictEqual(resp, apiResponse); + assert.strictEqual(resp, apiResponse); + + done(); + } + ); + }); + it('should check for options', done => { + const databaseInstance = extend({}, DATABASE, { + createSession: function(options, callback) { + assert.deepStrictEqual(options, {}); + callback(null, {}, apiResponse); + }, + }); + + const session = new Session(databaseInstance, NAME); + const apiResponse = {}; + + session.calledWith_[0].createMethod(null, (err, sess, resp) => { + assert.ifError(err); + assert.strictEqual(sess, session); + assert.strictEqual(resp, apiResponse); done(); }); }); @@ -161,7 +184,7 @@ describe('Session', () => { const session = new Session(databaseInstance, NAME); assert(session instanceof FakeGrpcServiceObject); - session.calledWith_[0].createMethod({}, (err, sess, resp) => { + session.calledWith_[0].createMethod(null, {}, (err, sess, resp) => { assert.strictEqual(err, error); assert.strictEqual(sess, null); assert.strictEqual(resp, apiResponse); From fdae3ebe1aebecdc5a7b14c5b50f4a3b5717eca6 Mon Sep 17 00:00:00 2001 From: Dave Gramlich Date: Tue, 16 Oct 2018 14:52:12 -0400 Subject: [PATCH 2/2] preserve user options --- src/database.js | 11 ++++++----- test/database.js | 5 ++++- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/database.js b/src/database.js index c361cd95d..41a4631c4 100644 --- a/src/database.js +++ b/src/database.js @@ -344,14 +344,15 @@ class Database extends ServiceObject { callback = options; options = {}; } - options = options || {}; + + const gaxOpts = extend({}, options); const reqOpts = { database: this.formattedName_, }; - if (options.labels) { - reqOpts.session = {labels: options.labels}; - delete options.labels; + if (gaxOpts.labels) { + reqOpts.session = {labels: gaxOpts.labels}; + delete gaxOpts.labels; } this.request( @@ -359,7 +360,7 @@ class Database extends ServiceObject { client: 'SpannerClient', method: 'createSession', reqOpts: reqOpts, - gaxOpts: options, + gaxOpts: gaxOpts, }, (err, resp) => { if (err) { diff --git a/test/database.js b/test/database.js index 61607cc3e..d8b4c0229 100644 --- a/test/database.js +++ b/test/database.js @@ -1461,7 +1461,7 @@ describe('Database', () => { database: database.formattedName_, }); - assert.strictEqual(config.gaxOpts, OPTIONS); + assert.deepStrictEqual(config.gaxOpts, OPTIONS); done(); }; @@ -1485,9 +1485,12 @@ describe('Database', () => { it('should send labels correctly', done => { const labels = {a: 'b'}; + const options = {a: 'b', labels}; + const originalOptions = extend(true, {}, options); database.request = function(config) { assert.deepStrictEqual(config.reqOpts.session, {labels}); + assert.deepStrictEqual(options, originalOptions); done(); };