From 7ec99e88a434d32d60a088196ff9d01b4d20fb1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Fournier?= Date: Thu, 28 Sep 2023 10:21:16 +0200 Subject: [PATCH] fix: partial index on events announced (#4856) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## About the changes Add partial index on events by announced. This should help avoid `Seq Scan on events` when the majority of events are announced=true --- Co-authored-by: Ivar Østhus Co-authored-by: Gard Rimestad --- .github/workflows/build.yaml | 2 +- .github/workflows/build_coverage.yaml | 2 +- .github/workflows/build_prs.yaml | 2 +- .github/workflows/build_prs_jest_report.yaml | 2 +- .github/workflows/build_prs_test_report.yaml | 2 +- .../workflows/gradual-strict-null-checks.yml | 2 +- .github/workflows/release.yaml | 2 +- src/lib/db/event-store.test.ts | 53 +++++++------------ src/lib/db/event-store.ts | 7 +-- .../20230927172930-events-announced-index.js | 17 ++++++ 10 files changed, 45 insertions(+), 46 deletions(-) create mode 100644 src/migrations/20230927172930-events-announced-index.js diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index e265b33746af..586b06cff7b7 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -36,7 +36,7 @@ jobs: - name: Use Node.js ${{ matrix.node-version }} uses: actions/setup-node@v3 with: - node-version: ${{ matrix.node-version }} + node-version: 18.17 cache: 'yarn' - run: yarn - run: yarn build:frontend diff --git a/.github/workflows/build_coverage.yaml b/.github/workflows/build_coverage.yaml index 951f76e32fc7..d0151c41ba25 100644 --- a/.github/workflows/build_coverage.yaml +++ b/.github/workflows/build_coverage.yaml @@ -36,7 +36,7 @@ jobs: - name: Use Node.js ${{ matrix.node-version }} uses: actions/setup-node@v3 with: - node-version: ${{ matrix.node-version }} + node-version: 18.17 cache: 'yarn' - run: yarn - run: yarn build:frontend diff --git a/.github/workflows/build_prs.yaml b/.github/workflows/build_prs.yaml index 147f26502c65..d86c71bf3c0c 100644 --- a/.github/workflows/build_prs.yaml +++ b/.github/workflows/build_prs.yaml @@ -18,7 +18,7 @@ jobs: - name: Use Node.js ${{ matrix.node-version }} uses: actions/setup-node@v3 with: - node-version: ${{ matrix.node-version }} + node-version: 18.17 cache: 'yarn' - run: yarn install --frozen-lockfile --ignore-scripts - run: yarn lint diff --git a/.github/workflows/build_prs_jest_report.yaml b/.github/workflows/build_prs_jest_report.yaml index 759db0a9de11..c920afb15dd6 100644 --- a/.github/workflows/build_prs_jest_report.yaml +++ b/.github/workflows/build_prs_jest_report.yaml @@ -34,7 +34,7 @@ jobs: - name: Use Node.js 18.x uses: actions/setup-node@v3 with: - node-version: 18.x + node-version: 18.17 cache: 'yarn' - name: Tests on 18.x id: coverage diff --git a/.github/workflows/build_prs_test_report.yaml b/.github/workflows/build_prs_test_report.yaml index 58183b245566..346165deb925 100644 --- a/.github/workflows/build_prs_test_report.yaml +++ b/.github/workflows/build_prs_test_report.yaml @@ -28,7 +28,7 @@ jobs: - name: Use Node.js 18 uses: actions/setup-node@v3 with: - node-version: 18 + node-version: 18.17 cache: 'yarn' - run: yarn install --frozen-lockfile --ignore-scripts - run: yarn build:backend diff --git a/.github/workflows/gradual-strict-null-checks.yml b/.github/workflows/gradual-strict-null-checks.yml index c90023aae674..38d0a55639e8 100644 --- a/.github/workflows/gradual-strict-null-checks.yml +++ b/.github/workflows/gradual-strict-null-checks.yml @@ -30,7 +30,7 @@ jobs: - name: Use Node.js ${{ matrix.node-version }} uses: actions/setup-node@v3 with: - node-version: ${{ matrix.node-version }} + node-version: 18.17 cache: 'yarn' cache-dependency-path: | current/yarn.lock diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index 129cecd9d9a0..ac75c65ee7e5 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -17,7 +17,7 @@ jobs: - name: Setup to npm uses: actions/setup-node@v3 with: - node-version: ${{ matrix.node-version }} + node-version: 18.17 registry-url: 'https://registry.npmjs.org' cache: 'yarn' - name: Build diff --git a/src/lib/db/event-store.test.ts b/src/lib/db/event-store.test.ts index 9f6e42d127b4..2cf7d955fc90 100644 --- a/src/lib/db/event-store.test.ts +++ b/src/lib/db/event-store.test.ts @@ -19,6 +19,7 @@ test('Trying to get events if db fails should yield empty list', async () => { const store = new EventStore(db, getLogger); const events = await store.getEvents(); expect(events.length).toBe(0); + await db.destroy(); }); test('Trying to get events by name if db fails should yield empty list', async () => { @@ -29,39 +30,25 @@ test('Trying to get events by name if db fails should yield empty list', async ( const events = await store.searchEvents({ type: 'application-created' }); expect(events).toBeTruthy(); expect(events.length).toBe(0); + await db.destroy(); }); -test.each([ - { - createdAt: formatRFC3339(subHours(new Date(), 1)), - expectedCount: 1, - }, - { - createdAt: formatRFC3339(subHours(new Date(), 23)), - expectedCount: 1, - }, - { - createdAt: formatRFC3339(subHours(new Date(), 25)), - expectedCount: 0, - }, -])( - 'Find unnanounced events is capped to last 24hs', - async ({ createdAt, expectedCount }) => { - const db = await dbInit('events_test', getLogger); - const type = 'application-created' as const; - const insertQuery = db.rawDatabase('events'); - await insertQuery - .insert({ - type, - created_at: createdAt, - created_by: 'a test', - data: { name: 'test', createdAt }, - }) - .returning(['id']); +// We might want to cap this to 500 and this test can help checking that +test('Find unannounced events returns all events', async () => { + const db = await dbInit('events_test', getLogger); + const type = 'application-created' as const; - const store = new EventStore(db.rawDatabase, getLogger); - const events = await store.setUnannouncedToAnnounced(); - expect(events).toBeTruthy(); - expect(events.length).toBe(expectedCount); - }, -); + const allEvents = Array.from({ length: 505 }).map((_, i) => ({ + type, + created_at: formatRFC3339(subHours(new Date(), i)), + created_by: `test ${i}`, + data: { name: 'test', iteration: i }, + })); + await db.rawDatabase('events').insert(allEvents).returning(['id']); + + const store = new EventStore(db.rawDatabase, getLogger); + let events = await store.setUnannouncedToAnnounced(); + expect(events).toBeTruthy(); + expect(events.length).toBe(505); + await db.destroy(); +}); diff --git a/src/lib/db/event-store.ts b/src/lib/db/event-store.ts index bafe604a4b9c..c28d1ad4657b 100644 --- a/src/lib/db/event-store.ts +++ b/src/lib/db/event-store.ts @@ -12,7 +12,6 @@ import { sharedEventEmitter } from '../util/anyEventEmitter'; import { Db } from './db'; import { Knex } from 'knex'; import EventEmitter from 'events'; -import { subDays } from 'date-fns'; const EVENT_COLUMNS = [ 'id', @@ -412,11 +411,7 @@ class EventStore implements IEventStore { const rows = await this.db(TABLE) .update({ announced: true }) .where('announced', false) - .whereNotNull('announced') - .where('created_at', '>', subDays(Date.now(), 1)) - .returning(EVENT_COLUMNS) - .limit(500); - + .returning(EVENT_COLUMNS); return rows.map(this.rowToEvent); } diff --git a/src/migrations/20230927172930-events-announced-index.js b/src/migrations/20230927172930-events-announced-index.js new file mode 100644 index 000000000000..ceb9ed778eff --- /dev/null +++ b/src/migrations/20230927172930-events-announced-index.js @@ -0,0 +1,17 @@ +'use strict'; + +exports.up = function (db, callback) { + db.runSql( + ` + UPDATE events set announced = false where announced IS NULL; + ALTER TABLE events ALTER COLUMN announced SET NOT NULL; + ALTER TABLE events ALTER COLUMN announced SET DEFAULT false; + CREATE INDEX events_unannounced_idx ON events(announced) WHERE announced = false; + `, + callback(), + ); +}; + +exports.down = function (db, callback) { + callback(); +};