Iridescent-CM / technovation-app

The team submission platform for the Technovation Challenge
https://technovationchallenge.org
GNU General Public License v3.0
7 stars 4 forks source link

HOTFIX - BUG: When trying to select onboarded mentors matched with a team the platform errors out #4783

Closed viviancan closed 1 month ago

viviancan commented 3 months ago

Original support request here

dboyer commented 3 months ago

The same error page is happening if you set these search parameters at /admin/participants#/

shaun-technovation commented 3 months ago

Using the "team matching" option is causing an n+1 query - for every account returned it is making an additional query (to the chapter ambassadors table), i.e. if there are 100 records being returned it cause an additional 100 queries to be made. This definitely would impact a timeout. This fix was easy to find and address using the Bullet gem, which I have installed and been playing with locally. Probably not as urgent given the workarounds discussed at stand-up, but nonetheless good to start addressing our n+1 issues and specifically this issue, IMHO.

shaun-technovation commented 3 months ago

This fix is on QA, but there's nothing really to test; we'll really only know if it fixes the timeout issue once it lands on Production.

dboyer commented 3 months ago

I did the participant search on QA but as you said, it's not really testable until production.

viviancan commented 3 months ago

Confirmed this is working on PROD

I selected mentors, matched with a team, onboarded

shaun-technovation commented 3 months ago

Awesome, thanks for testing this on Production @viviancan! I'm glad this fixed worked!

dboyer commented 3 months ago

I'm still getting the error page when I search on production for mentors who are onboarded and matched with a team.

Screen Shot 2024-07-01 at 11 48 36 AM
shaun-technovation commented 3 months ago

I'm still getting the error page when I search on production for mentors who are onboarded and matched with a team.

Ah, that's a bummer, I'm getting the same error too.

I see two more potential things to address.

We could try splitting up the "matched with a team" filter, one filter for "students matched with a team" and one filter for "mentors matched with a team". Right now, we're making two separate queries, one for finding all mentors matched w/ a team, and one for finding all students; and then we combine the results to find accounts. Splitting them up would make it more efficient, not sure it would fix the issue, but it should help.

Also, we're querying the memberships table based on member_type (where member_type is for a mentor), adding an index on this column should help w/ this query too.

shaun-technovation commented 3 months ago

I'll move this one back to the Product Backlog so we don't forgot about it.

shaun-technovation commented 3 months ago

I have a few more improvements for this one. I addressed another n+1 query and added the index for member_type.

I tested with production data locally, and it never timed out, but I do see that it took more than 3 seconds and somehow I think that's the maximum time on production to cause a timeout. 🤔 Anywho, the time improved by at least a second after the improvements.

 Rendered datagrid/_datagrid.en.html.erb (Duration: 31731.0ms | Allocations: 1836386)

After the improvements:

 Rendered datagrid/_datagrid.en.html.erb (Duration: 16607.7ms | Allocations: 1842005)

I think the real issue is the way we try to find people "matched with a team". We query the memberships table twice, once for students and once for mentors. I tried improving/consolidating these queries, but I couldn't get the same number of records returned. I think how specific this filter is is indicative that we need a separate mentor and student datagrid pages. This way the mentors datagrid would only be searching for mentors matched with a team and not include students.

shaun-technovation commented 3 months ago

These new improvements are now on QA, but like the last attempt at a fix, we won't really know if it works until it lands on Production.

If this doesn't work, I can revisit the queries I was looking at today, I'm pretty sure I can get those working now that I've had more time to think about them.

shaun-technovation commented 3 months ago

The latest fix didn't work.

I think the real way to address this is separating the matched with a team filter on the participants datagrid (one for students matched with a team and one for mentors matched with a team).

Also, having a separate datagrid for mentors (#4842), including the "matched with a team" filter is moving us in the right direction too.

shaun-technovation commented 2 months ago

Another potential fix for this is on QA.

I added two additional filters:

The existing "Matched with a team" filter is still there, when this option is picked, it finds all students and mentors matched with a team; for this issue finding all students is completely unnecessary, so having the separate filters will definitely be an improvement, but whether it will fix the issue, we'll see when it gets to Production.

To test this, the results from "Matched with a team" and "Yes, fully onboarded" mentors should match the results from "Mentors matched with a team" and "Yes, fully onboarded" mentors.

After this looks good on QA, I'll follow the hotfix process to get this to Production.

dboyer commented 2 months ago

Tested on QA and confirmed that I get 15 results when searching:

If I search mentor profile type and matched with a team, I get 20 results as it also includes ChAs. I think that makes sense though.

shaun-technovation commented 2 months ago

I deployed this latest attempt at a fix using our hotfix process (#4831), and the new "Mentors match with a team" filter is working for me.

dboyer commented 2 months ago

VET also confirmed this is now working on production

dboyer commented 2 months ago

Leaving this in the Done column as a reminder to check when the next deployment goes out since this was a hotfix.

shaun-technovation commented 1 month ago

Leaving this in the Done column as a reminder to check when the next deployment goes out since this was a hotfix.

I see the separate student/mentor matched with a team filters, so I think we're good!

dboyer commented 1 month ago

Also seeing the separate filters! We can assume our deployment post hotfix process is working as expected! I'm closing this.