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(query-on-demand): don't resolve get-page-data if query is marked as dirty #28535

Conversation

pieh
Copy link
Contributor

@pieh pieh commented Dec 8, 2020

Description

We currently have potential dead lock situation as we skip writing page-data (in query on demand only!) when query is marked as dirty. This means that getPageData can not resolve (or rather resolve with stale data due to timeout guards) if:

  • page query did run but didn't flush yet and query was marked as dirty at some point after running query and before flushing

This PR only skips flushing page-data if query didn't run yet ( FLAG_DIRTY_NEW_PAGE check instead of checking if query is dirty for whatever reason) and triggers another run of doGetPageData that checks if query is dirty / running / pending to trigger another run if it's in fact still dirty after flushing page-data.

I'm uncertain about not touching part that does emit websocket message - in this case we might not resolve getPageData after flushing but still emit websocket update. This should not cause weirdness with loadPage that fetches data, but because we have single cache that is being saturated from fetch requests and websocket updates it could potentially result in weirdness but to repro it it would have to be real weird timing scenario.

[ch19354]

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Dec 8, 2020
@pieh pieh force-pushed the qod/handle-potential-dead-lock-between-get-page-data-and-flush-page-data branch from 243ad52 to 86fc037 Compare December 8, 2020 15:28
@pieh pieh removed the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Dec 8, 2020
@pieh pieh changed the title fix(gatsby): don't resolve get-page-data if query is marked as dirty fix(query-on-demand): don't resolve get-page-data if query is marked as dirty Dec 8, 2020
vladar
vladar previously approved these changes Dec 8, 2020
Copy link
Contributor

@vladar vladar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great stuff! 👍 I'll leave swapping flag values to your preference.

@pieh pieh merged commit 56c2b4c into master Dec 9, 2020
@pieh pieh deleted the qod/handle-potential-dead-lock-between-get-page-data-and-flush-page-data branch December 9, 2020 13:06
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