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

Fix for upserts, question pages (WIP), history #71

Merged
merged 6 commits into from
Apr 27, 2022
Merged

Conversation

berekuk
Copy link
Collaborator

@berekuk berekuk commented Apr 27, 2022

So this is a mix of several changes, some are unfinished:

  • question pages (still unfinished)
  • history api in graphql (mostly working)
  • fix for upserts which I lazily implemented in the same branch (maybe I should've built it on top of master instead, but I disabled the links to question pages for now, so it should be ok)

Fix for upserts - kinda urgent

I was working on adding foreign keys from history to questions table when I discovered that fetchers were broken in prod due to the unfortunate combination of: (1) frontpage table is now implemented with foreign keys to questions table now; (2) fetched questions are inserted with delete+recreate trick.

But foreign keys forbid the questions from being deleted, so deletes failed, and cron job skipped the platform entirely if it had any questions on the frontpage. At least that's what I assume have happened, I haven't checked the full logs.

defaultdb=> select date(timestamp) as d, count(1) from questions group by d order by d desc limit 5;
     d      | count
------------+-------
 2022-04-27 |  1805
 2022-04-25 |   191
 2022-04-23 |   110
 2022-04-22 |  2189
 2022-03-30 |   168
(5 rows)

What's worse is that there's no easy to fix this while keeping the same performance. The only fix I could come up with quickly negates the benefits of #30: in this PR I just delete the questions which don't exist any more, create the new ones and update the ones which existed. But there's no bulk update API in Prisma (or in SQL, actually), so all updates are done in separate SQL queries. Or, to be more precise, in 5 queries per question:

prisma:query BEGIN
prisma:query SELECT "public"."questions"."id" FROM "public"."questions" WHERE "public"."questions"."id" = $1
prisma:query UPDATE "public"."questions" SET "extra" = $1, "timestamp" = $2, "title" = $3, "url" = $4, "platform" = $5, "description" = $6, "options" = $7, "qualityindicators" = $8, "id" = $9 WHERE "public"."questions"."id" IN ($10)
prisma:query SELECT "public"."questions"."id", "public"."questions"."title", "public"."questions"."url", "public"."questions"."platform", "public"."questions"."description", "public"."questions"."options", "public"."questions"."timestamp", "public"."questions"."qualityindicators", "public"."questions"."extra" FROM "public"."questions" WHERE "public"."questions"."id" = $1 LIMIT $2 OFFSET $3
prisma:query COMMIT

It shouldn't be that bad, though, since our DB is now geographically close to Heroku's instance. And this can be optimized further:

  • by doing deep comparison and updating only the questions which have actually updated
  • or we could insert questions into a temporary table and then do UPDATE ... FROM ... (but this is impossible with Prisma, so we'll have to drop back to the raw SQL...)

On other stuff in this PR:

  • question pages are on /questions/[id]; there's an icon linking to them in DisplayQuestion, but I disabled it for now
  • also, only the question title links to the original url now (is this ok?)
  • history API is mostly working, but I haven't used it on the frontend yet

@vercel
Copy link

vercel bot commented Apr 27, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
metaforecast ✅ Ready (Inspect) Visit Preview Apr 27, 2022 at 6:29PM (UTC)

@berekuk
Copy link
Collaborator Author

berekuk commented Apr 27, 2022

Ok, I'll just merge it.

@NunoSempere, let me know if you're not ok with this:

only the question title links to the original url now

(instead of an entire question card)

@berekuk berekuk merged commit 4d39186 into master Apr 27, 2022
@NunoSempere
Copy link
Collaborator

Hey, this looks good to me

@berekuk berekuk mentioned this pull request Apr 27, 2022
@berekuk berekuk deleted the question-pages branch May 12, 2022 22:32
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