Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize database reindex #4558

Merged
merged 23 commits into from
Jun 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
cf9d361
WIP: Refactor loop over resources
mattwiller May 13, 2024
ce3414b
Optimize database search reindex
mattwiller May 15, 2024
a231d05
Merge branch 'main' of github.com:medplum/medplum into db-reindex-opt…
mattwiller May 15, 2024
ca415a5
Add end time to allow for earlier termination
mattwiller May 16, 2024
94d7d72
Move reindex into reentrant async worker
mattwiller May 17, 2024
e107adb
Add tests for reindex job
mattwiller May 17, 2024
6670ebc
Add comments
mattwiller May 17, 2024
88d4991
Fix build
mattwiller May 17, 2024
5916b93
Log periodically during reindex
mattwiller May 17, 2024
1173998
Add test case for job failure
mattwiller May 17, 2024
110b89d
Add elapsed time to reindex progress log
mattwiller May 17, 2024
131ddb8
Fix comments
mattwiller May 17, 2024
100f3ee
Merge branch 'main' of github.com:medplum/medplum into db-reindex-opt…
mattwiller May 17, 2024
7765444
WIP: Fix
mattwiller May 21, 2024
7804a8f
Hardcode transaction isolation level
mattwiller May 22, 2024
6d2c009
Reindex multiple resource types
mattwiller May 22, 2024
f1b8e23
Revert isolation level change
mattwiller May 22, 2024
30acac5
Merge branch 'main' of github.com:medplum/medplum into db-reindex-opt…
mattwiller May 29, 2024
e5ad91e
Merge branch 'main' of github.com:medplum/medplum into db-reindex-opt…
mattwiller May 31, 2024
29dacc5
Merge branch 'main' of github.com:medplum/medplum into db-reindex-opt…
mattwiller Jun 19, 2024
fe431de
Remove legacy table scan
mattwiller Jun 19, 2024
dfefcb2
Fix extraneous references to removed code
mattwiller Jun 19, 2024
6282599
Fix test
mattwiller Jun 20, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 0 additions & 14 deletions packages/app/src/admin/SuperAdminPage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,20 +75,6 @@ describe('SuperAdminPage', () => {
expect(screen.getByText('Done')).toBeInTheDocument();
});

test('Rebuild compartments for resource type', async () => {
setup();

await act(async () => {
fireEvent.change(screen.getByPlaceholderText('Compartments Resource Type'), { target: { value: 'Project' } });
});

await act(async () => {
fireEvent.click(screen.getByRole('button', { name: 'Rebuild Compartments' }));
});

expect(screen.getByText('Done')).toBeInTheDocument();
});

test('Purge resources', async () => {
setup();

Expand Down
18 changes: 0 additions & 18 deletions packages/app/src/admin/SuperAdminPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,6 @@ export function SuperAdminPage(): JSX.Element {
startAsyncJob(medplum, 'Reindexing Resources', 'admin/super/reindex', formData);
}

function rebuildCompartments(formData: Record<string, string>): void {
startAsyncJob(medplum, 'Rebuilding Compartments', 'admin/super/compartments', formData);
}

function removeBotIdJobsFromQueue(formData: Record<string, string>): void {
medplum
.post('admin/super/removebotidjobsfromqueue', formData)
Expand Down Expand Up @@ -124,20 +120,6 @@ export function SuperAdminPage(): JSX.Element {
</Stack>
</Form>
<Divider my="lg" />
<Title order={2}>Rebuild Compartments</Title>
<p>
When Medplum changes how resource compartments are derived, the system may require a rebuild for old resources
to take effect.
</p>
<Form onSubmit={rebuildCompartments}>
<Stack>
<FormSection title="Resource Type">
<TextInput name="resourceType" placeholder="Compartments Resource Type" />
</FormSection>
<Button type="submit">Rebuild Compartments</Button>
</Stack>
</Form>
<Divider my="lg" />
<Title order={2}>Purge Resources</Title>
<p>As system generated resources accumulate, the system may require a purge to remove old resources.</p>
<Form onSubmit={purgeResources}>
Expand Down
61 changes: 21 additions & 40 deletions packages/server/src/admin/super.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { rebuildR4SearchParameters } from '../seeds/searchparameters';
import { rebuildR4StructureDefinitions } from '../seeds/structuredefinitions';
import { rebuildR4ValueSets } from '../seeds/valuesets';
import { createTestProject, waitForAsyncJob, withTestContext } from '../test.setup';
import { ReindexJobData, getReindexQueue } from '../workers/reindex';

jest.mock('../seeds/valuesets');
jest.mock('../seeds/structuredefinitions');
Expand Down Expand Up @@ -296,6 +297,9 @@ describe('Super Admin routes', () => {
});

test('Reindex with respond-async', async () => {
const queue = getReindexQueue() as any;
queue.add.mockClear();

const res = await request(app)
.post('/admin/super/reindex')
.set('Authorization', 'Bearer ' + adminAccessToken)
Expand All @@ -307,58 +311,35 @@ describe('Super Admin routes', () => {

expect(res.status).toEqual(202);
expect(res.headers['content-location']).toBeDefined();
await waitForAsyncJob(res.headers['content-location'], app, adminAccessToken);
});

test('Rebuild compartments access denied', async () => {
const res = await request(app)
.post('/admin/super/compartments')
.set('Authorization', 'Bearer ' + nonAdminAccessToken)
.type('json')
.send({
resourceType: 'PaymentNotice',
});

expect(res.status).toBe(403);
});

test('Rebuild compartments require async', async () => {
const res = await request(app)
.post('/admin/super/compartments')
.set('Authorization', 'Bearer ' + adminAccessToken)
.type('json')
.send({
resourceType: 'PaymentNotice',
});

expect(res.status).toEqual(400);
expect(res.body.issue[0].details.text).toBe('Operation requires "Prefer: respond-async"');
expect(queue.add).toHaveBeenCalledWith(
'ReindexJobData',
expect.objectContaining<Partial<ReindexJobData>>({
resourceTypes: ['PaymentNotice'],
})
);
});

test('Rebuild compartments invalid resource type', async () => {
const res = await request(app)
.post('/admin/super/compartments')
.set('Authorization', 'Bearer ' + adminAccessToken)
.type('json')
.send({
resourceType: 'XYZ',
});

expect(res.status).toBe(400);
});
test('Reindex with multiple resource types', async () => {
const queue = getReindexQueue() as any;
queue.add.mockClear();

test('Rebuild compartments with respond-async', async () => {
const res = await request(app)
.post('/admin/super/compartments')
.post('/admin/super/reindex')
.set('Authorization', 'Bearer ' + adminAccessToken)
.set('Prefer', 'respond-async')
.type('json')
.send({
resourceType: 'PaymentNotice',
resourceType: 'PaymentNotice, MedicinalProductManufactured,BiologicallyDerivedProduct',
});

expect(res.status).toEqual(202);
expect(res.headers['content-location']).toBeDefined();
expect(queue.add).toHaveBeenCalledWith(
'ReindexJobData',
expect.objectContaining<Partial<ReindexJobData>>({
resourceTypes: ['PaymentNotice', 'MedicinalProductManufactured', 'BiologicallyDerivedProduct'],
})
);
});

test('Set password access denied', async () => {
Expand Down
40 changes: 16 additions & 24 deletions packages/server/src/admin/super.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {
accepted,
allOk,
badRequest,
forbidden,
Expand All @@ -12,7 +13,7 @@ import { asyncWrap } from '../async';
import { setPassword } from '../auth/setpassword';
import { AuthenticatedRequestContext, getAuthenticatedContext } from '../context';
import { getDatabasePool } from '../database';
import { sendAsyncResponse } from '../fhir/operations/utils/asyncjobexecutor';
import { AsyncJobExecutor, sendAsyncResponse } from '../fhir/operations/utils/asyncjobexecutor';
import { invalidRequest, sendOutcome } from '../fhir/outcomes';
import { getSystemRepo } from '../fhir/repo';
import * as dataMigrations from '../migrations/data';
Expand All @@ -22,6 +23,9 @@ import { rebuildR4SearchParameters } from '../seeds/searchparameters';
import { rebuildR4StructureDefinitions } from '../seeds/structuredefinitions';
import { rebuildR4ValueSets } from '../seeds/valuesets';
import { removeBullMQJobByKey } from '../workers/cron';
import { ResourceType } from '@medplum/fhirtypes';
import { addReindexJob } from '../workers/reindex';
import { getConfig } from '../config';

export const superAdminRouter = Router();
superAdminRouter.use(authenticateRequest);
Expand Down Expand Up @@ -74,32 +78,20 @@ superAdminRouter.post(
requireSuperAdmin();
requireAsync(req);

const resourceType = req.body.resourceType;
validateResourceType(resourceType);
const resourceTypes = (req.body.resourceType as string).split(',').map((t) => t.trim());
for (const resourceType of resourceTypes) {
validateResourceType(resourceType);
}
const systemRepo = getSystemRepo();

await sendAsyncResponse(req, res, async () => {
const systemRepo = getSystemRepo();
await systemRepo.reindexResourceType(resourceType);
const exec = new AsyncJobExecutor(systemRepo);
await exec.init(`${req.protocol}://${req.get('host') + req.originalUrl}`);
await exec.run(async (asyncJob) => {
await addReindexJob(resourceTypes as ResourceType[], asyncJob);
});
})
);

// POST to /admin/super/compartments
// to rebuild compartments for a resource type.
// Run this after major changes to how compartments are constructed.
superAdminRouter.post(
'/compartments',
asyncWrap(async (req: Request, res: Response) => {
requireSuperAdmin();
requireAsync(req);

const resourceType = req.body.resourceType;
validateResourceType(resourceType);

await sendAsyncResponse(req, res, async () => {
const systemRepo = getSystemRepo();
await systemRepo.rebuildCompartmentsForResourceType(resourceType);
});
const { baseUrl } = getConfig();
sendOutcome(res, accepted(exec.getContentLocation(baseUrl)));
})
);

Expand Down
17 changes: 4 additions & 13 deletions packages/server/src/fhir/job.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import { AsyncJob } from '@medplum/fhirtypes';
import { randomUUID } from 'crypto';
import express from 'express';
import request from 'supertest';
import { initApp, shutdownApp } from '../app';
import { loadTestConfig } from '../config';
import { createTestProject, withTestContext } from '../test.setup';
import { createTestProject, waitForAsyncJob, withTestContext } from '../test.setup';
import { AsyncJobExecutor } from './operations/utils/asyncjobexecutor';
import { Repository } from './repo';

Expand Down Expand Up @@ -47,24 +46,16 @@ describe('Job status', () => {

test('completed', () =>
withTestContext(async () => {
const job = await asyncJobManager.init('http://example.com');
await asyncJobManager.init('http://example.com');
const callback = jest.fn();

await asyncJobManager.run(async () => {
await asyncJobManager.start(async () => {
callback();
});

expect(callback).toHaveBeenCalled();

const resCompleted = await request(app)
.get(`/fhir/R4/job/${job.id}/status`)
.set('Authorization', 'Bearer ' + accessToken);

expect(resCompleted.status).toBe(200);
expect(resCompleted.body).toMatchObject<Partial<AsyncJob>>({
resourceType: 'AsyncJob',
status: 'completed',
});
await waitForAsyncJob(asyncJobManager.getContentLocation('http://example.com/'), app, accessToken);
}));

test('cancel', () =>
Expand Down
Loading
Loading