Skip to content

Type error CdoTutorials can't be referred to - during initialize in database.rb#54703

Merged
vijayamanohararaj merged 3 commits into
stagingfrom
vijaya/honey_badger
Nov 14, 2023
Merged

Type error CdoTutorials can't be referred to - during initialize in database.rb#54703
vijayamanohararaj merged 3 commits into
stagingfrom
vijaya/honey_badger

Conversation

@vijayamanohararaj

@vijayamanohararaj vijayamanohararaj commented Nov 3, 2023

Copy link
Copy Markdown
Contributor

Issue: TypeError thrown upon first instance of deserializing contents from cache for tables that are populated dynamically through data from CSV files.

More details:

  1. Some data around HoC activities are populated from csv files upon application start. These are then cached in rails memory cache for subsequent requests to avoid repeated deserialization from CSV files.
  2. The model for these entries are also dynamically generated on first load - which are defined in pegasus/data/static_models.rb
  3. During some situations (probably when there are multiple threads that are sharing the memory cache), the cache is already hydrated, but the data model hasn't been loaded, leading to a type error.

Fix: Explicitly creating a new instance of the dynamic data models before cache look up to ensure the model is loaded.

Links

JIRA: https://codedotorg.atlassian.net/browse/ACQ-918?atlOrigin=eyJpIjoiOTlmMjYzNzExOTk0NDBkODhiMWI3OTA5M2I2MDY5MzQiLCJwIjoiaiJ9

Testing story

To repro this locally, I temporarily initialized the CDO.Cache object to a file store based cache instead of memory cache, so that cached data would persist between application restarts. With that change, observed the following behavior

  1. Restart application
  2. Hit "http://localhost:3000/api/hour/begin_mc.png" -> successful with cache getting hydrated
  3. Subsequent calls to http://localhost:3000/api/hour/begin_mc.png would serve data from cache
  4. Restart application
  5. Hit "http://localhost:3000/api/hour/begin_mc.png" -> will fail with _NameError at /api/hour/begin_mc.png
    uninitialized constant CdoTutorials
  6. Subsequent calls to http://localhost:3000/api/hour/begin_mc.png would be successful

This test setup seemed to be hitting the same problem from the error message. With the change in this PR, the above scenario was successful in step #5

Deployment strategy

None - will keep close eye on HoneyBadger once this goes out in DTP

Follow-up work

Given the local repro isn't exactly the same as production, this change might not fix the underlying issue. If a drop in the HoneyBadger error isn't observed, will go back to evaluating a different solution.

Another solution we considered was to serialize and deserialize the cached data to JSON. Given the JSON conversion can be expensive in terms of time, I deprioritized that solution. But, will revisit if this doesn't help.

Privacy

N/A

Security

N/A

Caching

Issue is in cache retrieval, additional caching not applicable here

PR Checklist:

  • Tests provide adequate coverage
  • Privacy and Security impacts have been assessed
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

@vijayamanohararaj vijayamanohararaj changed the title Type error CdoTutorials can't be referred to - raised in Honey badger Type error CdoTutorials can't be referred to - during initialize in database.rb Nov 7, 2023
@vijayamanohararaj vijayamanohararaj marked this pull request as ready for review November 8, 2023 17:38
@vijayamanohararaj vijayamanohararaj requested review from a team November 8, 2023 17:39

@Hamms Hamms left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great investigation! Testing story makes sense to me as our best attempt at approximating what's going on in production.

I have a couple minor nitpicky questions about the precise implementation of this fix, but in general it LGTM; thank you! I'm excited to see what effect this has on our error rate 🤞

Comment thread pegasus/src/database.rb Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants