-
Notifications
You must be signed in to change notification settings - Fork 0
Stress test more data & add server pagination/filtering/sorting #592
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
Conversation
5875e1d to
08184ff
Compare
|
Tested with 1000 participants, the participants list page was very slow to load (about 5 seconds) including 1.5 seconds for the backend to respond. Pagination clearly required. Implemented server side pagination and filtering/sorting. Field encryption severely limits server side sorting and filtering options. Sorting is not possible and filtering can only be done by exact match. Introducing these features would require application side filtering and sorting. |
61e0a1e to
076ccb6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances the participant listing endpoint with filtering, sorting, and pagination capabilities to support stress testing with larger datasets. It adds server-side data processing features and improves the admin UI to handle performance optimization for large participant lists.
Key changes:
- Added server-side filtering and sorting functionality with Prisma-compatible query parsing
- Enhanced participant endpoint to support pagination, filtering by participant profile fields, and sorting
- Implemented stress test data seeding script to generate 1000 test participants
Reviewed Changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Added faker.js for test data generation and vite-node for script execution |
| application/common/types/api/participants/getParticipants.ts | Added total count field to response interface |
| application/backend/swagger.json | Updated OpenAPI schema formatting and added new schema definitions |
| application/backend/src/utils/filtering.ts | New utility for parsing query parameters into Prisma filters and sorting |
| application/backend/src/routes.ts | Updated route definition to accept query parameters |
| application/backend/src/controllers/ParticipantsController.ts | Enhanced getParticipants method with filtering, sorting, and pagination |
| application/backend/src/controllers/ParticipantsController.test.ts | Added tests for pagination and filtering functionality |
| application/backend/scripts/stressTestSeed.ts | New script to generate 1000 test participants using faker.js |
| application/backend/package.json | Added stress test script and vite-node dependency |
| application/admin-client/src/providers/dataProvider.ts | Enhanced data provider to support server-side pagination, filtering, and sorting |
| application/admin-client/src/pages/responses/all.tsx | Updated data grid import |
| application/admin-client/src/pages/participants/list.tsx | Added client-side filtering UI with debounced input |
| application/admin-client/package.json | Updated data grid dependencies |
Comments suppressed due to low confidence (1)
application/admin-client/src/pages/participants/list.tsx:1
- [nitpick] Using an alias 'x-data-grid-8' for a package import is confusing and may cause maintenance issues. Consider using a more descriptive alias or documenting why this specific version aliasing is needed.
import { ParticipantAnswerStatus } from '@common/types/api/participants/participant'
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
ignatiusm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works well! Also tested with 10000 to check some other pages in the app (not systematically, just clicking around). Just two comments: one suggestion and a question.
| const participants: Partial<StudyParticipant>[] = [] | ||
| const profiles: Partial<ParticipantProfile>[] = [] | ||
| const answers: Partial<SurveyVersionAnswers>[] = [] | ||
| for (let i = 0; i < 1000; i++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the script would be more valuable if the number of participants was passed in as an argument. This would make it much easier to run the script with different numbers. yarn prisma:stresstest 5000
Not absolutely necessary, but a nice-to-have
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good suggestion, ee45b5f
|
|
||
| //eslint-disable-next-line | ||
| const SeedSurveyStepData = require('../prisma/seed/seedSurveyStepData.json') | ||
| const exampleAnswers = createDefaultAnswers(SeedSurveyStepData) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried with 10000 participants and the responses page was not slow :) do you think this is due to it being a simpler query? Is there value in using faker to generate a mix of different answers, as well as some non-responses (not as part of this PR)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it is because the participants list pings the database once for every record, whereas the the responses page does it all at once.
I actually just tried optimising the participants query then and was able to get it down to 10ms for the whole dataset without pagination.
Filtering and sorting is limited to only the current page when doing client side pagination, so we needed to go server-side anyway.
I just added in the optimisation I did, which makes the query 3x faster
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolves #516