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.

Sign up for GitHub

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

Jump to bottom

[Feature Request]: AndroidActivityHandler shouldn't use Redis or schema conversion #21253

Open
BenHenning opened this issue Nov 14, 2024 · 1 comment
Open

[Feature Request]: AndroidActivityHandler shouldn't use Redis or schema conversion #21253

BenHenning opened this issue Nov 14, 2024 · 1 comment
Labels
enhancement Label to indicate an issue is a feature/improvement Impact: High Blocks or significantly slows down a core workflow. Work: High It's not clear what the solution is; will need investigation.

Comments

@BenHenning
Copy link
Member

Is your feature request related to a problem? Please describe.

This corresponds to the lesson download script used by the Android team, and represents two different problems observed:

  1. The download script can sometimes request very large numbers of lesson structures (>50k) which can quickly overload web's Redis cache. The simplest solution seems to be to simply remove caching for requests coming in via the download script since these can potentially enumerate the entirety of Oppia's curated curriculum, logically invalidating the cache.
  2. Requested assets should never be schema-converted since the script uses a fallback failure recovery mechanism if it encounters newer lessons that are incompatible with the Android data pipeline. Requesting older versions due to a schema incompatibility current results in a failure case due to the older versions also ending up schema-converted (which essentially causes the script to try to download every exploration version which quickly leads to a very large number of downloads).

Describe the solution (or solutions) you'd like

  1. Redis caching should be disabled for lesson structure requests coming via AndroidActivityHandler.
  2. Lesson structures returned by AndroidActivityHandler should not undergo schema version.

Note for (2) that if specific structure suggestions need to be provided, please follow up. At the moment, I'm assuming (1) and (2) can be tackled together by introducing a different retrieval mechanism that essentially directly retrieves lesson structures from the data store without Redis or domain object conversion, and thereafter converts the structures to a basic raw dict structure that can be rendered to JSON.

Describe alternatives you've considered and rejected

There aren't obvious alternatives here.

Additional context

No response
@BenHenning BenHenning added triage needed enhancement Label to indicate an issue is a feature/improvement labels Nov 14, 2024
@BenHenning
Copy link
Member Author

BenHenning commented Nov 14, 2024 •
edited
Loading

/cc @seanlip. I think we talked about this a few times in past discussions, but it's becoming quite apparent that this will be a repeated pain point when trying to use the asset download script after a schema upgrade (as it leads to the script trying to download 10s of thousands of exploration versions which quickly overloads the server's Redis cache). Plus, the compatibility algorithm in the script will never work without the "fetch-without-conversion" flow implemented.

For what it's worth, this support will also be useful for when in the future we move the compatibility fetching logic from the script to Oppia web (and eventual proto conversion). What exact form this takes may change, but the same concept will still be needed (that is, the ability to fetch raw structures and make the determination for conversion separately from the web domain objects).

Edit: actually that last point may be wrong. It occurs to me that we can alternatively perform conversion and simply update the code that then converts the domain structures to protos. This perhaps means that solving (2) in this issue actually is script-specific, so perhaps it's not worth doing unless it can be done with minimal effort.

@BenHenning BenHenning changed the title [Feature Request]: AndroidActivityHandler shouldn't use redis or schema conversion [Feature Request]: AndroidActivityHandler shouldn't use Redis or schema conversion Nov 14, 2024
BenHenning added a commit to oppia/oppia-android that referenced this issue Nov 14, 2024
@BenHenning
Fix download script and improve robustness.
e24fd24 
This incldues several things:
1. Add support for state schema version 56.
2. Make cache dir optional for the structure list downloader.
3. Add a fail-fast mechanism for incompatible state schema versions.
4. Add better error handling for when Oppia web's Redis cache fills up,
   preventing continued downloads.
5. Remove some dead/unused code.
6. Reinstate a localization check and add more exemptions for current
   content.
7. Reenable multi-version fetching and improve its robustness by
   accounting for the response missing certain versions.

Note that oppia/oppia#21253 was filed to track
work related to (3) and (4).
@seanlip seanlip added Impact: High Blocks or significantly slows down a core workflow. Work: High It's not clear what the solution is; will need investigation. labels Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement Label to indicate an issue is a feature/improvement Impact: High Blocks or significantly slows down a core workflow. Work: High It's not clear what the solution is; will need investigation.
Projects
LaCE Quality Team
Status: Todo
Milestone
No milestone
Development

No branches or pull requests
2 participants
@seanlip @BenHenning