-
Notifications
You must be signed in to change notification settings - Fork 107
fix(zero-cache): deleteClients from stale WebSocket crashes view-syncer
#5509
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
| Branch | 0xcadams/delete-clients |
| Testbed | Linux |
Click to view all benchmark results
| Benchmark | File Size | Benchmark Result kilobytes (KB) (Result Δ%) | Upper Boundary kilobytes (KB) (Limit %) |
|---|---|---|---|
| zero-package.tgz | 📈 view plot 🚷 view threshold | 1,830.71 KB(+0.00%)Baseline: 1,830.70 KB | 1,867.31 KB (98.04%) |
| zero.js | 📈 view plot 🚷 view threshold | 244.11 KB(0.00%)Baseline: 244.11 KB | 248.99 KB (98.04%) |
| zero.js.br | 📈 view plot 🚷 view threshold | 66.92 KB(0.00%)Baseline: 66.92 KB | 68.26 KB (98.04%) |
| } | ||
| } | ||
| return result as R; | ||
| return result; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This typecast was hiding the bug. Removed and types now reflect actual response types.
3c1aed2 to
b70f7a5
Compare
|
| Branch | 0xcadams/delete-clients |
| Testbed | self-hosted |
Click to view all benchmark results
| Benchmark | Throughput | Benchmark Result operations / second (ops/s) (Result Δ%) | Lower Boundary operations / second (ops/s) (Limit %) |
|---|---|---|---|
| 1 exists: track.exists(album) | 📈 view plot 🚷 view threshold | 13,975.83 ops/s(+0.23%)Baseline: 13,944.45 ops/s | 11,298.86 ops/s (80.85%) |
| 10 exists (AND) | 📈 view plot 🚷 view threshold | 202,010.56 ops/s(-1.23%)Baseline: 204,526.14 ops/s | 162,821.87 ops/s (80.60%) |
| 10 exists (OR) | 📈 view plot 🚷 view threshold | 4,021.86 ops/s(+0.14%)Baseline: 4,016.08 ops/s | 3,294.18 ops/s (81.91%) |
| 12 exists (AND) | 📈 view plot 🚷 view threshold | 183,112.42 ops/s(+1.82%)Baseline: 179,833.87 ops/s | 142,204.12 ops/s (77.66%) |
| 12 exists (OR) | 📈 view plot 🚷 view threshold | 3,390.31 ops/s(-0.44%)Baseline: 3,405.44 ops/s | 2,795.41 ops/s (82.45%) |
| 12 level nesting | 📈 view plot 🚷 view threshold | 2,883.84 ops/s(-2.48%)Baseline: 2,957.06 ops/s | 2,373.52 ops/s (82.30%) |
| 2 exists (AND): track.exists(album).exists(genre) | 📈 view plot 🚷 view threshold | 5,211.02 ops/s(-0.64%)Baseline: 5,244.49 ops/s | 4,277.13 ops/s (82.08%) |
| 3 exists (AND) | 📈 view plot 🚷 view threshold | 2,054.07 ops/s(+0.30%)Baseline: 2,047.86 ops/s | 1,678.40 ops/s (81.71%) |
| 3 exists (OR) | 📈 view plot 🚷 view threshold | 1,020.56 ops/s(-0.28%)Baseline: 1,023.41 ops/s | 830.11 ops/s (81.34%) |
| 5 exists (AND) | 📈 view plot 🚷 view threshold | 323.57 ops/s(+0.46%)Baseline: 322.10 ops/s | 260.90 ops/s (80.63%) |
| 5 exists (OR) | 📈 view plot 🚷 view threshold | 169.07 ops/s(-0.18%)Baseline: 169.37 ops/s | 135.92 ops/s (80.39%) |
| Nested 2 levels: track > album > artist | 📈 view plot 🚷 view threshold | 4,589.12 ops/s(+0.64%)Baseline: 4,560.03 ops/s | 3,663.38 ops/s (79.83%) |
| Nested 4 levels: playlist > tracks > album > artist | 📈 view plot 🚷 view threshold | 746.55 ops/s(-0.10%)Baseline: 747.27 ops/s | 605.82 ops/s (81.15%) |
| Nested with filters: track > album > artist (filtered) | 📈 view plot 🚷 view threshold | 3,792.54 ops/s(-0.54%)Baseline: 3,813.06 ops/s | 3,144.13 ops/s (82.90%) |
| planned: playlist.exists(tracks) | 📈 view plot 🚷 view threshold | 577.42 ops/s(-7.20%)Baseline: 622.21 ops/s | 519.51 ops/s (89.97%) |
| planned: track.exists(album) OR exists(genre) | 📈 view plot 🚷 view threshold | 154.20 ops/s(-6.48%)Baseline: 164.89 ops/s | 143.20 ops/s (92.86%) |
| planned: track.exists(album) where title="Big Ones" | 📈 view plot 🚷 view threshold | 7,122.27 ops/s(-6.05%)Baseline: 7,581.32 ops/s | 6,506.61 ops/s (91.36%) |
| planned: track.exists(album).exists(genre) | 📈 view plot 🚷 view threshold | 35.34 ops/s(-10.58%)Baseline: 39.52 ops/s | 33.22 ops/s (94.01%) |
| planned: track.exists(album).exists(genre) with filters | 📈 view plot 🚷 view threshold | 4,897.26 ops/s(-8.23%)Baseline: 5,336.50 ops/s | 4,473.82 ops/s (91.35%) |
| planned: track.exists(playlists) | 📈 view plot 🚷 view threshold | 3.64 ops/s(-9.15%)Baseline: 4.01 ops/s | 3.34 ops/s (91.64%) |
| unplanned: playlist.exists(tracks) | 📈 view plot 🚷 view threshold | 559.10 ops/s(-7.73%)Baseline: 605.95 ops/s | 507.51 ops/s (90.77%) |
| unplanned: track.exists(album) OR exists(genre) | 📈 view plot 🚷 view threshold | 39.33 ops/s(-12.41%)Baseline: 44.90 ops/s | 36.94 ops/s (93.93%) |
| unplanned: track.exists(album) where title="Big Ones" | 📈 view plot 🚷 view threshold | 51.64 ops/s(-8.42%)Baseline: 56.39 ops/s | 47.36 ops/s (91.72%) |
| unplanned: track.exists(album).exists(genre) | 📈 view plot 🚷 view threshold | 36.02 ops/s(-8.20%)Baseline: 39.24 ops/s | 32.83 ops/s (91.14%) |
| unplanned: track.exists(album).exists(genre) with filters | 📈 view plot 🚷 view threshold | 49.74 ops/s(-9.86%)Baseline: 55.18 ops/s | 46.39 ops/s (93.27%) |
| unplanned: track.exists(playlists) | 📈 view plot 🚷 view threshold | 3.63 ops/s(-9.27%)Baseline: 4.00 ops/s | 3.36 ops/s (92.58%) |
| zpg: all playlists | 📈 view plot 🚷 view threshold | 5.33 ops/s(-4.96%)Baseline: 5.61 ops/s | 4.94 ops/s (92.70%) |
| zql: all playlists | 📈 view plot 🚷 view threshold | 7.72 ops/s(+0.87%)Baseline: 7.65 ops/s | 6.02 ops/s (78.01%) |
| zql: edit for limited query, inside the bound | 📈 view plot 🚷 view threshold | 209,608.25 ops/s(-1.45%)Baseline: 212,682.04 ops/s | 176,275.66 ops/s (84.10%) |
| zql: edit for limited query, outside the bound | 📈 view plot 🚷 view threshold | 213,529.36 ops/s(-1.05%)Baseline: 215,798.98 ops/s | 168,593.45 ops/s (78.96%) |
| zql: push into limited query, inside the bound | 📈 view plot 🚷 view threshold | 105,178.67 ops/s(-2.74%)Baseline: 108,136.56 ops/s | 89,885.99 ops/s (85.46%) |
| zql: push into limited query, outside the bound | 📈 view plot 🚷 view threshold | 373,226.02 ops/s(-6.63%)Baseline: 399,744.02 ops/s | 304,223.11 ops/s (81.51%) |
| zql: push into unlimited query | 📈 view plot 🚷 view threshold | 314,771.70 ops/s(-3.41%)Baseline: 325,878.49 ops/s | 254,611.75 ops/s (80.89%) |
| zqlite: all playlists | 📈 view plot 🚷 view threshold | 1.67 ops/s(-5.32%)Baseline: 1.76 ops/s | 1.41 ops/s (84.35%) |
| zqlite: edit for limited query, inside the bound | 📈 view plot 🚷 view threshold | 75,363.72 ops/s(-1.02%)Baseline: 76,141.86 ops/s | 62,039.59 ops/s (82.32%) |
| zqlite: edit for limited query, outside the bound | 📈 view plot 🚷 view threshold | 69,088.92 ops/s(-8.54%)Baseline: 75,543.01 ops/s | 56,317.34 ops/s (81.51%) |
| zqlite: push into limited query, inside the bound | 📈 view plot 🚷 view threshold | 3,865.82 ops/s(-4.35%)Baseline: 4,041.66 ops/s | 3,633.75 ops/s (94.00%) |
| zqlite: push into limited query, outside the bound | 📈 view plot 🚷 view threshold | 85,729.45 ops/s(-3.13%)Baseline: 88,496.56 ops/s | 74,812.93 ops/s (87.27%) |
| zqlite: push into unlimited query | 📈 view plot 🚷 view threshold | 126,565.41 ops/s(+2.01%)Baseline: 124,067.69 ops/s | 98,088.45 ops/s (77.50%) |
|
| Branch | 0xcadams/delete-clients |
| Testbed | self-hosted |
Click to view all benchmark results
| Benchmark | Throughput | Benchmark Result operations / second (ops/s) x 1e3 (Result Δ%) | Lower Boundary operations / second (ops/s) x 1e3 (Limit %) |
|---|---|---|---|
| src/client/custom.bench.ts > big schema | 📈 view plot 🚷 view threshold | 137.76 ops/s x 1e3(-0.58%)Baseline: 138.56 ops/s x 1e3 | 117.78 ops/s x 1e3 (85.49%) |
| src/client/zero.bench.ts > basics > All 1000 rows x 10 columns (numbers) | 📈 view plot 🚷 view threshold | 2.40 ops/s x 1e3(-2.82%)Baseline: 2.47 ops/s x 1e3 | 2.10 ops/s x 1e3 (87.20%) |
| src/client/zero.bench.ts > pk compare > pk = N | 📈 view plot 🚷 view threshold | 60.53 ops/s x 1e3(-5.54%)Baseline: 64.08 ops/s x 1e3 | 52.23 ops/s x 1e3 (86.29%) |
| src/client/zero.bench.ts > with filter > Lower rows 500 x 10 columns (numbers) | 📈 view plot 🚷 view threshold | 3.60 ops/s x 1e3(-4.97%)Baseline: 3.79 ops/s x 1e3 | 3.27 ops/s x 1e3 (90.71%) |
darkgnotic
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very impressive investigative skillz!!!
deleteClientsmessages sent from a stale WebSocket can crash view-syncer.Steps:
deleteClientsarrives from wsID A.#runInLockForClientearly‑returns on wsID mismatch, leaving result unset.#runInLockForClientcast undefined to array even when it could be undefined and caller reads.length.Test Failure:
Related to
Cannot read properties of undefined (reading 'length')error in zbugs: https://rocicorp.slack.com/archives/C07RLKFNSF6/p1769897748417939