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.

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

feat: redis cache should fallback to internal in case of error #2861

Merged
merged 11 commits into from
Aug 26, 2024

Conversation

natanasow
Copy link
Collaborator

@natanasow natanasow commented Aug 21, 2024

Description:

Currently, in the cacheService.ts the methods are written in a way that suggests that if an error occurs in Redis during set and getAsync operations we will fallback to the internal cache. However, this doesn't happen.

Related issue(s):

Fixes #2788

Notes for reviewer:

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

@natanasow natanasow self-assigned this Aug 21, 2024
@natanasow natanasow added this to the 0.55.0 milestone Aug 21, 2024
@natanasow natanasow added the enhancement New feature or request label Aug 21, 2024
Copy link

github-actions bot commented Aug 21, 2024

Acceptance Tests

  21 files  293 suites   30m 44s ⏱️
615 tests 606 ✔️ 4 💤 5
996 runs  985 ✔️ 4 💤 7

Results for commit 0e02e97.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Aug 22, 2024

Tests

       3 files     207 suites   17s ⏱️
1 052 tests 1 051 ✔️ 1 💤 0
1 064 runs  1 063 ✔️ 1 💤 0

Results for commit 0e02e97.

♻️ This comment has been updated with latest results.

Signed-off-by: nikolay <[email protected]>
Signed-off-by: nikolay <[email protected]>
@natanasow natanasow marked this pull request as ready for review August 22, 2024 14:11
Signed-off-by: nikolay <[email protected]>
Signed-off-by: nikolay <[email protected]>
Copy link

🚨 Memory Leak Detected 🚨

A potential memory leak has been detected in the test titled should execute "eth_chainId". This may impact the application's performance and stability.

Details

📊 Memory Leak Detection Report 📊

GC Type: Scavenge
Cost: 11,608.5 ms

Heap Statistics (before vs after executing the test):

  • Total Heap Size: increased with 1.95 MB
  • Total Heap Size Executable: no changes
  • Total Physical Size: increased with 1.95 MB
  • Total Available Size: increased with 10.79 MB
  • Total Global Handles Size: no changes
  • Used Global Handles Size: decreased with 352.00 bytes
  • Used Heap Size: decreased with 10.28 MB
  • Heap Size Limit: no changes
  • Malloced Memory: no changes
  • External Memory: no changes
  • Peak Malloced Memory: no changes

Heap Space Statistics (before vs after executing the test):

  • Old Space:

    • Space Size: increased with 2.10 MB
    • Space Used Size: increased with 3.08 MB
    • Space Available Size: decreased with 1.05 MB
    • Physical Space Size: increased with 2.10 MB
  • Large Object Space:

    • Space Size: increased with 434.18 KB
    • Space Used Size: increased with 421.07 KB
    • Space Available Size: no changes
    • Physical Space Size: increased with 434.18 KB

Recommendations

Please investigate the memory allocations in this test, focusing on objects that are not being properly deallocated.

konstantinabl
konstantinabl previously approved these changes Aug 26, 2024
Signed-off-by: nikolay <[email protected]>
Copy link

@ebadiere ebadiere merged commit cfb0760 into main Aug 26, 2024
40 checks passed
@ebadiere ebadiere deleted the 2788-redis-cache-should-fallback-to-internal branch August 26, 2024 13:38
Copy link

codecov bot commented Aug 26, 2024

Codecov Report

Attention: Patch coverage is 91.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 83.05%. Comparing base (af09470) to head (0e02e97).
Report is 6 commits behind head on main.

Files Patch % Lines
...elay/src/lib/services/cacheService/cacheService.ts 91.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2861      +/-   ##
==========================================
+ Coverage   82.66%   83.05%   +0.38%     
==========================================
  Files          49       49              
  Lines        3554     3559       +5     
  Branches      751      751              
==========================================
+ Hits         2938     2956      +18     
+ Misses        370      357      -13     
  Partials      246      246              
Flag Coverage Δ
relay 82.57% <83.33%> (+0.73%) ⬆️
server 81.19% <ø> (ø)
ws-server 97.87% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...elay/src/lib/services/cacheService/cacheService.ts 97.22% <91.66%> (+20.41%) ⬆️

... and 6 files with indirect coverage changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Redis cache should fallback to internal on set and get operations
4 participants