Skip to content

Commit

Permalink
Fix/embedded proxy memory leak (#2345)
Browse files Browse the repository at this point in the history
* Fixes a memory leak where events would trigger the data polling to restart. Any event would setup another polling interval, which would strain our database. Separated the logic for fetching the data and the polling, and made sure that the polling was only initialized once.
  • Loading branch information
FredrikOseberg authored and nunogois committed Nov 8, 2022
1 parent f624f44 commit 4574c9f
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 3 deletions.
10 changes: 7 additions & 3 deletions src/lib/proxy/proxy-repository.ts
Expand Up @@ -73,7 +73,7 @@ export class ProxyRepository
}

async start(): Promise<void> {
await this.loadDataForToken();
await this.dataPolling();

// Reload cached token data whenever something relevant has changed.
// For now, simply reload all the data on any EventStore event.
Expand All @@ -88,11 +88,15 @@ export class ProxyRepository
clearTimeout(this.timer);
}

private async loadDataForToken() {
private async dataPolling() {
this.timer = setTimeout(async () => {
await this.loadDataForToken();
await this.dataPolling();
}, this.randomizeDelay(this.interval, this.interval * 2)).unref();

await this.loadDataForToken();
}

private async loadDataForToken() {
try {
this.features = await this.featuresForToken();
this.segments = await this.segmentsForToken();
Expand Down
31 changes: 31 additions & 0 deletions src/test/e2e/api/proxy/proxy.e2e.test.ts
Expand Up @@ -10,6 +10,7 @@ import {
import { startOfHour } from 'date-fns';
import { IConstraint, IStrategyConfig } from '../../../../lib/types/model';
import { ProxyRepository } from '../../../../lib/proxy/proxy-repository';
import { FEATURE_UPDATED } from '../../../../lib/types/events';

let app: IUnleashTest;
let db: ITestDb;
Expand Down Expand Up @@ -898,3 +899,33 @@ test('Should change fetch interval', async () => {
expect(spy.mock.calls.length > 30).toBe(true);
jest.useRealTimers();
});

test('Should not recursively set off timers on events', async () => {
jest.useFakeTimers();

const frontendToken = await createApiToken(ApiTokenType.FRONTEND);
const user = await app.services.apiTokenService.getUserForToken(
frontendToken.secret,
);

const spy = jest.spyOn(ProxyRepository.prototype as any, 'dataPolling');
const proxyRepository = new ProxyRepository(
{
getLogger,
frontendApi: { refreshIntervalInMs: 5000 },
},
db.stores,
app.services,
user,
);

await proxyRepository.start();

db.stores.eventStore.emit(FEATURE_UPDATED);

jest.advanceTimersByTime(10000);

proxyRepository.stop();
expect(spy.mock.calls.length < 3).toBe(true);
jest.useRealTimers();
});

0 comments on commit 4574c9f

Please sign in to comment.