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

Bug: users receiving two emails for each digest notification #2133

Open
ClaireValdivia opened this issue Oct 20, 2023 · 12 comments
Open

Bug: users receiving two emails for each digest notification #2133

ClaireValdivia opened this issue Oct 20, 2023 · 12 comments
Assignees
Labels
Grant Finder Issues related to the Grant Finder partner submission

Comments

@ClaireValdivia
Copy link
Contributor

ClaireValdivia commented Oct 20, 2023

Why is this issue important?

emails are duplicates and confusing

Current State

A State partner reported receiving duplicate email digest notifications for her saved search Transportation (email is recieved at same time with same exact content). She noted that others within her org have reported the same. Have examples of duplicate emails if needed (removed from ticket for security reasons but can add with identifying info redacted if needed)

Expected State

Users should receive one email per day per saved search if new grants enter the saved search

Implementation Plan

The following functions need to be added...

Relevant Code Snippets

No response

@ed-snodgrass
Copy link
Contributor

Just noting that there is an integration test around this behavior that is currently passing. I also set up my dev environment to send an email to myself for a particular search and I received only one email. I think the next step may be to check the production database to see if a duplicate Saved Search somehow got entered.

I verified that you cannot add a Saved Search with the same name or edit a Saved Search to have the same name. Even attempting to modify the data within the database triggers the duplicate key value violates unique constraint "grants_saved_searches_name_created_by_idx" response.

That said, I was able to add a space to the end and save that and both emails looked exactly the same (I couldn't see the space at the end of the title in the subject of the email).

Next steps might be to check the logs to see if there are multiple crons getting kicked off and to check the database to see if there is something related to the data.

@ClaireValdivia
Copy link
Contributor Author

@replicantSocks forgot to mention this one in the meeting yesterday, but would you be able to take a look at this one? higher priority than the end date ticket.

@replicantSocks
Copy link
Contributor

I had a similar intuition to Ed in terms of checking logs. I looked through yesterday's (10/31) logs at around 9 am and saw that there were basically 2 of every log entry related to saved search and email digests. Each log entry seems to occur within a few 10s of ms of each other. This could suggest maybe the cron is executing twice at the same time. The log entries don't have a process ID, but python doesn't generally execute in parallel.

Here's a link to the logs on the prod host:
https://app.datadoghq.com/logs?query=host%3Agost-prod-api-20230303023921715300000008&cols=host%2Cservice&index=%2A&messageDisplay=inline&refresh_mode=paused&stream_sort=desc&viz=stream&from_ts=1698756900000&to_ts=1698757800000&live=false

Earliest 2 lines are:

Oct 31 09:00:00.048
gost-prod-api-20230303023921715300000008
cloudwatch
Building and sending Grants Digest email for user: undefined on 2023-10-30

Oct 31 09:00:00.024
gost-prod-api-20230303023921715300000008
cloudwatch
Building and sending Grants Digest email for user: undefined on 2023-10-30

Those map to the buildAndSendUserSavedSearchGrantDigest function in email.js, which is called directly by the CronJob. So, my first guess was that the cron got installed twice somehow. I will attempt to validate that

@replicantSocks
Copy link
Contributor

Just realized the cron is node based on never installed on the OS. Also, we look to have 2 containers running the app in prod. Not immediately sure how those coordinate between each other. Will check in with Aditya or Ty to validate my assumptions in case I'm misreading/misunderstanding something.

@TylerHendrickson
Copy link
Member

I think @ed-snodgrass and @replicantSocks are on to something here – still confirming, but I don't immediately see any mechanism that would prevent email crons from triggering simultaneously on separate Fargate tasks.

If that is in fact the case:

  1. This issue would presumably be resolved by feat: adds infra required to support digest emails #2023, although we may want to look into a nearer-term fix depending on impact.
  2. My assumption is that pretty much every digest email would be sent (at least) twice – we always (barring an outage) have at least 2 Fargate tasks (i.e. server-side worker processes) running in Production, so without anything preventing multiple simultaneous cron runs (like a lock that can only be acquired by one cron process at a time, along with something that short-circuits a cron task if a digest job has already run) then each Fargate task is likely running through the same email job more or less concurrently, independently of any other in-progress email job.

@as1729 anything to add here?

@as1729
Copy link
Contributor

as1729 commented Nov 1, 2023

Yes this issue exists because of two fargate tasks running simultaneously. Moving the implementation to a queue system as described in this PR will fix the issue. #2023

@TylerHendrickson
Copy link
Member

@ClaireValdivia Can you opine on priority here? As noted above, #2023 should resolve this issue but I'm not clear on the current urgency of that ticket (at least relative to other on-deck work).

If this is in fact more urgent than previously thought, it might be worth seeing who might be interested in collaborating on #2023, or else perhaps there's a tidy-ish short-term option that someone on this thread can recommend?

@ClaireValdivia
Copy link
Contributor Author

hmm I think #2023 was not on my radar as we don't have an issue open for it. based on what we decide here, I'll either create a new linked issue or link this ticket to #2023.

@as1729 or @TylerHendrickson curious for rough thoughts on size/complexity of the work outlined in #2023

I think the double emails issues is likely to increase in urgency over the next couple months as we have a few new partners who are planning to roll the tool out to more users in the coming months (NJ, HI, and PR). I think this is lower priority than CPF but higher priority than other items in the Ready status right now.

@dysmento dysmento moved this from 🔖 Ready to 🏗 In progress in Grants Team Agile Planning Nov 10, 2023
@dysmento dysmento removed their assignment Nov 29, 2023
@dysmento
Copy link
Contributor

I've incorporated all the feedback I'm aware of on this PR: #2023

@ClaireValdivia
Copy link
Contributor Author

This one has been sitting for a little...@as1729 when you are able, could you take a look at the PR and let me know your thoughts on what is completed/outstanding here, and what the next steps are?

@as1729 as1729 removed their assignment Mar 20, 2024
@as1729
Copy link
Contributor

as1729 commented Mar 20, 2024

@ClaireValdivia I'm going to un-assign myself from this issue. If there are other volunteers who can continue where #2023 is left off that would be ideal.

@as1729 as1729 moved this from 🏗 In progress to 🔖 Ready in Grants Team Agile Planning Mar 20, 2024
@jeffsmohan jeffsmohan moved this from 🔖 Ready to 🏗 In progress in Grants Team Agile Planning Mar 26, 2024
@jeffsmohan jeffsmohan moved this from 🏗 In progress to 🔖 Ready in Grants Team Agile Planning Mar 26, 2024
@jeffsmohan jeffsmohan moved this from 🔖 Ready to 🏗 In progress in Grants Team Agile Planning Apr 2, 2024
@jeffsmohan jeffsmohan moved this from 🏗 In progress to 👀 In review in Grants Team Agile Planning Apr 3, 2024
@ClaireValdivia ClaireValdivia moved this from 👀 In review to ✅ Staging in Grants Team Agile Planning Apr 9, 2024
@ClaireValdivia
Copy link
Contributor Author

Closing out this issue as resolved and opened #2932 for longer term work to rearchitect email infrastructure

@ClaireValdivia ClaireValdivia moved this from ✅ Staging to 🚢 Completed in Grants Team Agile Planning Apr 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Grant Finder Issues related to the Grant Finder partner submission
Projects
Status: 🚢 Completed
Development

No branches or pull requests

7 participants