[#11015] Break down question result fetch by section for scalability #11017
Conversation
moziliar
force-pushed
the
11015-break-down-question-result-fetch-by-section-for-scalability
branch
from
336b198
to
f24dfd0
Compare
…section-for-scalability
There was a problem hiding this comment.
The code looks good to me! 👍
Do you know what might be causing the e2e tests to fail? I've checked out the error logs but I'm still unsure about how we can fix it.
Anyways, I'm currently adding the circular progress indicator (as planned in #11015) - will make a PR against this branch soon.
|
Thanks @daongochieu2810 @teikjun ! I am still investigating the e2e failure too. It will take quite awhile unfortunately. Will update again if there's anything to change in order to pass the test. |
|
@teikjun i've identified the problem and it appears that the problem also involves the unfinished Question Stats type definition in the frontend. @wkurniawan07 I see that you left the TODO there, should we tackle that first before carrying on with this PR? The issue with the current code is that it naively overwrites the current question's data with each section's data; it should be an |
|
I highly doubt it's the issue. The only time where statistics is obtained from back-end is when the question type is contribution. All other statistics are calculated on demand based on available data. |
|
@wkurniawan07 yup. Do you think I can just jsonify the string and perform concat before throwing it back as a string as a temporary measure? |
moziliar
force-pushed
the
11015-break-down-question-result-fetch-by-section-for-scalability
branch
2 times, most recently
from
1556af4
to
e8550e4
Compare
moziliar
force-pushed
the
11015-break-down-question-result-fetch-by-section-for-scalability
branch
from
7806ca4
to
6d7bd35
Compare
|
Removing on hold status as #11037 closed due to inactivity |
madanalogy
added
s.Ongoing
|
Guys, This PR seems to be stalling (no activities for the past 7 days). 🐌 😢 |
|
Guys, This PR seems to be stalling (no activities for the past 8 days). 🐌 😢 |
|
@moziliar kind reminder on this PR |
|
Will pick it up within the week. Thanks for the reminder. |
|
Guys, This PR seems to be stalling (no activities for the past 7 days). 🐌 😢 |
|
Guys, This PR seems to be stalling (no activities for the past 8 days). 🐌 😢 |
|
@moziliar kind reminder on this PR. |
|
Guys, This PR seems to be stalling (no activities for the past 7 days). 🐌 😢 |
1 similar comment
|
Guys, This PR seems to be stalling (no activities for the past 7 days). 🐌 😢 |
|
@moziliar kind reminder on this PR. |
|
Sorry for leaving this PR hanging for so long; I might need some time to reconcile it with a recent change. |
|
Guys, This PR seems to be stalling (no activities for the past 8 days). 🐌 😢 |
|
Guys, This PR seems to be stalling (no activities for the past 7 days). 🐌 😢 |
4 similar comments
|
Guys, This PR seems to be stalling (no activities for the past 7 days). 🐌 😢 |
|
Guys, This PR seems to be stalling (no activities for the past 7 days). 🐌 😢 |
|
Guys, This PR seems to be stalling (no activities for the past 7 days). 🐌 😢 |
|
Guys, This PR seems to be stalling (no activities for the past 7 days). 🐌 😢 |
|
Guys, This PR seems to be stalling (no activities for the past 9 days). 🐌 😢 |
|
Guys, This PR seems to be stalling (no activities for the past 8 days). 🐌 😢 |
|
Guys, This PR seems to be stalling (no activities for the past 7 days). 🐌 😢 |
3 similar comments
|
Guys, This PR seems to be stalling (no activities for the past 7 days). 🐌 😢 |
|
Guys, This PR seems to be stalling (no activities for the past 7 days). 🐌 😢 |
|
Guys, This PR seems to be stalling (no activities for the past 7 days). 🐌 😢 |
|
Closed in favour of #11567 |
Fixes #11015
Outline of Solution
Using the existing group by section toolkit, this PR breaks down per question result fetch into one or more result fetch by question and section.
By the nature of the section size limit, we can upper bound the sheer number of responses fetched per api call, therefore mitigating the OOM and GAE timeout issue caused by large courses where a question could consist of too many responses.
Pending stress test to confirm if this solution solves any reasonably sized question response set per section. The prime factor for such consideration is the size of the response (say, essay question which consists of 500 words minimally).
Update
Also initiates the splitting of backend response fetch by giver and receiver. A trichotomy enum
ResultFetchTypealong the data path is used as a common object propagated along UI, Logic and Storage layers for a cleaner look.The rationale behind the backend changes is to upper bound the number of responses fetched via this way, utilizing the section size limit of 100. By allowing fetching by section but for both giver and receiver, the size of the return could be higher considering a chained fetch.