Skip to content

Commit

Permalink
fix: partial index on events announced (#4856)
Browse files Browse the repository at this point in the history
## 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 <ivar@getunleash.io>
Co-authored-by: Gard Rimestad <gard@getunleash.io>
  • Loading branch information
gastonfournier committed Sep 28, 2023
1 parent 19053cd commit f9c3259
Show file tree
Hide file tree
Showing 10 changed files with 45 additions and 46 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/build.yaml
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/build_coverage.yaml
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/build_prs.yaml
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/build_prs_jest_report.yaml
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/build_prs_test_report.yaml
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/gradual-strict-null-checks.yml
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/release.yaml
Expand Up @@ -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
Expand Down
53 changes: 20 additions & 33 deletions src/lib/db/event-store.test.ts
Expand Up @@ -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 () => {
Expand All @@ -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();
});
7 changes: 1 addition & 6 deletions src/lib/db/event-store.ts
Expand Up @@ -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',
Expand Down Expand Up @@ -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);
}

Expand Down
17 changes: 17 additions & 0 deletions 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();
};

0 comments on commit f9c3259

Please sign in to comment.