Skip to content

Move uploader out of acid-state - #1394

Merged
gbaz merged 6 commits into
haskell:masterfrom
Kleidukos:store-uploaders-in-a-cache
Jan 15, 2026
Merged

Move uploader out of acid-state#1394
gbaz merged 6 commits into
haskell:masterfrom
Kleidukos:store-uploaders-in-a-cache

Conversation

@Kleidukos

@Kleidukos Kleidukos commented May 12, 2025

Copy link
Copy Markdown
Member

Follow-up of #1375 to use a Map rather than storing usernames in acid-state.

Since the datatype used for storage no longer reflects the shape of the API payload, I made a separate DTO for it.

@Kleidukos

Copy link
Copy Markdown
Member Author

@gbaz I'm still unclear on how to initialise the Map and keep it updated. Shall we pass it in a TVar, and update it in the upload endpoint? Again, I'm very unfamiliar with the usual patterns of this project, but I'm willing to learn! :)

@Kleidukos

Kleidukos commented May 22, 2025

Copy link
Copy Markdown
Member Author

@gbaz I'd be grateful if you could help me put the last pieces of the puzzle together, when time permits. :)

@gbaz

gbaz commented May 25, 2025

Copy link
Copy Markdown
Contributor

ok i think i see the confusion. what i mean is why do we need any acid-state for any of this? can't we just compute it on the fly? before i had suggested a better way to cache, but i don't think we need to cache it either. hitting these endpoints should be rare, and all the data is available to query elsewhere. I think we shouldn't need acid-state specific to back this json endpoint at all, much less to migrate it.

@Kleidukos

Copy link
Copy Markdown
Member Author

@gbaz ok then I must be mis-understanding the role of acid-state. In my head it's the one source of truth to fetch persistent data related to the index, including who has uploaded what. If the data isn't coming from acid-state, where does it come from?

@gbaz

gbaz commented Jan 10, 2026

Copy link
Copy Markdown
Contributor

i'm saying don't have a cache. there's no need to cache the data fetch it on demand.

@Kleidukos
Kleidukos force-pushed the store-uploaders-in-a-cache branch 2 times, most recently from dc8abe7 to 924e889 Compare January 15, 2026 16:48
@Kleidukos
Kleidukos marked this pull request as ready for review January 15, 2026 16:48
@Kleidukos
Kleidukos force-pushed the store-uploaders-in-a-cache branch from 924e889 to 31b0e67 Compare January 15, 2026 16:49
@Kleidukos

Copy link
Copy Markdown
Member Author

@gbaz I tried it locally and the results are good. While pairing on this with @edmundnoble we noticed that the package description cache does not seem to be so useful, now that we always resolve pkgId, but maybe I am mistaken regarding the architecture.

@gbaz

gbaz commented Jan 15, 2026

Copy link
Copy Markdown
Contributor

by package description cache you mean the cachedDescr <- Framework.queryState packageInfoState $ GetDescriptionFor (pkgid, metadataRev) stuff? If you can remove usage of acid-state from the package json description altogether imho that would be ideal.

@Kleidukos

Copy link
Copy Markdown
Member Author

@gbaz ok! I will probably do it in another PR after this one is merged then. :-)

@Kleidukos
Kleidukos requested a review from gbaz January 15, 2026 19:37
@gbaz

gbaz commented Jan 15, 2026

Copy link
Copy Markdown
Contributor

Wouldn't that subsume this?

@Kleidukos

Kleidukos commented Jan 15, 2026

Copy link
Copy Markdown
Member Author

@gbaz yes indeed. See my last commits. I removed the cache interface for package descriptions as well, if that' ok with you. If not I can revert that particular commit without problems.

@gbaz

gbaz commented Jan 15, 2026

Copy link
Copy Markdown
Contributor

Thanks for the work. I went ahead and cleared out the state cache entirely to finish the job.

@Kleidukos

Copy link
Copy Markdown
Member Author

@gbaz Cheers. :) I wasn't sure how deep we could clean this up and not get bitten in prod. :)

@gbaz
gbaz merged commit 8ce9679 into haskell:master Jan 15, 2026
12 checks passed
@Kleidukos
Kleidukos deleted the store-uploaders-in-a-cache branch January 15, 2026 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants