fix: relay document listeners #6563

bjoerge · 2024-05-03T12:17:12Z

Description

This implements a relay approach for listener connections:

When we now set up a listener, we also schedule a new listener connection after somewhere between 12-19 minutes.
We’ll keep the old listener connected for 20s after the new one has connected and has received the welcome-event - (in case there’s in-flight messages yet to arrive).
There’s a distinct filtering on eventId so we avoid processing the same message twice.

These steps are repeated indefinitely (for as long as needed).

What to review

The intervals are currently hard coded, so the best way to test is probably to check out this branch, and edit the values here to something shorter (e.g. 10-20s with 4s overlap) and observe that a new listener is set up before the old one is shut down.

sanity/packages/sanity/src/core/store/_legacy/document/document-pair/checkoutPair.ts

Lines 196 to 202 in 4eedef7

    
           /** How long to wait after the first connection is set up to start the exchange */ 
        
           exchangeWaitMin: 1000 * 60 * 12, 
        
           exchangeWaitMax: 1000 * 60 * 19, 
        
           /** How long should the overlap between the two listeners be */ 
        
           exchangeOverLapTime: 1000 * 20, 
        
           /** If we're unable to set up the next listener within this time, we'll just continue with the current one */ 
        
           exchangeTimeout: 1000 * 20,

It might be a bit hard to unpack the code, but I believe the logic is correct and there's tests to verify that it behaves correctly.

Since this can potentially make it easier to hit the the max number of concurrent connections allowed by browsers on http/1. I didn't include a config flag to disable this right now, but it should be quite straightforward to add a config flag if we wish to do so. For now, there's a timeout on the second listener that will cancel it if it's not able to connect within a certain time. If this happens, the first listener will keep runnning.

Since the server now should allow connections to live for a lot longer – should we wait for longer before exchanging the current one with a new one?

Testing

Added unit tests asserting that:

Listener setup, handoff and teardown works as it should
De-duping listener events based on eventId works
Timeout on "next" leg keeps the current
It waits for the first connection to be set up and have received the welcome event before scheduling the next one

Notes for release

N/A - It should be transparent for the user.

vercel · 2024-05-03T12:17:20Z

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

Name	Status	Preview	Comments	Updated (UTC)
performance-studio	✅ Ready (Inspect)	Visit Preview		May 13, 2024 11:04am
test-next-studio	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 13, 2024 11:04am
test-studio	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 13, 2024 11:04am

1 Ignored Deployment

Name	Status	Preview	Comments	Updated (UTC)
studio-workshop	⬜️ Ignored (Inspect)	Visit Preview		May 13, 2024 11:04am

github-actions · 2024-05-03T12:18:11Z

No changes to documentation

github-actions · 2024-05-03T12:24:37Z

Component Testing Report Updated May 13, 2024 11:09 AM (UTC)

File	Status	Duration	Passed	Skipped
comments/CommentInput.spec.tsx	✅ Passed (Inspect)	33s	15	0
formBuilder/ArrayInput.spec.tsx	✅ Passed (Inspect)	6s	3	0
formBuilder/inputs/PortableText/Annotations.spec.tsx	✅ Passed (Inspect)	26s	6	0
formBuilder/inputs/PortableText/copyPaste/CopyPaste.spec.tsx	✅ Passed (Inspect)	31s	11	7
formBuilder/inputs/PortableText/Decorators.spec.tsx	✅ Passed (Inspect)	14s	6	0
formBuilder/inputs/PortableText/FocusTracking.spec.tsx	✅ Passed (Inspect)	35s	15	0
formBuilder/inputs/PortableText/Input.spec.tsx	✅ Passed (Inspect)	1m 3s	15	0
formBuilder/inputs/PortableText/ObjectBlock.spec.tsx	✅ Passed (Inspect)	1m 2s	18	0
formBuilder/inputs/PortableText/PresenceCursors.spec.tsx	✅ Passed (Inspect)	7s	3	9
formBuilder/inputs/PortableText/RangeDecoration.spec.tsx	✅ Passed (Inspect)	20s	9	0
formBuilder/inputs/PortableText/Styles.spec.tsx	✅ Passed (Inspect)	14s	6	0
formBuilder/inputs/PortableText/Toolbar.spec.tsx	✅ Passed (Inspect)	22s	9	0

* fix(studio): adding tooltip to read-only bool inputs * fix(studio): testing for tooltip on boolean read-only inputs * fix(studio): removing memoisation as it was useless

Co-authored-by: juice49 <1454914+juice49@users.noreply.github.com>

* feat: add canHandleIntent to S.component * fix: properly type canHandleIntent * Update packages/sanity/src/structure/structureBuilder/Component.ts Co-authored-by: Ash <ash@sanity.io> --------- Co-authored-by: Ash <ash@sanity.io>

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* feat(vision): add download as json/csv buttons * fix(vision): use blob urls for downloads (#6213) * fix(vision): use Translate component to avoid splitting i18n strings * fix(vision): clean up i18n resources for result saving feature --------- Co-authored-by: Espen Hovlandsdal <espen@hovlandsdal.com>

…ects

packages/sanity/src/core/store/_legacy/document/createPairListener.ts

… reconnects

juice49

This looks great!

The merge-base changed after approval.

vercel bot deployed to Preview – test-next-studio May 3, 2024 12:17 View deployment

vercel bot deployed to Preview – test-studio May 3, 2024 12:17 View deployment

jordanl17 and others added 8 commits May 8, 2024 10:51

fix(studio): adding tooltip to read-only bool inputs (#6580)

a5cd98c

* fix(studio): adding tooltip to read-only bool inputs * fix(studio): testing for tooltip on boolean read-only inputs * fix(studio): removing memoisation as it was useless

chore(deps): dedupe pnpm-lock.yaml (#6607)

46d3605

Co-authored-by: juice49 <1454914+juice49@users.noreply.github.com>

fix(structure): use case insensitive search for inspect dialog (#6588)

8c4d49d

test(core): await publish action to prevent test flakiness (#6609)

46cf0d3

fix(deps): update dependency get-it to ^8.4.29 (#6603)

b912091

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

fix(deps): update dependency @sanity/client to ^6.18.0 (#6604)

f82f900

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

bjoerge force-pushed the sdx-1311/relay-document-listeners branch from 8098363 to 1813dc4 Compare May 8, 2024 18:27

vercel bot had a problem deploying to Preview – performance-studio May 8, 2024 18:29 Failure

vercel bot had a problem deploying to Preview – test-next-studio May 8, 2024 18:29 Failure

vercel bot had a problem deploying to Preview – test-studio May 8, 2024 18:29 Failure

bjoerge force-pushed the sdx-1311/relay-document-listeners branch 2 times, most recently from 3cdc78a to d466e25 Compare May 8, 2024 18:51

vercel bot deployed to Preview – performance-studio May 8, 2024 18:56 View deployment

vercel bot deployed to Preview – test-studio May 8, 2024 18:58 View deployment

vercel bot deployed to Preview – test-next-studio May 8, 2024 19:00 View deployment

bjoerge added 2 commits May 8, 2024 21:41

chore: remove debug code

6224a45

chore(sanity): rename getPairListener => createPairListener

f2de4b3

bjoerge force-pushed the sdx-1311/relay-document-listeners branch from d466e25 to 32fbe7d Compare May 8, 2024 19:54

bjoerge marked this pull request as ready for review May 8, 2024 19:57

bjoerge requested a review from a team as a code owner May 8, 2024 19:57

bjoerge requested a review from juice49 May 8, 2024 19:57

vercel bot deployed to Preview – performance-studio May 8, 2024 19:57 View deployment

bjoerge force-pushed the sdx-1311/relay-document-listeners branch from 32fbe7d to 9acdb1f Compare May 8, 2024 19:57

fix(core): implement listener connection relay to avoid abrupt reconn…

4eedef7

…ects

bjoerge force-pushed the sdx-1311/relay-document-listeners branch from 9acdb1f to 4eedef7 Compare May 8, 2024 19:59

vercel bot deployed to Preview – performance-studio May 8, 2024 20:03 View deployment

vercel bot deployed to Preview – test-studio May 8, 2024 20:04 View deployment

vercel bot deployed to Preview – test-next-studio May 8, 2024 20:05 View deployment

juice49 reviewed May 13, 2024

View reviewed changes

packages/sanity/src/core/store/_legacy/document/createPairListener.ts Outdated Show resolved Hide resolved

juice49 reviewed May 13, 2024

View reviewed changes

packages/sanity/src/core/store/_legacy/document/createPairListener.ts Outdated Show resolved Hide resolved

bjoerge added 2 commits May 13, 2024 12:29

fixup! fix(core): implement listener connection relay to avoid abrupt…

6eeb47c

… reconnects

fixup! fix(core): implement listener connection relay to avoid abrupt…

a8dc8b1

… reconnects

vercel bot deployed to Preview – performance-studio May 13, 2024 11:03 View deployment

bjoerge requested a review from juice49 May 13, 2024 11:03

vercel bot deployed to Preview – test-studio May 13, 2024 11:03 View deployment

vercel bot deployed to Preview – test-next-studio May 13, 2024 11:04 View deployment

juice49 previously approved these changes May 14, 2024

View reviewed changes

ricokahler force-pushed the next branch from 73c3440 to 4beb29e Compare May 14, 2024 15:52

bjoerge force-pushed the next branch from f88eed7 to a7f9b40 Compare May 15, 2024 15:39

ricokahler force-pushed the next branch from 7ae3afd to ab4a9e8 Compare May 15, 2024 21:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: relay document listeners #6563

fix: relay document listeners #6563

bjoerge commented May 3, 2024 •

edited

vercel bot commented May 3, 2024 •

edited

github-actions bot commented May 3, 2024

github-actions bot commented May 3, 2024 •

edited

juice49 left a comment

	/** How long to wait after the first connection is set up to start the exchange */
	exchangeWaitMin: 1000 * 60 * 12,
	exchangeWaitMax: 1000 * 60 * 19,
	/** How long should the overlap between the two listeners be */
	exchangeOverLapTime: 1000 * 20,
	/** If we're unable to set up the next listener within this time, we'll just continue with the current one */
	exchangeTimeout: 1000 * 20,

fix: relay document listeners #6563

Are you sure you want to change the base?

fix: relay document listeners #6563

Conversation

bjoerge commented May 3, 2024 • edited

Description

What to review

Testing

Notes for release

vercel bot commented May 3, 2024 • edited

github-actions bot commented May 3, 2024

github-actions bot commented May 3, 2024 • edited

juice49 left a comment

Choose a reason for hiding this comment

bjoerge commented May 3, 2024 •

edited

vercel bot commented May 3, 2024 •

edited

github-actions bot commented May 3, 2024 •

edited