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

Additional Websocket Support #1700

Open
chriskacerguis opened this issue Apr 19, 2017 · 11 comments
Open

Additional Websocket Support #1700

chriskacerguis opened this issue Apr 19, 2017 · 11 comments

Comments

@chriskacerguis
Copy link
Contributor

The BU wants enable push updates to the SUI when something changes. Currently the SUI is using polling, and only the the notifications endpoint. We need to add support for web sockets to all endpoints.

@miq-bot assign skateman
@miq-bot add_label enhancement

[edit]: updated for clarity

/cc @martinpovolny, @himdel

@chriskacerguis
Copy link
Contributor Author

@skateman in Gitter https://gitter.im/ManageIQ/manageiq?at=58f7b6723e27cac331a46547 you mentioned some issues with Apache. Could you please elaborate a little here (for posterity)?

@himdel
Copy link
Contributor

himdel commented Apr 20, 2017

So.. these are the changes needed to achieve push updates:

  • first, we need the backend to know that an update has happened for any kind of change - since we can have multiple appliances touching the same DB, this would have to either happen on the db level (postgres triggers, but not sure about scalability), or on a separate redis instance shared by all the appliances

  • then we need a way to subscribe for updates to specific collections or records - again, the backend needs to know what each and every connected client is watching so that it can push the right updates to the right client

  • then there may be some websocket-related details to solve :)

So.. IMO this has nothing to do with websockets at this point, it's a rewrite-the-backend kind of task :).

@skateman
Copy link
Member

This is theoretically doable, but there are some technical constraints:

  • ActionCable uses PG's LISTEN and NOTIFY in its pub-sub implementation, i.e. there is more DB usage compared with a normal API call as the LISTEN is blocking on a persistent PG connection.
  • Under certain conditions the Apache version we're using keeps closed WebSocket connections in CLOSE_WAIT state until the TCP timeout (120s). If Apache opens a new WebSocket connection, there is a chance that it uses the same TCP port as a previously closed connection in CLOSE_WAIT state. From this the browser notices only that it can't connect to the WebSocket server. There is an open BZ for this and maybe @psav can tell you more.

@chriskacerguis
Copy link
Contributor Author

@himdel - Agree, LOTS of dependancies here for sure.

This is a non-trivial task for sure. Ultimately, John will need to make the call on the priority and @chessbyte will need to do the resourcing mojo :)

/cc @Fryguy

@martinpovolny
Copy link
Member

martinpovolny commented Apr 21, 2017

I'll sum up what was said here and on gitter.

What we have:

  • we have some basic notifications. That includes:
    • a mechanism to get the notifications from the appliance to the browser w/o polling (websockets)
    • a mechanism to get the notification from the DB w/o polling (actiontable using PG's LISTEN/NOTIFY)

Now we just dislay the notifications in the UIs.

What do we want?

We can have the UIs react to the notifications. React in such a way that when a notification of certain type is received a certain part of the UI is refreshed.

The notification can be received via the websocket interface, but there's no reason to do the refreshing via the websocket. I can be done as it is now.

Meaning we don't really need to change much about websockets.

That gets me to the point where we should change the title of this issue to something like:

Make ManageIQ emitt notifications on changes of entities such as VMs or Services and have the UIs react to such notifications by refreshing the display immediately.

I believe that this is not a trivial task and that this is by large part backend work.

I believe that this can be done area by area (e.g. starting with services and then adding VMs or instances etc.).

Hope that this description better reflects the PM intent and gets us to the same page.

@miq-bot
Copy link
Member

miq-bot commented Oct 28, 2017

This issue has been automatically marked as stale because it has not been updated for at least 6 months.

If you can still reproduce this issue on the current release or on master, please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions!

@chriskacerguis
Copy link
Contributor Author

This is still very much a need. Adding this comment to remove the "stale".

@skateman
Copy link
Member

@miq-bot rm_label stale

@chessbyte
Copy link
Member

@martinpovolny any update on this issue?

@martinpovolny
Copy link
Member

Significant backend work is needed so that the UI could subscribe to changes in particular entities and collections based on where in the UI one is located and what permissions one has.

W/o that the UI has no other way to detect changes than to poll.

With this, the information about changes could come via the websocket w/o polling and the UI could reload only what is needed and when it is needed.

@skateman
Copy link
Member

The best example of polling is using the wait_for_task, we could replace all of it with a websocket-backed server push. But as @martinpovolny stated, it would require some adjustments on the backend side. However, if we really gonna do the jobs & tasks UI, it would require backend adjustments anyway, so we could incorporate into these changes the websocket emits from running & completed tasks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants