8000 New Arch, New ChangeSet by jobelenus · Pull Request #6148 · systeminit/si · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

New Arch, New ChangeSet #6148

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
8000
Merged
merged 1 commit into from
May 19, 2025
Merged

New Arch, New ChangeSet #6148

merged 1 commit into from
May 19, 2025

Conversation

jobelenus
Copy link
Contributor
@jobelenus jobelenus commented May 18, 2025 •
edited
Loading

Calling

  1. Before we make API calls, detect when we are on HEAD and create a new change set
  2. Once that is complete, send the API POST/PUT/DELETE against the new change set
  3. Tell the caller about the new change set
  4. router.push appropriately

Bifrosting

  1. Listen for new change sets on the OG web socket.
  2. Get the HEAD snapshot in sqlite
  3. Insert the snapshot record
  4. create MTM links based on the HEAD snapshot
  5. insert the change set record
  6. ping the /index endpoint to create the index MV in frigg

Behavior Change Needed

The current behavior leaves a lot to be desired:

  • I have two open tabs, both sitting on HEAD.
  • Tab 1, I make the new change set
  • Tab 2, is pristine, it receives the WsEvent, creates the MTM behind the scenes ✅ no errors
  • Tab 1, creates a race condition because...
  • it redirects you to the new change set, before the index is created, before the MTM is done
  • niflheim does happen, waits for index to build
  • all the query keys bust b/c change set changed
  • all queries miss, and throw mjolnir
  • all mjolnirs 404, because the index isn't rebuilt yet
  • index finally rebuilds and cold start finishes
  • all cache keys bust
  • data shows up

Problem: I can't actually put up a spinner for "all the queries that completed, and missed, because in the future they will re-fire". So, users will see lots of blank no spinner, wondering "wait.. did it work?!"... snap the data shows up.

Proposal: if we block the commit of /create_changeset endpoint until the new change set's index is created, then we get no ugliness.. no 404s, no "can't spinner" problem—because I can absolutely show a spinner for "I did an HTTP POST to /create_changeset and it hasn't returned yet". This doesn't "lengthen" the wait time for users—in the current behavior where they get redirected immediately, they can't see anything anyway.

@github-actions github-actions bot added A-sdf Area: Primary backend API service [Rust] A-dal A-web A-luminork labels May 18, 2025
@github-actions GitHub Actions
Copy link
github-actions bot commented May 18, 2025 •
edited
Loading

Dependency Review

✅ No vulnerabilities or OpenSSF Scorecard issues found.

Scanned Files

None

@jobelenus jobelenus force-pushed the jobelenus/new-change-set-mtm branch 4 times, most recently from ab0e717 to 29a5db7 Compare May 19, 2025 14:16
@jobelenus jobelenus marked this pull request as ready for review May 19, 2025 14:17
@jobelenus jobelenus force-pushed the jobelenus/new-change-set-mtm branch 3 times, most recently from 0912f0e to 8436226 Compare May 19, 2025 16:17
@jobelenus jobelenus force-pushed the jobelenus/new-change-set-mtm branch from 8436226 to 744a3ef Compare May 19, 2025 17:19
@jobelenus jobelenus enabled auto-merge May 19, 2025 18:56
@jobelenus jobelenus added this pull request to the merge queue May 19, 2025
Merged via the queue into main with commit 1d24806 May 19, 2025
12 checks passed
@jobelenus jobelenus deleted the jobelenus/new-change-set-mtm branch May 19, 2025 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@britmyerss britmyerss britmyerss approved these changes

@wendybujalski wendybujalski Awaiting requested review from wendybujalski

Assignees
No one assigned
Labels
A-dal A-luminork A-sdf Area: Primary backend API service [Rust] A-web
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jobelenus @britmyerss
0