Skip to content
This repository has been archived by the owner on Sep 13, 2022. It is now read-only.

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
Signed-off-by: Won Jun Jang <wjang@uber.com>
  • Loading branch information
black-adder committed Apr 5, 2018
1 parent 681a67b commit 7a144b5
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 12 deletions.
23 changes: 11 additions & 12 deletions src/throttler/remote_throttler.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,9 @@ export default class RemoteThrottler {
* @param {object} [options.logger] - optional logger, see _flow/logger.js
* @param {object} [options.metrics] - instance of Metrics object
* @param {number} [options.refreshIntervalMs] - interval in milliseconds that determines how often credits
* are fetched (0 to not refresh, NOT RECOMMENDED!)
* are fetched
* @param {number} [options.initialDelayMs] - interval in milliseconds that determines how soon after initialization
* credits are first fetched (0 to not refresh, NOT RECOMMENDED!).
* credits are first fetched
* @param {string} [options.host] - host for jaeger-agent, defaults to 'localhost'
* @param {number} [options.port] - port for jaeger-agent for /credits endpoint
* @param {function} [options.onCreditsUpdate] - callback function once credits are updated. Used for testing.
Expand All @@ -74,7 +74,7 @@ export default class RemoteThrottler {
this._host = options.host || DEFAULT_THROTTLER_HOST;
this._port = options.port || DEFAULT_THROTTLER_PORT;

this._credits = Object.create(null);
this._credits = {};
this._onCreditsUpdate = options.onCreditsUpdate;

this._initialDelayTimeoutHandle = setTimeout(
Expand All @@ -97,9 +97,8 @@ export default class RemoteThrottler {
if (operation in this._credits) {
return this._isAllowed(operation);
}
// Credits for the operation will be asynchronously fetched
this._credits[operation] = 0;
// If seen for the first time, async fetch credits
this._refreshCredits();
return false;
}

Expand All @@ -118,7 +117,7 @@ export default class RemoteThrottler {
return;
}
const keys = Object.keys(this._credits);
if (keys.length == 0) {
if (keys.length === 0) {
// No point fetching credits if there's no operations to fetch
return;
}
Expand All @@ -132,15 +131,15 @@ export default class RemoteThrottler {
}

_fetchCredits(operations: any) {
let serviceName: string = encodeURIComponent(this._serviceName);
let uuid: string = encodeURIComponent(this._uuid);
let ops = operations.map(encodeURIComponent).join('&operation=');
let url = `/credits?service=${serviceName}&uuid=${uuid}&operation=${ops}`;
const serviceName: string = encodeURIComponent(this._serviceName);
const uuid: string = encodeURIComponent(this._uuid);
const ops: string = operations.map(encodeURIComponent).join('&operation=');
const url: string = `/credits?service=${serviceName}&uuid=${uuid}&operation=${ops}`;

let success: Function = body => {
const success: Function = body => {
this._parseCreditResponse(body);
};
let error: Function = err => {
const error: Function = err => {
this._logger.error(`Error in fetching credits: ${err}.`);
this._metrics.throttlerUpdateFailure.increment(1);
};
Expand Down
7 changes: 7 additions & 0 deletions test/throttler/remote_throttler.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,20 @@ describe('RemoteThrottler should', () => {
done();
};
assert.isNotOk(throttler.isAllowed(operation));
throttler._refreshCredits();
});

it('log an error if _refreshCredits is called prior to UUID being set', () => {
throttler._refreshCredits();
assert.equal(logger._errorMsgs.length, 1);
});

it('not fetch credits if uuid is invalid', () => {
throttler = new RemoteThrottler(serviceName);
throttler.setProcess({ uuid: null });
throttler._refreshCredits();
});

it("return false for _isAllowed if operation isn't in _credits or operation has no credits", () => {
assert.isNotOk(
throttler._isAllowed(operation),
Expand Down

0 comments on commit 7a144b5

Please sign in to comment.