fix: Makes ICacheClient return type promise and changes LRUCache accordingly #2410

konstantinabl · 2024-04-25T11:37:58Z

Description:
Currently, the redisCache service is implementing the ICacheClient interface. However, in this interface the methods are not asynchronous as it should be when we are using redis. The goal of this PR is to change the interface and make the methods asynchronous as well as adding specific interface for redis and the internal cache.

Main Files Changed

ICacheClient.ts: Added Promise return types.
IRedisCacheClient.ts: Added an interface for the RedisCache service.
localLRUCache.ts: Changed method return types to Promise.
redisCache.ts: Changed method return types to Promise and added 'await' for asynchronous methods.
eth.ts: Updated methods to use Promise return types and added 'await'.
cacheService.ts: Made methods asynchronous, added return types, and implemented conditions to use internalCache when Redis is unavailable.
Test files: Updated tests to comply with new logic.

Type of Change:

Related issue(s):
Fixes #2387

sonarqubecloud · 2024-04-25T11:59:36Z

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

github-actions · 2024-07-16T15:53:02Z

Tests

    3 files 165 suites 14s ⏱️
885 tests 884 ✔️ 1 💤 0 ❌
898 runs 897 ✔️ 1 💤 0 ❌

Results for commit 6220250.

♻️ This comment has been updated with latest results.

github-actions · 2024-07-16T16:10:50Z

Acceptance Tests

  19 files 246 suites 30m 1s ⏱️
613 tests 605 ✔️ 4 💤 4 ❌
806 runs 798 ✔️ 4 💤 4 ❌

Results for commit 6220250.

♻️ This comment has been updated with latest results.

.gitignore

packages/relay/src/lib/clients/cache/ICacheClient.ts

packages/relay/src/lib/clients/cache/localLRUCache.ts

packages/relay/src/lib/services/cacheService/cacheService.ts

victor-yanev

LG, I left a few suggestions

packages/relay/src/lib/clients/cache/ICacheClient.ts

packages/relay/src/lib/clients/cache/IRedisCacheClient.ts

packages/server/tests/acceptance/cacheService.spec.ts

github-actions · 2024-07-30T10:23:33Z

🚨 Memory Leak Detected 🚨

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

Details

📊 Memory Leak Detection Report 📊

GC Type: MarkSweepCompact
Cost: 30,396.7 ms

Heap Statistics (before vs after executing the test):

Total Heap Size: increased with 524.29 KB
Total Heap Size Executable: no changes
Total Physical Size: increased with 524.29 KB
Total Available Size: increased with 2.56 MB
Total Global Handles Size: no changes
Used Global Handles Size: decreased with 64.00 bytes
Used Heap Size: decreased with 2.57 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 524.29 KB
- Space Used Size: increased with 540.25 KB
- Space Available Size: increased with 217.37 KB
- Physical Space Size: increased with 524.29 KB

Recommendations

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

Signed-off-by: Konstantina Blazhukova <[email protected]>

…ined via ? operator Signed-off-by: Konstantina Blazhukova <[email protected]>

Signed-off-by: Konstantina Blazhukova <[email protected]>

sonarqubecloud · 2024-08-06T15:03:08Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.7% Duplication on New Code

See analysis details on SonarCloud

codecov · 2024-08-06T15:13:54Z

Codecov Report

Attention: Patch coverage is 88.15789% with 9 lines in your changes missing coverage. Please review.

Project coverage is 77.86%. Comparing base (750f590) to head (6220250).
Report is 1 commits behind head on main.

Files	Patch %	Lines
...elay/src/lib/services/cacheService/cacheService.ts	61.11%	4 Missing and 3 partials ⚠️
...kages/relay/src/lib/clients/cache/localLRUCache.ts	66.66%	1 Missing ⚠️
packages/relay/src/lib/eth.ts	96.29%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2410      +/-   ##
==========================================
+ Coverage   77.44%   77.86%   +0.41%     
==========================================
  Files          43       43              
  Lines        3330     3329       -1     
  Branches      707      706       -1     
==========================================
+ Hits         2579     2592      +13     
+ Misses        505      491      -14     
  Partials      246      246

Flag	Coverage Δ
relay	`78.95% <88.15%> (+0.60%)`	⬆️
server	`74.61% <ø> (-0.31%)`	⬇️
ws-server	`61.70% <ø> (ø)`

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

Files	Coverage Δ
packages/relay/src/lib/clients/cache/redisCache.ts	`44.44% <100.00%> (+6.34%)`	⬆️
packages/relay/src/lib/clients/mirrorNodeClient.ts	`88.43% <100.00%> (ø)`
packages/relay/src/lib/clients/sdkClient.ts	`50.74% <100.00%> (ø)`
.../lib/services/ethService/ethCommonService/index.ts	`89.14% <100.00%> (+0.08%)`	⬆️
.../lib/services/ethService/ethFilterService/index.ts	`80.00% <100.00%> (+1.25%)`	⬆️
...kages/relay/src/lib/clients/cache/localLRUCache.ts	`78.78% <66.66%> (+2.31%)`	⬆️
packages/relay/src/lib/eth.ts	`81.57% <96.29%> (ø)`
...elay/src/lib/services/cacheService/cacheService.ts	`63.76% <61.11%> (+11.06%)`	⬆️

... and 1 file with indirect coverage changes

georgi-l95

LG, a step in the right direction.

konstantinabl linked an issue Apr 25, 2024 that may be closed by this pull request

Interface for Redis and LRU cache (ICacheClient) has void return types instead of Promise<void> #2387

Closed

konstantinabl force-pushed the 2387-interface-for-redis-and-lru-cache-icacheclient-has-void-return-types-instead-of-promisevoid branch from bad263e to 48370eb Compare April 25, 2024 11:39

konstantinabl force-pushed the 2387-interface-for-redis-and-lru-cache-icacheclient-has-void-return-types-instead-of-promisevoid branch 2 times, most recently from 1291746 to 5ea2204 Compare July 16, 2024 10:54

konstantinabl changed the title ~~Makes ICacheClient return type promise and changes LRUCache accordingly~~ fix: Makes ICacheClient return type promise and changes LRUCache accordingly Jul 16, 2024

konstantinabl added the bug Something isn't working label Jul 16, 2024

konstantinabl added this to the 0.53.0 milestone Jul 16, 2024

konstantinabl marked this pull request as ready for review July 17, 2024 13:21

konstantinabl requested review from AlfredoG87, ebadiere, Nana-EC, a team and lukelee-sl as code owners July 17, 2024 13:21

konstantinabl requested review from tomzhenghedera and leninmehedy July 17, 2024 13:21

victor-yanev reviewed Jul 18, 2024

View reviewed changes

.gitignore Show resolved Hide resolved

victor-yanev reviewed Jul 18, 2024

View reviewed changes

victor-yanev reviewed Jul 23, 2024

View reviewed changes

packages/relay/src/lib/services/cacheService/cacheService.ts Outdated Show resolved Hide resolved

victor-yanev reviewed Jul 23, 2024

View reviewed changes

packages/relay/src/lib/services/cacheService/cacheService.ts Show resolved Hide resolved

victor-yanev reviewed Jul 23, 2024

View reviewed changes

packages/relay/src/lib/services/cacheService/cacheService.ts Outdated Show resolved Hide resolved

victor-yanev reviewed Jul 23, 2024

View reviewed changes

packages/relay/src/lib/services/cacheService/cacheService.ts Outdated Show resolved Hide resolved

victor-yanev requested changes Jul 23, 2024

View reviewed changes

konstantinabl requested a review from victor-yanev July 26, 2024 12:46

victor-yanev reviewed Jul 29, 2024

View reviewed changes

packages/relay/src/lib/clients/cache/ICacheClient.ts Outdated Show resolved Hide resolved

victor-yanev reviewed Jul 29, 2024

View reviewed changes

packages/relay/src/lib/clients/cache/IRedisCacheClient.ts Show resolved Hide resolved

victor-yanev reviewed Jul 29, 2024

View reviewed changes

packages/server/tests/acceptance/cacheService.spec.ts Show resolved Hide resolved

konstantinabl force-pushed the 2387-interface-for-redis-and-lru-cache-icacheclient-has-void-return-types-instead-of-promisevoid branch from bf2b252 to 9740166 Compare July 30, 2024 08:47

victor-yanev previously approved these changes Jul 30, 2024

View reviewed changes

konstantinabl dismissed victor-yanev’s stale review via 83ff74e July 30, 2024 10:16

konstantinabl force-pushed the 2387-interface-for-redis-and-lru-cache-icacheclient-has-void-return-types-instead-of-promisevoid branch from 9740166 to 83ff74e Compare July 30, 2024 10:16

konstantinabl force-pushed the 2387-interface-for-redis-and-lru-cache-icacheclient-has-void-return-types-instead-of-promisevoid branch from 83ff74e to d2700ba Compare July 31, 2024 10:42

konstantinabl added 14 commits August 6, 2024 17:04

Makes ICacheClient return type promise and changes LRUCache accordingly

baafd4a

Signed-off-by: Konstantina Blazhukova <[email protected]>

Adds disconnect in separate interface and remove it from LRUCache

26e2703

Signed-off-by: Konstantina Blazhukova <[email protected]>

Adds correct method to filterService when using cache

c98697f

Signed-off-by: Konstantina Blazhukova <[email protected]>

Fixes failing unit tests

65267e1

Signed-off-by: Konstantina Blazhukova <[email protected]>

Fixes sonar cloud issues

0090fd2

Signed-off-by: Konstantina Blazhukova <[email protected]>

Removes unnecessary ioredis instance from cacheService acceptance test

4fff033

Signed-off-by: Konstantina Blazhukova <[email protected]>

Removes uneccesarry undefined type in ICacheClient interface

b79ccf4

Signed-off-by: Konstantina Blazhukova <[email protected]>

Removes uneccessary Promise wrap in localLRUCache

510dc44

Signed-off-by: Konstantina Blazhukova <[email protected]>

Renames getSharedWithFallback method and addresses PR comments

9130a13

Signed-off-by: Konstantina Blazhukova <[email protected]>

Removes old shared parameter for shared cache

4d4ac4b

Signed-off-by: Konstantina Blazhukova <[email protected]>

Removes undefined type for ttl parameter, its implicitly set to undef…

d2a9611

…ined via ? operator Signed-off-by: Konstantina Blazhukova <[email protected]>

Adds missing copyright

d56e113

Signed-off-by: Konstantina Blazhukova <[email protected]>

Reverts back to old approach of instantiating internal and shared cache

dd05c12

Signed-off-by: Konstantina Blazhukova <[email protected]>

fixes merge conflict

7a53da8

Signed-off-by: Konstantina Blazhukova <[email protected]>

konstantinabl force-pushed the 2387-interface-for-redis-and-lru-cache-icacheclient-has-void-return-types-instead-of-promisevoid branch from 5b81b10 to 7a53da8 Compare August 6, 2024 14:04

Removes unecessary check from getFromSharedCache

6220250

Signed-off-by: Konstantina Blazhukova <[email protected]>

georgi-l95 approved these changes Aug 7, 2024

View reviewed changes

konstantinabl requested a review from victor-yanev August 7, 2024 08:20

victor-yanev approved these changes Aug 7, 2024

View reviewed changes

konstantinabl merged commit d74f13b into main Aug 7, 2024
37 of 38 checks passed

konstantinabl deleted the 2387-interface-for-redis-and-lru-cache-icacheclient-has-void-return-types-instead-of-promisevoid branch August 7, 2024 09:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Makes ICacheClient return type promise and changes LRUCache accordingly #2410

fix: Makes ICacheClient return type promise and changes LRUCache accordingly #2410

konstantinabl commented Apr 25, 2024 •

edited

Loading

sonarqubecloud bot commented Apr 25, 2024

github-actions bot commented Jul 16, 2024 •

edited

Loading

github-actions bot commented Jul 16, 2024 •

edited

Loading

victor-yanev left a comment

github-actions bot commented Jul 30, 2024

sonarqubecloud bot commented Aug 6, 2024

codecov bot commented Aug 6, 2024

georgi-l95 left a comment

fix: Makes ICacheClient return type promise and changes LRUCache accordingly #2410

fix: Makes ICacheClient return type promise and changes LRUCache accordingly #2410

Conversation

konstantinabl commented Apr 25, 2024 • edited Loading

sonarqubecloud bot commented Apr 25, 2024

Quality Gate passed

github-actions bot commented Jul 16, 2024 • edited Loading

Tests

github-actions bot commented Jul 16, 2024 • edited Loading

Acceptance Tests

victor-yanev left a comment

Choose a reason for hiding this comment

github-actions bot commented Jul 30, 2024

🚨 Memory Leak Detected 🚨

Details

Recommendations

sonarqubecloud bot commented Aug 6, 2024

Quality Gate passed

codecov bot commented Aug 6, 2024

Codecov Report

georgi-l95 left a comment

Choose a reason for hiding this comment

konstantinabl commented Apr 25, 2024 •

edited

Loading

github-actions bot commented Jul 16, 2024 •

edited

Loading

github-actions bot commented Jul 16, 2024 •

edited

Loading