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

Vapor Storage Threading #206

Closed
cohoe opened this issue Mar 19, 2023 · 4 comments
Closed

Vapor Storage Threading #206

cohoe opened this issue Mar 19, 2023 · 4 comments
Labels
2023 Item that came up during 2023 sailing. No implicit priority. bug Something isn't working high priority Something that really should be implemented NowTM
Milestone

Comments

@cohoe
Copy link
Member

cohoe commented Mar 19, 2023

We got burned by vapor/vapor#2330 again. While we fixed the websocket storage in #53, we didn't consider that under the hood Vapor also uses this for Redis/Postgres connection storage (among other things). Chall was able to hack together a custom version of the Vapor framework to get around this problem on boat. There is an open PR on Vapor that would address this: vapor/vapor#2873

When we start getting closer to 2024 we should either:

  • Fork our own Vapor for real and make it part of our build process.
  • Contribute a fix upstream for this specific issue.
  • Rewrite everything.
@cohoe cohoe added bug Something isn't working high priority Something that really should be implemented NowTM 2023 Item that came up during 2023 sailing. No implicit priority. labels Mar 19, 2023
@hendricksond
Copy link
Member

vapor/vapor#2873 was closed in favor of vapor/vapor#3000. Monitor the new PR.

@hendricksond hendricksond added this to the 2024 Sailing milestone Jul 25, 2023
@hendricksond hendricksond removed the 2024 label Jul 25, 2023
@cohoe
Copy link
Member Author

cohoe commented Jan 7, 2024

vapor/vapor#3000 was closed in favor of other PRs, such as vapor/vapor#3082 and vapor/vapor#3093. The latter of which indicates that the move to Sendable is complete. We'd need to update to Vapor >= 4.86.0.

@cohoe
Copy link
Member Author

cohoe commented Jan 7, 2024

Package.resolved already has Vapor up to 4.89.0 from when Chall last did an update not that long ago. Based on https://forums.swift.org/t/concurrency-checking-in-swift-packages-unsafeflags/61135/3 we can add the following block to the Package.swift if we want to get compilation warnings about potential concurrency issues:

swiftSettings: [
	// This one is the most strict
	// .unsafeFlags(["-Xfrontend", "-strict-concurrency=complete"])
	.unsafeFlags(["-Xfrontend", "-warn-concurrency"])
]

Which indeed spits out a bunch but mostly seems to indicate that we're supposed to mark our API handler functions with @Sendable (for example: @Sendable func openHandler(_ req: Request) async throws -> FezListData {...}.

I don't know if marking all of those is necessary or if it just generates a warning until you do.

cohoe added a commit to cohoe/swiftarr that referenced this issue Jan 7, 2024
cohoe added a commit that referenced this issue Jan 18, 2024
* resolve #189, you spin me right round

* resolves #250, partially addresses #135

* resolves #214, adds pagination to moderator log

* resolves #177, allows admin and tho in site ui with certain disable conditions

* resolves #135, adds about processing via api

* resolves #239, fixes pagination for joined fez and mod log

* resolves #251, brings back new und alertwords and makes UI less surprising

* The guts for #136

* more seamail muting

* resolves #136, finalized mute of seamail I think

* resolves #182, adds support for accessing mentions of twitarrteam and moderator

* #206 bumps up minimum Vapor version, adds notes for concurrency checking

* resolves #216, fixes seamail unread for special users (I think)

* add dummy schedule generator

* resolves #240, makes all lfgs not be perma-unread for moderators

* resolves #173, implements late day flip setting

* resolves #211, adds release calendar documentation

* #132 make the site ui identify itself

* comment

* resolves #183, enables one context button for lfg and seamail

* resolves #179, make open chat the default state

* resolves #176, adds links to profiles for seamail participants

* resolves #217, adds unread forum handlers

* resolves #168, site ui marks new forum read for creator

* resolves #199, correct forum counts after calendar update

* resolves #188, adds team field to user profile

* resolves #180, improves page titles for most common things

* resolves #181, improves user mention and hashtag detection

* resolves #210, moves preferredPronoun to UserHeader for wide consumability

* add username to profile page titles

* continues #231 add timeZoneID to FezData and ForumListdata to match EventData

add timezoneID to forumlistdata like fezdata and eventdata

* improve pronoun display by limiting to useful contexts

* improve pronoun display by limiting to useful contexts

* improve pronoun display with no displayname

* resolves #167, adds view forum posts by user to mods

* dedicated jobs for event schedule update

* address occasional redis connection timeouts
@cohoe
Copy link
Member Author

cohoe commented Mar 5, 2024

We haven't seen this yet. Closing now, so that I can re-open with much sadness if it comes up day one again.

@cohoe cohoe closed this as completed Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2023 Item that came up during 2023 sailing. No implicit priority. bug Something isn't working high priority Something that really should be implemented NowTM
Projects
None yet
Development

No branches or pull requests

2 participants