From 797387e95d0af5a3f3ee060fe3173a791e90a80c Mon Sep 17 00:00:00 2001 From: Dominique Emond Date: Thu, 1 Aug 2019 16:54:44 -0400 Subject: [PATCH 1/4] fix: prevent max listeners warning If establishing a database connection is slow and database migration runs and there are many models, sql operations are queued up and this leads to the node.js max emitters exceeded warning. A default value for max emitters has now been introduced, and it can also be configured in datasources.json. Signed-off-by: Dominique Emond --- lib/datasource.js | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/lib/datasource.js b/lib/datasource.js index 6b6fe32e8..e47559ff6 100644 --- a/lib/datasource.js +++ b/lib/datasource.js @@ -207,6 +207,11 @@ DataSource.prototype._setupConnector = function() { this.connector.dataSource = this; } const dataSource = this; + + // Set max listeners to a default/configured value + // solves bug https://github.com/strongloop/loopback-next/issues/2198 + dataSource.setMaxListeners(dataSource.getMaxOfflineRequests()); + this.connector.log = function(query, start) { dataSource.log(query, start); }; @@ -2699,6 +2704,22 @@ DataSource.prototype.beginTransaction = function(options) { return Transaction.begin(this.connector, options); }; +/** + * Get the maximum number of event listeners + */ +DataSource.prototype.getMaxOfflineRequests = function() { + // Set max listeners to a default value + // Override this default value with a datasource setting + // 'maxOfflineRequests' from an application's datasources.json + // solves bug https://github.com/strongloop/loopback-next/issues/2198 + let maxOfflineRequests = 16; + if (this.settings && this.settings.maxOfflineRequests) { + if (this.settings.maxOfflineRequests > maxOfflineRequests) + maxOfflineRequests = this.settings.maxOfflineRequests; + } + return maxOfflineRequests; +}; + /*! The hidden property call is too expensive so it is not used that much */ /** From fc10a115ab9c87db4fa532043b0e3966f847a69f Mon Sep 17 00:00:00 2001 From: Nora Date: Fri, 2 Aug 2019 17:01:28 -0400 Subject: [PATCH 2/4] test: add test for getMaxOfflineRequests --- lib/datasource.js | 16 ++++++++++++---- test/datasource.test.js | 24 ++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/lib/datasource.js b/lib/datasource.js index e47559ff6..f5f647f29 100644 --- a/lib/datasource.js +++ b/lib/datasource.js @@ -194,6 +194,11 @@ util.inherits(DataSource, EventEmitter); // allow child classes to supply a data access object DataSource.DataAccessObject = DataAccessObject; +/** + * Global maximum number of event listeners + */ +DataSource.prototype.DEFAULT_MAX_OFFLINE_REQUESTS = 16; + /** * Set up the connector instance for backward compatibility with JugglingDB schema/adapter * @private @@ -209,7 +214,6 @@ DataSource.prototype._setupConnector = function() { const dataSource = this; // Set max listeners to a default/configured value - // solves bug https://github.com/strongloop/loopback-next/issues/2198 dataSource.setMaxListeners(dataSource.getMaxOfflineRequests()); this.connector.log = function(query, start) { @@ -2711,9 +2715,13 @@ DataSource.prototype.getMaxOfflineRequests = function() { // Set max listeners to a default value // Override this default value with a datasource setting // 'maxOfflineRequests' from an application's datasources.json - // solves bug https://github.com/strongloop/loopback-next/issues/2198 - let maxOfflineRequests = 16; - if (this.settings && this.settings.maxOfflineRequests) { + + let maxOfflineRequests = this.DEFAULT_MAX_OFFLINE_REQUESTS; + if ( + this.settings && + this.settings.maxOfflineRequests && + typeof this.settings.maxOfflineRequests === 'number' + ) { if (this.settings.maxOfflineRequests > maxOfflineRequests) maxOfflineRequests = this.settings.maxOfflineRequests; } diff --git a/test/datasource.test.js b/test/datasource.test.js index a3795cd66..ae158506c 100644 --- a/test/datasource.test.js +++ b/test/datasource.test.js @@ -545,6 +545,30 @@ describe('DataSource', function() { ds.connector.should.equal(connector); }); }); + + describe('getMaxOfflineRequests', () => { + let ds; + beforeEach(() => ds = new DataSource('ds', {connector: 'memory'})); + + it('sets the default maximum number of event listeners to 16', () => { + ds.getMaxOfflineRequests().should.be.eql(16); + }); + + it('uses the default max number of event listeners if the provided one is less than 16', () => { + ds.settings.maxOfflineRequests = 15; + ds.getMaxOfflineRequests().should.be.eql(16); + }); + + it('uses provided number of listeners if it is greater than 16', () => { + ds.settings.maxOfflineRequests = 17; + ds.getMaxOfflineRequests().should.be.eql(17); + }); + + it('uses default if a non-number is provided for the max number of listeners', () => { + ds.settings.maxOfflineRequests = '17'; + ds.getMaxOfflineRequests().should.be.eql(16); + }); + }); }); function givenMockConnector(props) { From b14875da2a24b81821135075d4753d5e1092bef5 Mon Sep 17 00:00:00 2001 From: Nora Date: Wed, 7 Aug 2019 10:07:56 -0400 Subject: [PATCH 3/4] fixup! apply feedback --- lib/datasource.js | 16 +++++++++------- test/datasource.test.js | 11 +++++++---- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/lib/datasource.js b/lib/datasource.js index f5f647f29..e2d53ebed 100644 --- a/lib/datasource.js +++ b/lib/datasource.js @@ -197,7 +197,7 @@ DataSource.DataAccessObject = DataAccessObject; /** * Global maximum number of event listeners */ -DataSource.prototype.DEFAULT_MAX_OFFLINE_REQUESTS = 16; +DataSource.DEFAULT_MAX_OFFLINE_REQUESTS = 16; /** * Set up the connector instance for backward compatibility with JugglingDB schema/adapter @@ -214,7 +214,8 @@ DataSource.prototype._setupConnector = function() { const dataSource = this; // Set max listeners to a default/configured value - dataSource.setMaxListeners(dataSource.getMaxOfflineRequests()); + const maxOfflineRequests = dataSource.getMaxOfflineRequests(); + dataSource.setMaxListeners(maxOfflineRequests); this.connector.log = function(query, start) { dataSource.log(query, start); @@ -2716,14 +2717,15 @@ DataSource.prototype.getMaxOfflineRequests = function() { // Override this default value with a datasource setting // 'maxOfflineRequests' from an application's datasources.json - let maxOfflineRequests = this.DEFAULT_MAX_OFFLINE_REQUESTS; + let maxOfflineRequests = this.juggler.DataSource.DEFAULT_MAX_OFFLINE_REQUESTS; if ( this.settings && - this.settings.maxOfflineRequests && - typeof this.settings.maxOfflineRequests === 'number' + this.settings.maxOfflineRequests ) { - if (this.settings.maxOfflineRequests > maxOfflineRequests) - maxOfflineRequests = this.settings.maxOfflineRequests; + if (!(typeof this.settings.maxOfflineRequests === 'number')) + throw new Error('maxOfflineRequests must be a number'); + + maxOfflineRequests = this.settings.maxOfflineRequests; } return maxOfflineRequests; }; diff --git a/test/datasource.test.js b/test/datasource.test.js index ae158506c..d3392fee6 100644 --- a/test/datasource.test.js +++ b/test/datasource.test.js @@ -554,9 +554,9 @@ describe('DataSource', function() { ds.getMaxOfflineRequests().should.be.eql(16); }); - it('uses the default max number of event listeners if the provided one is less than 16', () => { + it('uses the provided max number of listeners even if it is less than 16', () => { ds.settings.maxOfflineRequests = 15; - ds.getMaxOfflineRequests().should.be.eql(16); + ds.getMaxOfflineRequests().should.be.eql(15); }); it('uses provided number of listeners if it is greater than 16', () => { @@ -564,9 +564,12 @@ describe('DataSource', function() { ds.getMaxOfflineRequests().should.be.eql(17); }); - it('uses default if a non-number is provided for the max number of listeners', () => { + it('throws an error if a non-number is provided for the max number of listeners', () => { ds.settings.maxOfflineRequests = '17'; - ds.getMaxOfflineRequests().should.be.eql(16); + + (function() { + return ds.getMaxOfflineRequests(); + }).should.throw('maxOfflineRequests must be a number'); }); }); }); From 487957a9fafd2225cf1598136c3732f8d6c1f6ec Mon Sep 17 00:00:00 2001 From: Nora Date: Wed, 7 Aug 2019 13:00:41 -0400 Subject: [PATCH 4/4] fixup! use datasource constant --- lib/datasource.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/datasource.js b/lib/datasource.js index e2d53ebed..b909a0c6b 100644 --- a/lib/datasource.js +++ b/lib/datasource.js @@ -214,8 +214,7 @@ DataSource.prototype._setupConnector = function() { const dataSource = this; // Set max listeners to a default/configured value - const maxOfflineRequests = dataSource.getMaxOfflineRequests(); - dataSource.setMaxListeners(maxOfflineRequests); + dataSource.setMaxListeners(dataSource.getMaxOfflineRequests()); this.connector.log = function(query, start) { dataSource.log(query, start); @@ -2717,7 +2716,7 @@ DataSource.prototype.getMaxOfflineRequests = function() { // Override this default value with a datasource setting // 'maxOfflineRequests' from an application's datasources.json - let maxOfflineRequests = this.juggler.DataSource.DEFAULT_MAX_OFFLINE_REQUESTS; + let maxOfflineRequests = DataSource.DEFAULT_MAX_OFFLINE_REQUESTS; if ( this.settings && this.settings.maxOfflineRequests