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

Decide on pg notify use and clean up #2156

Closed
matt2e opened this issue Jul 24, 2024 · 3 comments · Fixed by #2574
Closed

Decide on pg notify use and clean up #2156

matt2e opened this issue Jul 24, 2024 · 3 comments · Fixed by #2574
Assignees
@matt2e
Labels
core The core of FTL next Work that will be be picked up next P2

Comments

@matt2e
Copy link
Collaborator

matt2e commented Jul 24, 2024 •
edited
Loading

  • Is pg notify something we want to use?
  • If not, we should remove the db hooks and the unused code in notify.go
@github-actions github-actions bot added the triage Issue needs triaging label Jul 24, 2024
@ftl-robot ftl-robot mentioned this issue Jul 24, 2024
Open
@worstell worstell added the P2 label Jul 24, 2024
@alecthomas alecthomas removed the triage Issue needs triaging label Jul 24, 2024
@matt2e matt2e self-assigned this Jul 24, 2024
@matt2e
Copy link
Collaborator Author

matt2e commented Jul 24, 2024

Originally this issue was about stopping error logs.
I thought this was causing lingering error logs of Failed to receive notification: conn closed, but thats not true. It was just still coming from staging env because we hadn't updated FTL there.
Updating this issue to to be more about cleaning up after we decide on whether to use pg notify longer term or not

@matt2e matt2e changed the title Remove pg notify db hooks Decide on pg notify use and clean up Jul 24, 2024
@matt2e matt2e removed their assignment Jul 24, 2024
@matt2e matt2e added the next Work that will be be picked up next label Jul 24, 2024
@stuartwdouglas
Copy link
Collaborator

What was the decision here? Given the lack of support for this on AWS and the potential need to support AWS it seems like removing it might be the best course of action.

@matt2e
Copy link
Collaborator Author

matt2e commented Aug 15, 2024

I vote remove it as well

This was referenced Aug 27, 2024
Open
Open
Open
Open
Open
Open
Open
@matt2e matt2e self-assigned this Sep 2, 2024
@github-actions github-actions bot removed the next Work that will be be picked up next label Sep 2, 2024
@matt2e matt2e added next Work that will be be picked up next core The core of FTL labels Sep 2, 2024
@matt2e matt2e mentioned this issue Sep 2, 2024
Merged
github-merge-queue bot pushed a commit that referenced this issue Sep 2, 2024
@matt2e
chore: remove usused pg_notify code (#2574)
c5e8e40 
closes #2156

- pubsub was never using the pg_notify events, but the controller was
listening for them anyway
- deployment updates we switched to a polling approach and seems to be
working fine.
- This PR cleans up the code and sql that is no longer being used.

@alecthomas Feel free to reject this PR if you want to keep this
streaming code because you think it's worth bringing it back in the
future.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@matt2e matt2e

Labels
core The core of FTL next Work that will be be picked up next P2
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

chore: remove usused pg_notify code block/ftl
4 participants
@alecthomas @stuartwdouglas @matt2e @worstell