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

fix N+1 query and memory leak in Analytics V2 #1399

Open
donrestarone opened this issue Feb 5, 2023 · 3 comments
Open

fix N+1 query and memory leak in Analytics V2 #1399

donrestarone opened this issue Feb 5, 2023 · 3 comments
Assignees
@alis-khadka
Labels
bug Something isn't working high priority

Comments

@donrestarone
Copy link
Contributor

Describe the bug

Visiting Analytics V2 causes N+1 query. On markedrestaurant.com the N+1 causes a timeout when the range is 3 months or above:

Screen Shot 2023-02-03 at 7 36 50 PM
@donrestarone donrestarone added bug Something isn't working high priority labels Feb 5, 2023
@donrestarone
Copy link
Contributor Author

In addition to the N+1, there maybe an issue with Analytics V2 that causes a memory leak ⚠️ (evidence WIP)

@donrestarone donrestarone changed the title fix N+1 query in Analytics V2 fix N+1 query and memory leak in Analytics V2 Feb 5, 2023
@alis-khadka
Copy link
Contributor

In addition to the N+1, there maybe an issue with Analytics V2 that causes a memory leak ⚠️ (evidence WIP)

@donrestarone
Is there anything remaining to be updated here regarding the "memory leak" part?

@donrestarone
Copy link
Contributor Author

@alis-khadka not really, except that you can observe on top / htop the memory does not get freed after visiting the analytics page. eg; on a fresh boot the app can be using 600MB RAM, and after a user visits the analytics page it will jump to 1.2GB and stay there

This was referenced Feb 17, 2023
Closed
Merged
donrestarone pushed a commit that referenced this issue Mar 11, 2023
@alis-khadka
[bug fix] N+1 query and memory leak in Analytics V2
19b9ff8 
Addresses: #1399
@donrestarone donrestarone mentioned this issue Mar 11, 2023
Merged
donrestarone added a commit that referenced this issue Mar 11, 2023
@donrestarone @alis-khadka
[bug fix] N+1 query in Analytics V2 (#1451)
336836e 
Addresses: #1399

Co-authored-by: Prashant <[email protected]>
@alis-khadka alis-khadka mentioned this issue Mar 14, 2023
Merged
donrestarone pushed a commit that referenced this issue Apr 8, 2023
@alis-khadka
[fix] optimize analytics V2 further + lockdown profiler (#1462)
28fadbc 
Addresses: #1399 and #1452



We can seed the event analytics data at mass by using following command.

``` bash
docker-compose run -e SEED_ANALYTICS=true --rm solutions_app rails db:seed

```


## Profiling Results 📈 🧪 


### Slight improvements to user experience

When analysis going back 1 year is shown, there is a noticeable performance improvement:

<img width="1728" alt="Screen Shot 2023-04-08 at 11 03 31 AM" src="https://user-images.githubusercontent.com/35935196/230728720-31d5d2c0-83e0-4aa2-b3ef-fede1458ff4f.png">

### Less memory & objects used
When a 1 year analysis is shown, less memory and objects are allocated and retained: 

<img width="1728" alt="Screen Shot 2023-04-08 at 11 04 09 AM" src="https://user-images.githubusercontent.com/35935196/230728751-5302c578-4240-4f77-8ac8-166d2046be27.png">

### Garbage collector is running consistently 
on a per request basis, we observe that the garbage collector runs before the request is served. Indicating that used memory has been drained and freed to be used for other requests. 

<img width="1728" alt="Screen Shot 2023-04-08 at 11 06 48 AM" src="https://user-images.githubusercontent.com/35935196/230728822-c1f86bd8-b8fb-45ee-86fa-848c27698a6f.png">
@donrestarone donrestarone mentioned this issue Apr 8, 2023
Closed
@alis-khadka alis-khadka mentioned this issue Apr 9, 2023
Merged
donrestarone pushed a commit that referenced this issue Apr 9, 2023
@alis-khadka
[fix] optimize analytics V2 further + lockdown profiler (#1522)
25016f7 
Addresses: #1399 and #1452

## Profiling Results 📈 🧪 


### Slight improvements to user experience

When analysis going back 1 year is shown, there is a noticeable performance improvement:

<img width="1728" alt="Screen Shot 2023-04-08 at 11 03 31 AM" src="https://user-images.githubusercontent.com/35935196/230728720-31d5d2c0-83e0-4aa2-b3ef-fede1458ff4f.png">

### Less memory & objects used
When a 1 year analysis is shown, less memory and objects are allocated and retained: 

<img width="1728" alt="Screen Shot 2023-04-08 at 11 04 09 AM" src="https://user-images.githubusercontent.com/35935196/230728751-5302c578-4240-4f77-8ac8-166d2046be27.png">

### Garbage collector is running consistently 
on a per request basis, we observe that the garbage collector runs before the request is served. Indicating that used memory has been drained and freed to be used for other requests. 

<img width="1728" alt="Screen Shot 2023-04-08 at 11 06 48 AM" src="https://user-images.githubusercontent.com/35935196/230728822-c1f86bd8-b8fb-45ee-86fa-848c27698a6f.png">
@donrestarone donrestarone mentioned this issue Apr 9, 2023
Merged
donrestarone added a commit that referenced this issue Apr 9, 2023
@donrestarone @alis-khadka
[fix] optimize analytics V2 further + lockdown profiler (#1522) (#1523)
3d13e4c 
Addresses: #1399 and #1452

## Profiling Results 📈 🧪 


### Slight improvements to user experience

When analysis going back 1 year is shown, there is a noticeable performance improvement:

<img width="1728" alt="Screen Shot 2023-04-08 at 11 03 31 AM" src="https://user-images.githubusercontent.com/35935196/230728720-31d5d2c0-83e0-4aa2-b3ef-fede1458ff4f.png">

### Less memory & objects used
When a 1 year analysis is shown, less memory and objects are allocated and retained: 

<img width="1728" alt="Screen Shot 2023-04-08 at 11 04 09 AM" src="https://user-images.githubusercontent.com/35935196/230728751-5302c578-4240-4f77-8ac8-166d2046be27.png">

### Garbage collector is running consistently 
on a per request basis, we observe that the garbage collector runs before the request is served. Indicating that used memory has been drained and freed to be used for other requests. 

<img width="1728" alt="Screen Shot 2023-04-08 at 11 06 48 AM" src="https://user-images.githubusercontent.com/35935196/230728822-c1f86bd8-b8fb-45ee-86fa-848c27698a6f.png">


# Real life example, Marked Restaurant

## Resource usage

comparison of memory / CPU usage before and after patch

### Baseline 🆎 

The "resting memory rate" for a high traffic Violet system is around 600MB: 
<img width="1728" alt="Screen Shot 2023-04-09 at 11 44 16 AM" src="https://user-images.githubusercontent.com/35935196/230782692-84553698-fc07-4392-b7e6-45cda169d370.png">

### Before ⏪ 

Viewing the 1 year analysis: 

<img width="1728" alt="Screen Shot 2023-04-09 at 11 42 23 AM" src="https://user-images.githubusercontent.com/35935196/230782749-11df1621-27ce-4b08-bf65-3625e5eddf7f.png">

Viewing the 1 month analysis: 

<img width="1728" alt="Screen Shot 2023-04-09 at 11 42 08 AM" src="https://user-images.githubusercontent.com/35935196/230782771-8801aa10-13c3-4bc5-82bc-70d09924000b.png">

We observe 1.2 GB of memory use (double the resting rate)

Profiler result 📈 
While attempting to run the memory profiler on the 1 year analysis, we observed 3GB+ of memory usage ⚠️ 

<img width="1728" alt="Screen Shot 2023-04-09 at 11 43 51 AM" src="https://user-images.githubusercontent.com/35935196/230782803-0ca221c6-976b-4e28-a669-67f8e196f6d0.png">

⭐  After the test was run, puma was restarted to ensure system stability

### After ⏩ 

Viewing the 1 year analysis: 
<img width="1728" alt="Screen Shot 2023-04-09 at 12 08 06 PM" src="https://user-images.githubusercontent.com/35935196/230783850-ee5963b2-7280-4323-9dbf-73812671b040.png">

We observe 720MB of memory use

Viewing the 1 month analysis:
<img width="1728" alt="Screen Shot 2023-04-09 at 12 11 44 PM" src="https://user-images.githubusercontent.com/35935196/230783889-8fb54846-47d0-487f-9480-3ded87fc7217.png">

We observe 850 MB of memory use 


Profiler result 📈 
We observe 900MB of memory use when profiling the 1 year analysis
<img width="1728" alt="Screen Shot 2023-04-09 at 12 10 11 PM" src="https://user-images.githubusercontent.com/35935196/230783899-5a66ded5-8529-4900-aab2-9003d89e06b1.png">

### Result

The system is now consuming memory in analytics V2 comparable to its resting memory usage rate. 







Co-authored-by: Prashant <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@alis-khadka alis-khadka

Labels
bug Something isn't working high priority
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@alis-khadka @donrestarone