Merge pull request #1695 from spotify/shmidt-i/fix-catalog-locations-memory-leak

Catalog: add deduplication for logs join

plugins/catalog-backend/migrations/20200721115244_location_update_log_latest_deduplicate.js

@@ -0,0 +1,35 @@
+/*
+ * Copyright 2020 Spotify AB
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+exports.up = function up(knex) {
+  return knex.schema.raw(`DROP VIEW location_update_log_latest;`).raw(`
+  CREATE VIEW location_update_log_latest AS
+  SELECT t1.* FROM location_update_log t1
+  JOIN 
+  (
+     SELECT location_id, MAX(created_at) AS MAXDATE
+     FROM location_update_log
+     GROUP BY location_id
+  ) t2
+  ON t1.location_id = t2.location_id
+  AND t1.created_at = t2.MAXDATE
+  GROUP BY t1.location_id
+  ORDER BY created_at DESC;
+`);
+};
+
+exports.down = function down(knex) {
+  knex.schema.raw(`DROP VIEW location_update_log_latest;`);
+};

plugins/catalog-backend/src/database/CommonDatabase.test.ts

@@ -88,6 +88,31 @@ describe('CommonDatabase', () => {
     expect(locations).toEqual([output]);
     const location = await db.location(locations[0].id);
     expect(location).toEqual(output);
+
+    // If we add 2 new update log events,
+    // this should not result in location duplication
+    // due to incorrect join in DB
+    await db.addLocationUpdateLogEvent(
+      'dd12620d-0436-422f-93bd-929aa0788123',
+      DatabaseLocationUpdateLogStatus.SUCCESS,
+    );
+
+    // Have a second in-between
+    // To avoid having same timestamp on event
+    await new Promise(res => setTimeout(res, 1000));
+    await db.addLocationUpdateLogEvent(
+      'dd12620d-0436-422f-93bd-929aa0788123',
+      DatabaseLocationUpdateLogStatus.FAIL,
+    );
+
+    expect(await db.locations()).toEqual([
+      {
+        ...output,
+        status: DatabaseLocationUpdateLogStatus.FAIL,
+        timestamp: expect.any(String),
+      },
+    ]);
+
     await db.transaction(tx => db.removeLocation(tx, locations[0].id));
 
     await expect(db.locations()).resolves.toEqual([]);