-
Notifications
You must be signed in to change notification settings - Fork 2
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
Load balance support #135
Load balance support #135
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,7 +12,7 @@ export class ConfigurationClientWrapper { | |
endpoint: string; | ||
client: AppConfigurationClient; | ||
backoffEndTime: number = 0; // Timestamp | ||
#failedAttempts: number = 0; | ||
failedAttempts: number = 0; | ||
|
||
constructor(endpoint: string, client: AppConfigurationClient) { | ||
this.endpoint = endpoint; | ||
|
@@ -21,11 +21,17 @@ export class ConfigurationClientWrapper { | |
|
||
updateBackoffStatus(successfull: boolean) { | ||
if (successfull) { | ||
this.#failedAttempts = 0; | ||
if (this.failedAttempts > 0) { | ||
this.failedAttempts = 0; | ||
} | ||
this.failedAttempts -= 1; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. failedAttempts can be negative. This doesn't make sense. If this behavior is only for testing, don't do that. |
||
this.backoffEndTime = Date.now(); | ||
} else { | ||
this.#failedAttempts += 1; | ||
this.backoffEndTime = Date.now() + calculateBackoffDuration(this.#failedAttempts); | ||
if (this.failedAttempts < 0) { | ||
this.failedAttempts = 0; | ||
} | ||
this.failedAttempts += 1; | ||
this.backoffEndTime = Date.now() + calculateBackoffDuration(this.failedAttempts); | ||
} | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,113 @@ | ||
// Copyright (c) Microsoft Corporation. | ||
// Licensed under the MIT license. | ||
|
||
import * as chai from "chai"; | ||
import * as chaiAsPromised from "chai-as-promised"; | ||
chai.use(chaiAsPromised); | ||
const expect = chai.expect; | ||
import { load } from "./exportedApi.js"; | ||
import { mockAppConfigurationClientListConfigurationSettings, restoreMocks, createMockedConnectionString, sleepInMs, createMockedFeatureFlag, createMockedEndpoint, mockConfigurationManagerGetClients } from "./utils/testHelper.js"; | ||
import { AppConfigurationClient } from "@azure/app-configuration"; | ||
import { ConfigurationClientWrapper } from "../src/ConfigurationClientWrapper.js"; | ||
|
||
describe("load balance", function () { | ||
this.timeout(10000); | ||
|
||
beforeEach(() => { | ||
}); | ||
|
||
afterEach(() => { | ||
restoreMocks(); | ||
}); | ||
|
||
it("should load balance the request when loadBalancingEnabled", async () => { | ||
// mock multiple pages of feature flags | ||
const page1 = [ | ||
createMockedFeatureFlag("Alpha_1", { enabled: true }), | ||
createMockedFeatureFlag("Alpha_2", { enabled: true }), | ||
]; | ||
const page2 = [ | ||
createMockedFeatureFlag("Beta_1", { enabled: true }), | ||
createMockedFeatureFlag("Beta_2", { enabled: true }), | ||
]; | ||
const fakeEndpoint_1 = createMockedEndpoint("fake_1"); | ||
const fakeEndpoint_2 = createMockedEndpoint("fake_2"); | ||
const fakeClientWrapper_1 = new ConfigurationClientWrapper(fakeEndpoint_1, new AppConfigurationClient(createMockedConnectionString(fakeEndpoint_1))); | ||
const fakeClientWrapper_2 = new ConfigurationClientWrapper(fakeEndpoint_2, new AppConfigurationClient(createMockedConnectionString(fakeEndpoint_2))); | ||
const mockedClientWrappers = [fakeClientWrapper_1, fakeClientWrapper_2]; | ||
mockConfigurationManagerGetClients(mockedClientWrappers, false); | ||
mockAppConfigurationClientListConfigurationSettings(page1, page2); | ||
|
||
const connectionString = createMockedConnectionString(); | ||
const settings = await load(connectionString, { | ||
loadBalancingEnabled: true, | ||
featureFlagOptions: { | ||
enabled: true, | ||
selectors: [{ | ||
keyFilter: "*" | ||
}], | ||
refresh: { | ||
enabled: true, | ||
refreshIntervalInMs: 2000 // 2 seconds for quick test. | ||
} | ||
} | ||
}); | ||
// one request for key values, one request for feature flags | ||
expect(fakeClientWrapper_1.failedAttempts).eq(-1); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Check this pattern. You can follow this pattern to verify how many times something is called. You can do whatever you want when you stub the fakeClientWrapper. You can set a local counter and when the client is called, counter += 1 |
||
expect(fakeClientWrapper_2.failedAttempts).eq(-1); | ||
|
||
await sleepInMs(2 * 1000 + 1); | ||
await settings.refresh(); | ||
// refresh request for feature flags | ||
expect(fakeClientWrapper_1.failedAttempts).eq(-2); | ||
expect(fakeClientWrapper_2.failedAttempts).eq(-1); | ||
|
||
await sleepInMs(2 * 1000 + 1); | ||
await settings.refresh(); | ||
expect(fakeClientWrapper_1.failedAttempts).eq(-2); | ||
expect(fakeClientWrapper_2.failedAttempts).eq(-2); | ||
}); | ||
|
||
it("should load balance the request when loadBalancingEnabled", async () => { | ||
// mock multiple pages of feature flags | ||
const page1 = [ | ||
createMockedFeatureFlag("Alpha_1", { enabled: true }), | ||
createMockedFeatureFlag("Alpha_2", { enabled: true }), | ||
]; | ||
const page2 = [ | ||
createMockedFeatureFlag("Beta_1", { enabled: true }), | ||
createMockedFeatureFlag("Beta_2", { enabled: true }), | ||
]; | ||
const fakeEndpoint_1 = createMockedEndpoint("fake_1"); | ||
const fakeEndpoint_2 = createMockedEndpoint("fake_2"); | ||
const fakeClientWrapper_1 = new ConfigurationClientWrapper(fakeEndpoint_1, new AppConfigurationClient(createMockedConnectionString(fakeEndpoint_1))); | ||
const fakeClientWrapper_2 = new ConfigurationClientWrapper(fakeEndpoint_2, new AppConfigurationClient(createMockedConnectionString(fakeEndpoint_2))); | ||
const mockedClientWrappers = [fakeClientWrapper_1, fakeClientWrapper_2]; | ||
mockConfigurationManagerGetClients(mockedClientWrappers, false); | ||
mockAppConfigurationClientListConfigurationSettings(page1, page2); | ||
|
||
const connectionString = createMockedConnectionString(); | ||
// loadBalancingEnabled is default to false | ||
const settings = await load(connectionString, { | ||
featureFlagOptions: { | ||
enabled: true, | ||
selectors: [{ | ||
keyFilter: "*" | ||
}], | ||
refresh: { | ||
enabled: true, | ||
refreshIntervalInMs: 2000 // 2 seconds for quick test. | ||
} | ||
} | ||
}); | ||
// one request for key values, one request for feature flags | ||
expect(fakeClientWrapper_1.failedAttempts).eq(-2); | ||
expect(fakeClientWrapper_2.failedAttempts).eq(0); | ||
|
||
await sleepInMs(2 * 1000 + 1); | ||
await settings.refresh(); | ||
// refresh request for feature flags | ||
expect(fakeClientWrapper_1.failedAttempts).eq(-3); | ||
expect(fakeClientWrapper_2.failedAttempts).eq(0); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you have to expose failedAttempt to be public for testing, you should make this private and create another public property which only has getter. And the getter will return the private #failedAttempt. This can make sure failedAttempt won't be modified outside of the class. And add comment to explain the purpose.