diff --git a/src/lib/db/client-metrics-store-v2.ts b/src/lib/db/client-metrics-store-v2.ts index dfc8494f52..357457697b 100644 --- a/src/lib/db/client-metrics-store-v2.ts +++ b/src/lib/db/client-metrics-store-v2.ts @@ -116,9 +116,17 @@ export class ClientMetricsStoreV2 implements IClientMetricsStoreV2 { return prev; }, {}); + // Sort the rows to avoid deadlocks + const batchRow = Object.values(batch).sort( + (a, b) => + a.feature_name.localeCompare(b.feature_name) || + a.app_name.localeCompare(b.app_name) || + a.environment.localeCompare(b.environment), + ); + // Consider rewriting to SQL batch! const insert = this.db(TABLE) - .insert(Object.values(batch)) + .insert(batchRow) .toQuery(); const query = `${insert.toString()} ON CONFLICT (feature_name, app_name, environment, timestamp) DO UPDATE SET "yes" = "client_metrics_env"."yes" + EXCLUDED.yes, "no" = "client_metrics_env"."no" + EXCLUDED.no`; diff --git a/src/test/e2e/api/admin/client-metrics.e2e.test.ts b/src/test/e2e/api/admin/client-metrics.e2e.test.ts index 6558c076a8..f9f2ad7680 100644 --- a/src/test/e2e/api/admin/client-metrics.e2e.test.ts +++ b/src/test/e2e/api/admin/client-metrics.e2e.test.ts @@ -155,14 +155,19 @@ test('should return toggle summary', async () => { .expect('Content-Type', /json/) .expect(200); + const test = demo.lastHourUsage.find((u) => u.environment === 'test'); + const defaultEnv = demo.lastHourUsage.find( + (u) => u.environment === 'default', + ); + expect(demo.featureName).toBe('demo'); expect(demo.lastHourUsage).toHaveLength(2); - expect(demo.lastHourUsage[0].environment).toBe('default'); - expect(demo.lastHourUsage[0].yes).toBe(5); - expect(demo.lastHourUsage[0].no).toBe(4); - expect(demo.lastHourUsage[1].environment).toBe('test'); - expect(demo.lastHourUsage[1].yes).toBe(2); - expect(demo.lastHourUsage[1].no).toBe(6); + expect(test.environment).toBe('test'); + expect(test.yes).toBe(2); + expect(test.no).toBe(6); + expect(defaultEnv.environment).toBe('default'); + expect(defaultEnv.yes).toBe(5); + expect(defaultEnv.no).toBe(4); expect(demo.seenApplications).toStrictEqual(['backend-api', 'web']); }); @@ -219,13 +224,18 @@ test('should only include last hour of metrics return toggle summary', async () .expect('Content-Type', /json/) .expect(200); + const test = demo.lastHourUsage.find((u) => u.environment === 'test'); + const defaultEnv = demo.lastHourUsage.find( + (u) => u.environment === 'default', + ); + expect(demo.featureName).toBe('demo'); expect(demo.lastHourUsage).toHaveLength(2); - expect(demo.lastHourUsage[0].environment).toBe('default'); - expect(demo.lastHourUsage[0].yes).toBe(5); - expect(demo.lastHourUsage[0].no).toBe(4); - expect(demo.lastHourUsage[1].environment).toBe('test'); - expect(demo.lastHourUsage[1].yes).toBe(2); - expect(demo.lastHourUsage[1].no).toBe(6); + expect(defaultEnv.environment).toBe('default'); + expect(defaultEnv.yes).toBe(5); + expect(defaultEnv.no).toBe(4); + expect(test.environment).toBe('test'); + expect(test.yes).toBe(2); + expect(test.no).toBe(6); expect(demo.seenApplications).toStrictEqual(['backend-api', 'web']); });