perf(10749): move tasks_by_contact view to offline-only design document#10776
perf(10749): move tasks_by_contact view to offline-only design document#10776sh1vam31 wants to merge 18 commits intomedic:masterfrom
Conversation
4908220 to
5b87f24
Compare
5b87f24 to
67e737e
Compare
|
Hi @witash, I have updated the PR to move the tasks_by_contact view from the server-managed design document to an offline-only location (_design/medic-offline-tasks). This change ensures that the high-volume index is only built on the client devices, reducing unnecessary server-side disk and processing overhead as requested in #10749. I have also updated the Rules Engine to correctly query this new location. Looking forward to your feedback |
witash
left a comment
There was a problem hiding this comment.
Looks like there's some lint failures, you can run npm run lint locally to check (it will be fast).
They are in api/src/services/authorization.js, which separately I don't think needs to be changed.
Other than that looks great.
this will be a huge improvement on larger deployments
| const validatedIds = [MEDIC_CLIENT_DDOC, getUserSettingsId(authCtx.userCtx.name)]; | ||
| const validatedIds = [ | ||
| MEDIC_CLIENT_DDOC, | ||
| MEDIC_OFFLINE_TASKS_DDOC, |
There was a problem hiding this comment.
I don't think this change is necessarry? This list is of documents on the server side that will be replicated to offline clients...but MEDIC_OFFLINE_TASKS_DDOC doesn't exist on the server at all; we don't need to add medic-offline-freetext here so also shouldn't for medic-offline-tasks
|
Hi @witash, Let me know if everything looks good to go. |
|
thanks @sh1vam31 this is almost ready...hopefully one last thing this failure appears to be a real test failure, not flakiness (these tests historically have been very flaky, but it didn't pass after multiple retries, and I was able to reproduce the failure locally). |
|
Hi @witash, I've pushed the fix to the test file. |
witash
left a comment
There was a problem hiding this comment.
HI @sh1vam31 just one more change I think;
there's two unrelated changes still in this branch; we really want to keep only changes related to the issue in this branch; test fixes for changes in the branch are ok, but test fixes for other flaky tests need to be in their own branch.
| } | ||
| } | ||
| } | ||
| }, { ignoreReload: true })); |
There was a problem hiding this comment.
This looks unrelated, can you please remove it from this branch if so
| }; | ||
|
|
||
| const INDETERMINATE_FIELDS = ['current', 'uptime', 'date', 'fragmentation', 'node', 'sizes', 'file_size']; | ||
| const INDETERMINATE_FIELDS = ['current', 'uptime', 'date', 'fragmentation', 'node', 'sizes', 'file_size', 'doc_count']; |
There was a problem hiding this comment.
This looks unrelated, can you please remove it from this branch if so
|
Hi @witash, Thank you for the feedback. I have removed the two unrelated changes you pointed out: Restored the { ignoreReload: true } setting in records.spec.js. |
| ok: response.ok, | ||
| headers: response.headers | ||
| }; | ||
| const request = async (options, { debug, maxRetries = 3, initialDelay = 100 } = {}) => { |
There was a problem hiding this comment.
@sh1vam31 these look like unrelated changes. The tests that were specifically for this issue are passing, so lets not add test fixes for other things; and in any case, I wouldn't automatically retry failed requests in this very heavily used file
There was a problem hiding this comment.
@sh1vam31 these look like unrelated changes. The tests that were specifically for this issue are passing, so lets not add test fixes for other things; and in any case, I wouldn't automatically retry failed requests in this very heavily used file
Hi @witash,
Thanks for the feedback! You're absolutely right — the retry logic in request() was leftover from debugging CI failures during development and isn't related to this issue. I've reverted tests/utils/index.js back to its original form. Sorry for the noise!
|
Hi @witash Could you please take another look when you get a chance? Thanks so much for your time and guidance! |
Description
fixes: #10749
moving the tasks_by_contact view from the server-managed design documents to a strictly offline-only design document.
Problem:
The tasks_by_contact view is a high-volume index that is only used by the rules engine on offline clients (mobile/web). When defined in the medic-client design document, the CouchDB server builds and stores this index, consuming significant disk space and processing power for zero benefit on the server.
Solution:
We moved the view definition into the client-side bootstrapper (webapp/src/js/bootstrapper/offline-ddocs). This ensures the view is only created on the client's PouchDB.
Changes:
Moved tasks_by_contact view from server ddocs to webapp/src/js/bootstrapper/offline-ddocs/medic-offline-tasks.
Updated @medic/rules-engine to query the new _design/medic-offline-tasks location.
Cleaned up the medic-client server-side design document by removing the redundant view.
Updated @medic/memdown test harness to correctly load the new offline-only views for integration testing.
Testing:
1. Rules Engine Integration Tests (220 Passing)
2. Manual Bootstrapper Verification
Code review checklist
can_view_old_navigationpermission to see the old design. Test it has appropriate design for RTL languages.License
The software is provided under AGPL-3.0. Contributions to this project are accepted under the same license.