Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Max origin concurrent auctions #2743

Merged
12 changes: 9 additions & 3 deletions src/adaptermanager.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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();
Expand All @@ -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`);
Expand Down
84 changes: 33 additions & 51 deletions src/ajax.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -35,46 +36,24 @@ export function ajaxBuilder(timeout = 3000) {
callbacks.success = callback;
}

if (!window.XMLHttpRequest) {
useXDomainRequest = true;
} else {
x = new window.XMLHttpRequest();
if (x.responseType === undefined) {
useXDomainRequest = true;
}
}

if (useXDomainRequest) {
x = new window.XDomainRequest();
x.onload = function () {
callbacks.success(x.responseText, x);
};
x = new window.XMLHttpRequest();

// 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 () {
utils.logError(' xhr timeout after ', x.timeout, 'ms');
};

if (method === 'GET' && data) {
let urlInfo = parseURL(url, options);
Expand All @@ -86,18 +65,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 {
Expand Down
104 changes: 93 additions & 11 deletions src/auction.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,11 @@ events.on(CONSTANTS.EVENTS.BID_ADJUSTMENT, function (bid) {
adjustBids(bid);
});

const MAX_REQUESTS_PER_ORIGIN = 4;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the magic behind 4? I thought modern browser was 6?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On my specific test page there wasn't much difference with fast internet between 4 and 6 but there was a slight improvement on slow pages with 4 vs 6. I'm not sure why, but I decided it would be safer to go with that value; if other's test prove differently then we could change it down the line. It's also one of the reasons I made it configurable.

const outstandingRequests = {};
const sourceInfo = {};
const queuedCalls = [];

/**
* Creates new auction instance
*
Expand Down Expand Up @@ -176,26 +181,103 @@ 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 => {
addBidRequests(bidRequest);
});

_auctionStatus = AUCTION_IN_PROGRESS;
adaptermanager.callBids(_adUnits, bidRequests, addBidResponse.bind(this), done.bind(this));
};
let requests = {};

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);
increment(requests, source);

if (!sourceInfo[source]) {
sourceInfo[source] = {
SRA: true,
origin
};
}
if (requests[source] > 1) {
sourceInfo[source].SRA = false;
}
},
done(origin) {
Copy link
Collaborator

@jaiminpanchal27 jaiminpanchal27 Jun 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we rename this done to avoid confusion between auction done and this done ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could, although I'm not sure it's a huge deal. I intentionally kept it as short as possible since it's an object property, so it won't be minified.

outstandingRequests[origin]--;
if (queuedCalls[0]) {
if (runIfOriginHasCapacity(queuedCalls[0])) {
queuedCalls.shift();
}
}
}
});
}
};

if (!runIfOriginHasCapacity(call)) {
utils.logWarn('queueing auction due to limited endpoint capacity');
queuedCalls.push(call);
}

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'
: 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, maxRequests);
}
if (outstandingRequests[sourceInfo[source].origin] + requests > maxRequests) {
hasCapacity = false;
}
}
// return only used for terminating this .some() iteration early if it is determined we don't have capacity
return !hasCapacity;
});

if (hasCapacity) {
call.run();
}

return hasCapacity;
}

function increment(obj, prop) {
if (typeof obj[prop] === 'undefined') {
obj[prop] = 1
} else {
obj[prop]++;
}
}
}

return {
addBidReceived,
Expand Down
1 change: 1 addition & 0 deletions test/spec/unit/pbjs_api_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down