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

enable termination of 'outdated' long callbacks when page changes #2588

Open
JamesKunstle opened this issue Jul 6, 2023 · 16 comments
Open
Assignees
Labels
feature something new P3 backlog

Comments

@JamesKunstle
Copy link

Is your feature request related to a problem? Please describe.
Long callbacks run for all Figures on all of my application's pages. When I switch between pages quickly the tasks in the backend Celery queue execute in-order regardless of old requests being outdated.

Ex: If I'm on page 1, and I navigate to page 2, then quickly to page 3 then page 4, the long callbacks for figures corresponding to pages 2, then 3, then 4 will run in order, slowing the delivery of Figures pertinent to page 4.

However, this isn't the case if I refresh a single page multiple times. If I'm on page 2 and I refresh the page 5 times, the long callbacks for the Figures associated with page 2 don't execute 5 times in order- the first 4 'render requests' are 'revoked' in the Celery queue, with the final request completing.

Describe the solution you'd like
I'd like a page switch to trigger the termination of newly 'outdated' long callbacks in the backend.

Describe alternatives you've considered
In the universe of Dash objects, I considered implementing a 'Sentry' that watches the URL and triggers a 'cancel' function that's bound to all long callbacks, cancelling them via the 'cancelable' interface of callbacks. In the Dash execution graph, however, I don't think I can guarantee that this callback would always precede the scheduling of new, up-to-date callbacks, so I don't think this is a reasonable solution.

Additional context
Here's an idea for a patch:

The dash_renderer frontend is aware of when it's going to try to execute a callback for a job that is still running. If a to-be-called callback's output list matches the output list of the job that the frontend is waiting for, it issues an 'oldJob' ID to the backend via the request headers. Source

In the backend, receiving these 'oldJob' ID's triggers that job's termination in the Celery backend. Source

It's evident that the frontend is doing job bookkeeping, tracking the 'output' list of jobs that have been scheduled in the backend but that haven't returned for the frontend. If the frontend could also track the page that the job is intended for, and compare that to the window.location.pathname when the job cleanup is already happening, the 'oldJob' param could be set and the backend could clean up any currently running long callbacks for the previous page.

A parameter could be set in the Dash python object to enable and disable this feature- it'd generally NOT be mutually exclusive of memoization because cancellations wouldn't always happen but it'd be more conservative, and memoization wouldn't always have an opportunity to happen.

@cdolfi
Copy link

cdolfi commented Jul 10, 2023

Very interested in this patch as well

@BSd3v
Copy link
Contributor

BSd3v commented Jul 10, 2023

@BSd3v
Copy link
Contributor

BSd3v commented Jul 10, 2023

When trying to use this:

import time
import os

import dash
from dash import Dash, DiskcacheManager, CeleryManager, Input, Output, html, callback, dcc

if 'REDIS_URL' in os.environ:
    # Use Redis & Celery if REDIS_URL set as an env variable
    from celery import Celery
    celery_app = Celery(__name__, broker=os.environ['REDIS_URL'], backend=os.environ['REDIS_URL'])
    background_callback_manager = CeleryManager(celery_app)

else:
    # Diskcache for non-production apps when developing locally
    import diskcache
    cache = diskcache.Cache("./cache")
    background_callback_manager = DiskcacheManager(cache)

app = Dash(__name__, background_callback_manager=background_callback_manager, use_pages=True, pages_folder='')

app.layout = html.Div(
    [
        dcc.Link('page1', '/'), dcc.Link('page2', '/2'),
        dash.page_container
    ]
)

layout1 = [html.Div([html.P(id="paragraph_id", children=["Button not clicked"])]),
        html.Button(id="button_id", children="Run Job!"),
        html.Button(id="cancel_button_id", children="Cancel Running Job!")]

layout2 = [html.Div([html.P(id="paragraph_id2", children=["Button not clicked"])]),
        html.Button(id="button_id2", children="Run Job!"),
        html.Button(id="cancel_button_id2", children="Cancel Running Job!")]

dash.register_page('page1', path='/', layout=layout1)
dash.register_page('page1', path='/2', layout=layout2)

@callback(
    output=Output("paragraph_id", "children"),
    inputs=Input("button_id", "n_clicks"),
    background=True,
    running=[
        (Output("button_id", "disabled"), True, False),
        (Output("cancel_button_id", "disabled"), False, True),
    ],
    cancel=[Input("cancel_button_id", "n_clicks"), Input('_pages_location', 'href')],
)

@callback(
    output=Output("paragraph_id2", "children"),
    inputs=Input("button_id2", "n_clicks"),
    background=True,
    running=[
        (Output("button_id2", "disabled"), True, False),
        (Output("cancel_button_id2", "disabled"), False, True),
    ],
    cancel=[Input("cancel_button_id2", "n_clicks"), Input('_pages_location', 'href')],
)
def update_clicks(n_clicks):
    time.sleep(4.0)
    return [f"Clicked {n_clicks} times"]


if __name__ == "__main__":
    app.run(debug=True)

Ran into an issue with duplicate outputs on the ids, this is most likely due to the cancel being listed in a callback, should probably alter these to allow for duplicate outputs.

@alexcjohnson

@JamesKunstle
Copy link
Author

Here's a direct reference to the code where this change could be made:
plotly/dash/blob/a7a12d180e16eac0a84b88e2b8dc8f7c7601cbaf/dash/_callback.py#L174

@alexcjohnson
Copy link
Collaborator

Spot on @JamesKunstle @BSd3v - I think (a) the cancel callback should always allow duplicates since it's not actually doing anything with that output, and (b) we should explore a dedicated feature that maybe even by default cancels background callbacks on page changes, since it seems like generally the output would be discarded in that case anyhow. It would need a way to opt out of course, or we make the feature opt-in if we think this would constitute a breaking change. @T4rk1n thoughts?

@T4rk1n
Copy link
Contributor

T4rk1n commented Jul 11, 2023

We should add allow_duplicate to the outputs of the cancel callback, but there is probably more to do in the renderer to collect the cancelJobs since it should have already been working with only one callback.

@T4rk1n
Copy link
Contributor

T4rk1n commented Jul 12, 2023

Found more issues when trying to fix:

  • Adding allow_duplicate with a single same Input still throw a DuplicateOutput on the renderer
  • the cancel inputs on the pages are not found on the renderer check.

@T4rk1n
Copy link
Contributor

T4rk1n commented Jul 12, 2023

@alexcjohnson I tried combining all the cancel inputs into a single callback, there is a validation error:

A nonexistent object was used in an "Input" of a Dash callback. The id of this object is

That seems like a duplicate of another validation we have that takes the validation_layout and is configurable with suppress_callback_exception. The other validation runs on every callbacks seems to clash with pages if you have a callback that shares inputs it makes that impossible. Think we can remove that?

@alexcjohnson
Copy link
Collaborator

If there are bg callbacks on separate pages (or in whatever way not all in the DOM simultaneously) then we'd have some inputs present and others missing, right? That's not going to work, unless we make optional inputs, something we've talked about at various points in the past.

But I guess due to the DuplicateOutput issue we'll need to combine any bg callbacks with identical cancel triggers into a single callback.

@T4rk1n
Copy link
Contributor

T4rk1n commented Jul 13, 2023

we'd have some inputs present and others missing, right? That's not going to work, unless we make optional inputs, something we've talked about at various points in the past.

Yes, that is what I was looking to do, turn out to be a pretty heavy refactor.

Maybe one last thing I can try is isolate all the callback inputs in single callbacks for each input, I think that might work.

T4rk1n added a commit that referenced this issue Jul 14, 2023
T4rk1n added a commit that referenced this issue Jul 18, 2023
Create a single cancel callback for each unique input.
@JamesKunstle
Copy link
Author

@T4rk1n @BSd3v I saw that a PR was merged for this, allowing for the 'cancel' Input callback to be reused for multiple background callbacks. Wanted to check in on whether this issue should also be closed, or whether something else needs to be finished first?

Is it reasonable to expect this patch to be in the next major release?

@T4rk1n
Copy link
Contributor

T4rk1n commented Jul 20, 2023

@JamesKunstle It was a partial fix, I think there is still value in adding auto cancellation of running tasks when leaving the page so we may leave this issue open. What do you think @alexcjohnson ?

Is it reasonable to expect this patch to be in the next major release?

Yes, it's going in next release.

@JamesKunstle
Copy link
Author

@T4rk1n That's awesome, thank you so much for working on this, I really appreciate it as a community member. This has been a performance blocker for us (we're relatively resource-limited so overhead, especially superfluous overhead, is blocking).

Having cancellation happen at the dash/dash-renderer level would be ideal, especially given that the dash-renderer knows which pages it's on, and which page it WAS on, and which promised components haven't finished.

If I can be of any help for anything else, please let me know. Otherwise I'll just be enthusiastically following this issue.

@JamesKunstle
Copy link
Author

@alexcjohnson @T4rk1n Following up on this issue- curious if there has been any further discussion on this?

@AlesyaSeliazniova30032012
Copy link

AlesyaSeliazniova30032012 commented Sep 14, 2023

Hi everybody. I have the same problem when switching between pages while loading data with cancelable callback. here is an example of what I'm doing:
@dash.callback( output=Output('dropdown2', 'options'), inputs=Input('dropdown1', 'value'), background=True, running=[ (Output('dropdown2', 'placeholder'), 'Loading...', 'Select container',), (Output('dropdown2', 'disabled'), True, False), ], cancel=[ Input("_pages_location", 'pathname'), ], prevent_initial_call=True, ) def choose_cnt(value): try: if not value: raise PreventUpdate elif value: options = some func() return options except: raise PreventUpdate
image

How to fix it? Thanks for any help

@gvwilson
Copy link
Contributor

@T4rk1n is this one still relevant?

@gvwilson gvwilson added P3 backlog feature something new labels Aug 13, 2024
@gvwilson gvwilson changed the title [Feature Request] Termination of 'outdated' long callbacks when page changes. enable termination of 'outdated' long callbacks when page changes Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature something new P3 backlog
Projects
None yet
Development

No branches or pull requests

7 participants