Increase live data poll rate to 2 seconds in OSS only #17000
Conversation
| <CommunityNux /> | ||
| </App> | ||
| </AppProvider> | ||
| <LiveDataPollRateContext.Provider value={2000}> |
There was a problem hiding this comment.
I kept this outside of AppProvider intentionally because we only want to do this in OSS and not Cloud
|
Deploy preview for dagit-core-storybook ready! ✅ Preview Built with commit 5c4dda6. |
|
How difficult would this be to make command line option to |
|
@schrockn Not very difficult |
|
Yeah let's do that then.
…On Wed, Oct 04, 2023 at 11:58 AM, Marco polo < ***@***.*** > wrote:
@ schrockn ( https://github.com/schrockn ) Not very difficult
—
Reply to this email directly, view it on GitHub (
#17000 (comment) )
, or unsubscribe (
https://github.com/notifications/unsubscribe-auth/AG3IK6N2SLPAQ6P3UUCWVZ3X5WBRHAVCNFSM6AAAAAA5STVZISVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONBXGIYDANZRHE
).
You are receiving this because you were mentioned. Message ID: <dagster-io/dagster/pull/17000/c1747200719
@ github. com>
|
|
makes sense to me in cloud is the idea that deployments happen much less frequently so there isn't a need to poll as frequently? |
|
This isn't about deployments. This is about seeing live updates about the state of assets. E.g, "Materialized at XXX" time. |
|
cool - so why is it 2 secs in OSS and (afaict right now) 2 minutes in cloud? |
I believe it is 30 seconds in Cloud and deployed OSS instances. That is a fairly arbitrary limit. We don't do 2 seconds for production deployments because it could/would get prohibitively expense (both in terms of dollars and in ensuring reasonable database load). The theory is that when you are doing dagster dev you are far more likely to be working in cases where you are only kicking off ad hoc runs and the local dev deployment has much less state. |
There was a problem hiding this comment.
The code for this looks good to me, but I'm curious why this change feels necessary. If the run log subscriptions are working, the refresh rate should automatically increase (up to SUBSCRIPTION_MAX_POLL_RATE = 2s) achieving this 2s refresh rate when events are coming in for on-screen assets. If this change makes a significant difference, it feels like something in there is probably not working? (If the issue is the app not picking up new runs fast enough, maybe we throw a comment in so we know we can undo this if/when we add a run launch subscription.)
If we're just going to refresh every 2s all the time we should remove the run log subscriptions in OSS entirely -- streaming run log events to every asset page is expensive and if MAX_POLL_RATE and IDLE_POLL_RATE are the same it doesn't do anything.
|
@bengotow Reason why is that for demos and local dev if you are, for example, kicking off runs with a sensor, the UI isn't updated for 30 seconds and it looks like something is going wrong. This will make it all feel much more realtime and less buggy. |
|
Ahhh yeah the |
|
Would still vote to disable the run log subscriptions in this PR since they now have no impact on the refresh rate. |
They would still have an impact in cloud. Also we're going to make it configurable via the CLI so it should still have an impact if they choose a setting greater than 2 seconds |
|
@alangenfeld or @gibsondan could I get a review on the python bits adding the CLI argument. Could use feedback on the the argument name / help text |
Summary & Motivation
Due to our recent changes to how we fetch asset data we can no longer provide a query refresh countdown. This makes the app feel less alive so for OSS only we want to increase the poll rate for data to every 2 seconds. In practice this means assets could be fetched at best every 2 seconds but it could take longer due to the chunking/queueing behavior of AssetLiveDataProvider (which is good).
@schrockn Asked for this specifically.
How I Tested These Changes
Locally I checked the network tab and made sure we were polling every 2 seconds