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

Add support for parallel input event handler #295

Open
fscherf opened this issue Dec 2, 2022 · 13 comments
Open

Add support for parallel input event handler #295

fscherf opened this issue Dec 2, 2022 · 13 comments
Labels
enhancement New feature or request

Comments

@fscherf
Copy link
Member

fscherf commented Dec 2, 2022

I try to encourage Lona users to use callback based input event handler to make their apps more scaleable, but currently input event handler run serialized.

Let's say we have two buttons. The first button triggers a long running operation (getting a value from a database), the second triggers a short running operation (toggling a popup). When the first button is pressed the popup can't be opened or closed, until get_count_from_database() is done. For the end-user the app then feels slow and unresponsive.

from lona.html import HTML, H1, Span, Button
from lona import LonaView

from my_code import long_running_database_call, Modal


class MyLonaView(LonaView):
    def get_count_from_database(self, input_event):
        self.count = long_running_database_call()

    def toggle_modal(self, input_event):
        self.modal.toggle()

    def handle_request(self, request):
        self.counter = Span()
        self.modal = Modal()

        return HTML(
            H1('The count from the database is: ', self.counter),
            self.modal,
            Button(
                'Get count from Database',
                handle_click=self.get_count_from_database,
            ),
            Button(
                'Toggle modal',
                handle_click=self.toggle_modal,
            )
        )

You can solve this issue by runnnig get_count_from_database() in a thread, but i think we can automate this.

My idea is to allow parallel execution of input event handlers in the server, but very limited. I think we can't just use a thread for every click a user makes (that would be ascalability nightmare), but we can make the count of parallel input event handler configurable in the settings.
I would propose 2 as default. With two parallel handlers you can run one long running call and service input like button presses, without handling threading by yourself.

@SmithChart, @maratori, @grigolet, @laundmo, @korantu

@fscherf fscherf added the enhancement New feature or request label Dec 2, 2022
@maratori
Copy link
Member

maratori commented Dec 2, 2022

I think async is better for this case than threads, but it may require API changes.

@fscherf
Copy link
Member Author

fscherf commented Dec 2, 2022

@maratori: do you have an idea how an API could look like?

@maratori
Copy link
Member

maratori commented Dec 2, 2022

@maratori: do you have an idea how an API could look like?

Nope :)

@fscherf
Copy link
Member Author

fscherf commented Dec 2, 2022

:D

I am not sure if i want to introduce an async API. To make the whole API sync was an design criteria actually, because in my opinion giving end-users an async API can be error prone:

async def handle_input_event(self, request):
    long_running_database_operation()

For someone who try to solve a real world problem, like an accounting APP, this code might look just fine and it will work on their laptop, but the io loop is blocked until the database operation is done.

We could add an async API for users who know what they are doing, but we need a solution for the sync world as well, and i am not sure if i want to support both.

@maratori
Copy link
Member

maratori commented Dec 2, 2022

Totally agree

@korantu
Copy link
Contributor

korantu commented Dec 2, 2022

I like keeping API sync idea, actually. async can add a lot of mental overhead.
Not sure about adding a low default limit though. Maybe make parallelizing callbacks simple but not default - and shout severe warning to user if theirs take long time.
Just a thought.

@korantu
Copy link
Contributor

korantu commented Dec 2, 2022

I like keeping API sync idea, actually. async can add a lot of mental overhead.

Maybe just use thread pools if scalability is an issue? I am not convinced in default low limit...

I was originally suggesting making threaded callbacks explicit, but that would complicate API, likely unnecessary.

@fscherf
Copy link
Member Author

fscherf commented Dec 2, 2022

@korantu:

Maybe make parallelizing callbacks simple but not default - and shout severe warning to user if theirs take long time.

That is pretty much what JavaScript does. You can add a callback to a button and run a short running function when it is clicked. If you run an endless loop in that callback, the page hangs until you return, and the browser will issue an warning that your code takes to long to complete.
This is a valid approach. My problem with this is you need to maintain two ways to do things: long-running and short-running. In JavaScript, "normal" callbacks are for short-running callbacks, and if you want to do things that have a longer runtime you have to use task workers, or slice your code into shorter running pieces or something like that.
If possible, i want to have only one API, that works for both (not sure if this possible, but let's aim for that).

I am not convinced in default low limit...

Actually there is already a limit, which currently is 1. Currently we use the websocket as a queue, and process events sequentially.

Maybe just use thread pools if scalability is an issue?

Lona already uses thread pools to run input event handler. This way we don't block the io loop and the user don't has to deal with multi-threading or async code. But we have to limit the threads a view may use at a time, so power-clicking does not slow down the experience for other users.

@korantu
Copy link
Contributor

korantu commented Dec 2, 2022

Thank you for clarifying, @fscherf. In my Lona app there is no scalability concerns, as I use it as local UI, which is not the target for your proposed change

@fscherf
Copy link
Member Author

fscherf commented Dec 2, 2022

@korantu: That's totaly fine and intended! And that's how many projects start. It would be nice if all Lona projects scale, even if they don't have to at the moment.

@fscherf
Copy link
Member Author

fscherf commented Dec 18, 2022

I did some experiments and i realized just limiting, the jobs a view can run simultaneously, to a finite number, has multiple problems:

  1. No great solution for "power-clicking". The proposed solution would give the user one thread for long running tasks, and one for short running tasks, to make the app feel fast and responsive, but nothing to distinguish between these two kinds of work-load. This means, if you have one button, that triggers a long running operation and you click it two times, you will have the initial problem, i try to solve here, on the third click.

  2. Designed bottleneck. Setting a finite number would mean that, even if you are the only user of a machine with 10 cores and 100 threads running, your clicks are bottlenecked to 2 (or another number).

Priorities instead of limits

I think a better solution for this is to not limiting parallel jobs, but to prioritize them: All input events get pushed into one PriorityQueue with a timestamp as their priority. Per view we have a counter of pending input events, that gets incremented on an incoming input event, and decremented when an input event got handled.

When an input event comes in, it gets put into the queue with its priority set to
(now() + timedelta(seconds=pending_input_events)).total_seconds.

This way "power-clicking" would be accounted for, and slow only one user down, but not his neighbors, this scales up and down and parallel execution would be possible.

Why timestamps? In this context they are called deadlines and ensure that jobs can be prioritized, but can't get stuck in the queue indefinitely. If we would use a base priority of 1, views with only short running jobs would never decrease their priority and would clog the top of the queue.

@SmithChart
Copy link
Contributor

When an input event comes in, it gets put into the queue with its priority set to
(now() + timedelta(seconds=pending_input_events)).total_seconds.

This could lead to a situation where events from a user could be re-ordered on the server, right?
I can imagine use-cases where users click a few buttons in quick succession (especially when each action takes a short moment). I guess any power-clicking user would their events to be executed in the order they have been issued.
When a previous click is intended to change a state and a later click should work on this changed state re-orderung would break this use-case.

@fscherf
Copy link
Member Author

fscherf commented May 31, 2023

This could lead to a situation where events from a user could be re-ordered on the server, right?

No.

priority = (now() + timedelta(seconds=user.pending_input_events)).total_seconds

I changed the variable names to make my point more clear.

This deadline-scheduling basically, and should ensure that all tasks that use the same user.pending_input_events (so per user) will be executed in the right order, but other tasks can be scheduled before or between them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants