From f5ecbc00a70db3de436f456fa7fb5a3bf44005ab Mon Sep 17 00:00:00 2001 From: Rich Snapp Date: Tue, 19 Jun 2018 14:03:48 -0600 Subject: [PATCH 1/7] initial attempt at limiting concurrenet auctions by origin --- src/adaptermanager.js | 12 ++++-- src/ajax.js | 87 ++++++++++++++++++------------------------- src/auction.js | 84 ++++++++++++++++++++++++++++++++++++++++- 3 files changed, 127 insertions(+), 56 deletions(-) diff --git a/src/adaptermanager.js b/src/adaptermanager.js index f6c0d5c421e..3d5d56d6ee2 100644 --- a/src/adaptermanager.js +++ b/src/adaptermanager.js @@ -273,7 +273,7 @@ exports.checkBidRequestSizes = (adUnits) => { return adUnits; } -exports.callBids = (adUnits, bidRequests, addBidResponse, doneCb) => { +exports.callBids = (adUnits, bidRequests, addBidResponse, doneCb, requestCallbacks) => { if (!bidRequests.length) { utils.logWarn('callBids executed with no bidRequests. Were they filtered by labels or sizing?'); return; @@ -285,7 +285,10 @@ exports.callBids = (adUnits, bidRequests, addBidResponse, doneCb) => { }, [[], []]); if (serverBidRequests.length) { - const s2sAjax = ajaxBuilder(serverBidRequests[0].timeout); + const s2sAjax = ajaxBuilder(serverBidRequests[0].timeout, requestCallbacks ? { + request: requestCallbacks.request.bind(null, 's2s'), + done: requestCallbacks.done + } : undefined); let adaptersServerSide = _s2sConfig.bidders; const s2sAdapter = _bidderRegistry[_s2sConfig.adapter]; let tid = serverBidRequests[0].tid; @@ -336,7 +339,6 @@ exports.callBids = (adUnits, bidRequests, addBidResponse, doneCb) => { } } - const ajax = (clientBidRequests.length) ? ajaxBuilder(clientBidRequests[0].timeout) : null; // handle client adapter requests clientBidRequests.forEach(bidRequest => { bidRequest.start = timestamp(); @@ -347,6 +349,10 @@ exports.callBids = (adUnits, bidRequests, addBidResponse, doneCb) => { events.emit(CONSTANTS.EVENTS.BID_REQUESTED, bidRequest); bidRequest.doneCbCallCount = 0; let done = doneCb(bidRequest.bidderRequestId); + let ajax = ajaxBuilder(clientBidRequests[0].timeout, requestCallbacks ? { + request: requestCallbacks.request.bind(null, bidRequest.bidderCode), + done: requestCallbacks.done + } : undefined); adapter.callBids(bidRequest, addBidResponse, done, ajax); } else { utils.logError(`Adapter trying to be called which does not exist: ${bidRequest.bidderCode} adaptermanager.callBids`); diff --git a/src/ajax.js b/src/ajax.js index ded2f95f8a5..08c1d304314 100644 --- a/src/ajax.js +++ b/src/ajax.js @@ -15,12 +15,13 @@ const XHR_DONE = 4; */ export const ajax = ajaxBuilder(); -export function ajaxBuilder(timeout = 3000) { +export function ajaxBuilder(timeout = 3000, {request, done} = {}) { return function(url, callback, data, options = {}) { try { let x; - let useXDomainRequest = false; let method = options.method || (data ? 'POST' : 'GET'); + let parser = document.createElement('a'); + parser.href = url; let callbacks = typeof callback === 'object' && callback !== null ? callback : { success: function() { @@ -35,46 +36,27 @@ export function ajaxBuilder(timeout = 3000) { callbacks.success = callback; } - if (!window.XMLHttpRequest) { - useXDomainRequest = true; - } else { - x = new window.XMLHttpRequest(); - if (x.responseType === undefined) { - useXDomainRequest = true; - } - } + x = new window.XMLHttpRequest(); - if (useXDomainRequest) { - x = new window.XDomainRequest(); - x.onload = function () { - callbacks.success(x.responseText, x); - }; - - // http://stackoverflow.com/questions/15786966/xdomainrequest-aborts-post-on-ie-9 - x.onerror = function () { - callbacks.error('error', x); - }; - x.ontimeout = function () { - callbacks.error('timeout', x); - }; - x.onprogress = function() { - utils.logMessage('xhr onprogress'); - }; - } else { - x.onreadystatechange = function () { - if (x.readyState === XHR_DONE) { - let status = x.status; - if ((status >= 200 && status < 300) || status === 304) { - callbacks.success(x.responseText, x); - } else { - callbacks.error(x.statusText, x); - } + x.onreadystatechange = function () { + if (x.readyState === XHR_DONE) { + if (typeof done === 'function') { + done(parser.origin); } - }; - x.ontimeout = function () { - utils.logError(' xhr timeout after ', x.timeout, 'ms'); - }; - } + let status = x.status; + if ((status >= 200 && status < 300) || status === 304) { + callbacks.success(x.responseText, x); + } else { + callbacks.error(x.statusText, x); + } + } + }; + x.ontimeout = function () { + if (typeof done === 'function') { + done(parser.origin); + } + utils.logError(' xhr timeout after ', x.timeout, 'ms'); + }; if (method === 'GET' && data) { let urlInfo = parseURL(url, options); @@ -86,18 +68,21 @@ export function ajaxBuilder(timeout = 3000) { // IE needs timoeut to be set after open - see #1410 x.timeout = timeout; - if (!useXDomainRequest) { - if (options.withCredentials) { - x.withCredentials = true; - } - utils._each(options.customHeaders, (value, header) => { - x.setRequestHeader(header, value); - }); - if (options.preflight) { - x.setRequestHeader('X-Requested-With', 'XMLHttpRequest'); - } - x.setRequestHeader('Content-Type', options.contentType || 'text/plain'); + if (options.withCredentials) { + x.withCredentials = true; } + utils._each(options.customHeaders, (value, header) => { + x.setRequestHeader(header, value); + }); + if (options.preflight) { + x.setRequestHeader('X-Requested-With', 'XMLHttpRequest'); + } + x.setRequestHeader('Content-Type', options.contentType || 'text/plain'); + + if (typeof request === 'function') { + request(parser.origin); + } + if (method === 'POST' && data) { x.send(data); } else { diff --git a/src/auction.js b/src/auction.js index 8992f16218e..04c7c241e94 100644 --- a/src/auction.js +++ b/src/auction.js @@ -74,6 +74,11 @@ events.on(CONSTANTS.EVENTS.BID_ADJUSTMENT, function (bid) { adjustBids(bid); }); +const MAX_REQUESTS_PER_ORIGIN = 6; +const outstandingRequests = {}; +const sourceInfo = {}; +const queuedCalls = []; + /** * Creates new auction instance * @@ -193,9 +198,84 @@ export function newAuction({adUnits, adUnitCodes, callback, cbTimeout, labels}) addBidRequests(bidRequest); }); + let requests = {}; + _auctionStatus = AUCTION_IN_PROGRESS; - adaptermanager.callBids(_adUnits, bidRequests, addBidResponse.bind(this), done.bind(this)); - }; + + queuedCalls.push({ + bidRequests, + fn: () => { + adaptermanager.callBids(_adUnits, bidRequests, addBidResponse.bind(this), done.bind(this), { + request(source, origin) { + increment(outstandingRequests, origin); + increment(requests, source); + + if (!sourceInfo[source]) { + sourceInfo[source] = { + SRA: true, + origin + }; + } + if (requests[source] > 1) { + sourceInfo[source].SRA = false; + } + }, + done(origin) { + outstandingRequests[origin]--; + if (queuedCalls[0]) { + runIfOriginHasCapacity(queuedCalls[0]) + } + } + }); + } + }); + + if (queuedCalls.length === 1) { + queuedCalls.shift().fn(); + } else { + if (!runIfOriginHasCapacity(queuedCalls[0])) { + utils.logWarn('queueing auction due to limited endpoint capacity'); + } + } + + function runIfOriginHasCapacity(nextCall) { + let hasCapacity = true; + + nextCall.bidRequests.some(bidRequest => { + let requests = 1; + let source = (typeof bidRequest.src !== 'undefined' && bidRequest.src === CONSTANTS.S2S.SRC) ? 's2s' + : bidRequest.bidderCode; + // if we have no previous info on this source just let them through + if (sourceInfo[source]) { + if (sourceInfo[source].SRA === false) { + // some bidders might use more than the MAX_REQUESTS_PER_ORIGIN in a single auction. In those cases + // set their request count to MAX_REQUESTS_PER_ORIGIN so the auction isn't permanently queued waiting + // for capacity for that bidder + requests = Math.min(bidRequest.bids.length, MAX_REQUESTS_PER_ORIGIN); + } + if (outstandingRequests[sourceInfo[source].origin] + requests > MAX_REQUESTS_PER_ORIGIN) { + hasCapacity = false; + } + } + // return only used for terminating this .some() iteration early if it is determined we don't have capacity + return !hasCapacity; + }); + + if (hasCapacity) { + queuedCalls.shift().fn(); + } + + return hasCapacity; + } + + function increment(obj, prop) { + if (typeof obj[prop] === 'undefined') { + obj[prop] = 1 + } else { + obj[prop]++; + } + } + } return { addBidReceived, From 6bc848ddbf7ef0af3c5542dd11885f2f910bb745 Mon Sep 17 00:00:00 2001 From: Rich Snapp Date: Tue, 19 Jun 2018 15:04:07 -0600 Subject: [PATCH 2/7] fix queueing of auctions for max origin --- src/auction.js | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/src/auction.js b/src/auction.js index 04c7c241e94..e19c0f3bbcd 100644 --- a/src/auction.js +++ b/src/auction.js @@ -202,9 +202,9 @@ export function newAuction({adUnits, adUnitCodes, callback, cbTimeout, labels}) _auctionStatus = AUCTION_IN_PROGRESS; - queuedCalls.push({ + let call = { bidRequests, - fn: () => { + run: () => { adaptermanager.callBids(_adUnits, bidRequests, addBidResponse.bind(this), done.bind(this), { request(source, origin) { increment(outstandingRequests, origin); @@ -223,25 +223,24 @@ export function newAuction({adUnits, adUnitCodes, callback, cbTimeout, labels}) done(origin) { outstandingRequests[origin]--; if (queuedCalls[0]) { - runIfOriginHasCapacity(queuedCalls[0]) + if(runIfOriginHasCapacity(queuedCalls[0])) { + queuedCalls.shift(); + } } } }); } - }); + }; - if (queuedCalls.length === 1) { - queuedCalls.shift().fn(); - } else { - if (!runIfOriginHasCapacity(queuedCalls[0])) { - utils.logWarn('queueing auction due to limited endpoint capacity'); - } + if (!runIfOriginHasCapacity(call)) { + utils.logWarn('queueing auction due to limited endpoint capacity'); + queuedCalls.push(call); } - function runIfOriginHasCapacity(nextCall) { + function runIfOriginHasCapacity(call) { let hasCapacity = true; - nextCall.bidRequests.some(bidRequest => { + call.bidRequests.some(bidRequest => { let requests = 1; let source = (typeof bidRequest.src !== 'undefined' && bidRequest.src === CONSTANTS.S2S.SRC) ? 's2s' : bidRequest.bidderCode; @@ -262,7 +261,7 @@ export function newAuction({adUnits, adUnitCodes, callback, cbTimeout, labels}) }); if (hasCapacity) { - queuedCalls.shift().fn(); + call.run(); } return hasCapacity; From 7b1c3d7ac091d0513744b11a85e568bdda38a7f2 Mon Sep 17 00:00:00 2001 From: Rich Snapp Date: Tue, 19 Jun 2018 15:35:01 -0600 Subject: [PATCH 3/7] don't decrement on timeout as it is already called by onreadystatechange --- src/ajax.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/ajax.js b/src/ajax.js index 08c1d304314..e17f782ac30 100644 --- a/src/ajax.js +++ b/src/ajax.js @@ -52,9 +52,6 @@ export function ajaxBuilder(timeout = 3000, {request, done} = {}) { } }; x.ontimeout = function () { - if (typeof done === 'function') { - done(parser.origin); - } utils.logError(' xhr timeout after ', x.timeout, 'ms'); }; From 62df3fabe88bb635c6a7fe5daff91fd43012ee5c Mon Sep 17 00:00:00 2001 From: Rich Snapp Date: Tue, 19 Jun 2018 15:47:46 -0600 Subject: [PATCH 4/7] move auction timer so it doesn't start until queued auction starts --- src/auction.js | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/auction.js b/src/auction.js index e19c0f3bbcd..5b8861531e5 100644 --- a/src/auction.js +++ b/src/auction.js @@ -181,17 +181,9 @@ export function newAuction({adUnits, adUnitCodes, callback, cbTimeout, labels}) } function callBids() { - startAuctionTimer(); _auctionStatus = AUCTION_STARTED; _auctionStart = Date.now(); - const auctionInit = { - timestamp: _auctionStart, - auctionId: _auctionId, - timeout: _timeout - }; - events.emit(CONSTANTS.EVENTS.AUCTION_INIT, auctionInit); - let bidRequests = adaptermanager.makeBidRequests(_adUnits, _auctionStart, _auctionId, _timeout, _labels); utils.logInfo(`Bids Requested for Auction with id: ${_auctionId}`, bidRequests); bidRequests.forEach(bidRequest => { @@ -200,11 +192,20 @@ export function newAuction({adUnits, adUnitCodes, callback, cbTimeout, labels}) let requests = {}; - _auctionStatus = AUCTION_IN_PROGRESS; - let call = { bidRequests, run: () => { + startAuctionTimer(); + + _auctionStatus = AUCTION_IN_PROGRESS; + + const auctionInit = { + timestamp: _auctionStart, + auctionId: _auctionId, + timeout: _timeout + }; + events.emit(CONSTANTS.EVENTS.AUCTION_INIT, auctionInit); + adaptermanager.callBids(_adUnits, bidRequests, addBidResponse.bind(this), done.bind(this), { request(source, origin) { increment(outstandingRequests, origin); From e85160ea9f45f801b1c9aef1de1716682f5dabe7 Mon Sep 17 00:00:00 2001 From: Rich Snapp Date: Tue, 19 Jun 2018 16:45:16 -0600 Subject: [PATCH 5/7] set default max concurrent origin requests to 4 and make configurable --- src/auction.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/auction.js b/src/auction.js index 5b8861531e5..f80507da980 100644 --- a/src/auction.js +++ b/src/auction.js @@ -74,7 +74,7 @@ events.on(CONSTANTS.EVENTS.BID_ADJUSTMENT, function (bid) { adjustBids(bid); }); -const MAX_REQUESTS_PER_ORIGIN = 6; +const MAX_REQUESTS_PER_ORIGIN = 4; const outstandingRequests = {}; const sourceInfo = {}; const queuedCalls = []; @@ -241,6 +241,8 @@ export function newAuction({adUnits, adUnitCodes, callback, cbTimeout, labels}) function runIfOriginHasCapacity(call) { let hasCapacity = true; + let maxRequests = config.getConfig('maxRequestsPerOrigin') || MAX_REQUESTS_PER_ORIGIN; + call.bidRequests.some(bidRequest => { let requests = 1; let source = (typeof bidRequest.src !== 'undefined' && bidRequest.src === CONSTANTS.S2S.SRC) ? 's2s' @@ -251,9 +253,9 @@ export function newAuction({adUnits, adUnitCodes, callback, cbTimeout, labels}) // some bidders might use more than the MAX_REQUESTS_PER_ORIGIN in a single auction. In those cases // set their request count to MAX_REQUESTS_PER_ORIGIN so the auction isn't permanently queued waiting // for capacity for that bidder - requests = Math.min(bidRequest.bids.length, MAX_REQUESTS_PER_ORIGIN); + requests = Math.min(bidRequest.bids.length, maxRequests); } - if (outstandingRequests[sourceInfo[source].origin] + requests > MAX_REQUESTS_PER_ORIGIN) { + if (outstandingRequests[sourceInfo[source].origin] + requests > maxRequests) { hasCapacity = false; } } From 8cd5ad3b06f4e21c0a5170bdf23b1bccb9783220 Mon Sep 17 00:00:00 2001 From: Rich Snapp Date: Tue, 19 Jun 2018 18:26:57 -0600 Subject: [PATCH 6/7] fix tests to not queue for auction.callBids --- src/auction.js | 4 ++-- test/spec/unit/pbjs_api_spec.js | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/auction.js b/src/auction.js index f80507da980..8b82e733532 100644 --- a/src/auction.js +++ b/src/auction.js @@ -74,7 +74,7 @@ events.on(CONSTANTS.EVENTS.BID_ADJUSTMENT, function (bid) { adjustBids(bid); }); -const MAX_REQUESTS_PER_ORIGIN = 4; +export let MAX_REQUESTS_PER_ORIGIN = 4; const outstandingRequests = {}; const sourceInfo = {}; const queuedCalls = []; @@ -224,7 +224,7 @@ export function newAuction({adUnits, adUnitCodes, callback, cbTimeout, labels}) done(origin) { outstandingRequests[origin]--; if (queuedCalls[0]) { - if(runIfOriginHasCapacity(queuedCalls[0])) { + if (runIfOriginHasCapacity(queuedCalls[0])) { queuedCalls.shift(); } } diff --git a/test/spec/unit/pbjs_api_spec.js b/test/spec/unit/pbjs_api_spec.js index 93904dcfaf8..12e6bd14b3f 100644 --- a/test/spec/unit/pbjs_api_spec.js +++ b/test/spec/unit/pbjs_api_spec.js @@ -1305,6 +1305,7 @@ describe('Unit: Prebid Module', function () { ] }]; adUnitCodes = ['adUnit-code']; + configObj.setConfig({maxRequestsPerOrigin: Number.MAX_SAFE_INTEGER || 99999999}); let auction = auctionModule.newAuction({adUnits, adUnitCodes, callback: function() {}, cbTimeout: timeout}); spyCallBids = sinon.spy(adaptermanager, 'callBids'); createAuctionStub = sinon.stub(auctionModule, 'newAuction'); From 3d31b0e1be9c1eaecdc4932308cfa12251782909 Mon Sep 17 00:00:00 2001 From: Rich Snapp Date: Tue, 19 Jun 2018 19:23:55 -0600 Subject: [PATCH 7/7] change MAX_REQUEST_PER_ORIGIN to local var --- src/auction.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/auction.js b/src/auction.js index 8b82e733532..5722bcc6990 100644 --- a/src/auction.js +++ b/src/auction.js @@ -74,7 +74,7 @@ events.on(CONSTANTS.EVENTS.BID_ADJUSTMENT, function (bid) { adjustBids(bid); }); -export let MAX_REQUESTS_PER_ORIGIN = 4; +const MAX_REQUESTS_PER_ORIGIN = 4; const outstandingRequests = {}; const sourceInfo = {}; const queuedCalls = [];