Consider changing from polling to push-based model for Explorer

[bwateratmsft]

This comes from discussion in #1351. Currently we poll every 2 s (formerly 1 s) for data from Docker via Dockerode. We can use Dockerode's getEvents method to take advantage of a push-based model, and use the information from those events to only refresh specific nodes, or avoid refreshing entirely if we want to get more advanced.

[karolz-ms]

IMO this would be a good change to make, with one caveat: in my experience with incremental-push models, things get "out of sync" eventually, so there is always a need for occasional server poll and comparing what the client has with what the server has. But that poll can be done significantly less frequently than in the pure-poll model.

[bwateratmsft]

@karolz-ms That's where I'm leaning as well--just use the events as a hint to refresh (which we can also scope to the particular tree view affected). This would dramatically reduce the amount of refreshing we do, as networks/images/volumes change pretty slowly and containers a bit faster, but not once-every-second fast. We can also put a maximum amount of time to wait before doing a full refresh anyway, e.g. 1 minute.

@philliphoff Can you speak to the reliability of these events from the container tools window in VS? Is it a dependable channel?

[philliphoff]

I'm not aware of any reliability issues, but it does tend to push a lot of events so it's necessary to filter/debounce/throttle them before refreshing. (Note that CTW doesn't look at the event data itself when it refreshes, but simply uses them as a trigger to do a wholesale refresh. Using the event data to update only the changed objects could be a further optimization in the future.)

[GammaGames]

Would this fix the issue where I'm in the explorer and I swap to the docker panel and right click to attach to a container, but it gives me the error that the container doesn't exist anymore? It seems very slow to update since it was redesigned to have more panels

[philliphoff]

@GammaGames If the underlying issue was a refresh frequency issue, it might. The current refresh rate default is 2s, and auto-refresh requires the explorer to be visible and VS Code be in focus. That means the explorer will tend to lag behind continuous Docker activity. You can change the docker.explorerRefreshInterval setting to make refresh happen more frequently (at the cost of more resources spent, obviously).

Since (in my experience) Docker activity tends to be "burst-y" rather than continuous, switching to a push- (i.e. event-) based model will be more efficient as well as better keep the explorer in sync with Docker. That is the model we use in the Containers Tools Window in Visual Studio and it seems to work well.

[GammaGames]

@philliphoff changing it to 1s definitely made it refresh faster after I opened it. I agree that the event based system would be better (I was kinda surprised it wasn't already using one).
Would it be possible to force the explorer to pull when it is opened to ensure the current information isn't lagging behind when switching panels? That model could even be carried over into the push-based system to help prevent the interface from getting out of sync.

[philliphoff]

@GammaGames The explorer does schedule a refresh when it is brought into view (or a sub-panel expanded from being collapsed) but, you're right, that scheduled refresh uses the interval setting and therefore isn't immediate. I think the question would be, given that a refresh is inherently asynchronous, is refreshing "immediately" effectively faster than waiting for the next interval in that case. At 2s, probably yes, but for shorter intervals I think the answer becomes less obvious. It's definitely something that should be considered in the switch to an event-based model.

[GammaGames]

I think the question would be, given that a refresh is inherently asynchronous, is refreshing "immediately" effectively faster than waiting for the next interval in that case

My naive first attempt would probably be to add a debounce. The panel itself wouldn't fetch every time it is focused, but if it's been >10s since the last pull it would stop listening for events, pull current status, and begin listening again. Then after that repeat the pause, pull, resume steps every minute or so.
This would mean the panel wouldn't take any resources while it is hidden and it would hopefully stay more up to date. There could be a 10s grace period after the panel is closed until it turns off the event listener so it lines up with the above timing.

[bwateratmsft]

We do already have logic to skip refreshes if the window isn't focused IIRC, so that part should be covered.

[bwateratmsft]

@karolz-ms if we end up using the Docker SDK for Node, this might be easier, or at least a good time to switch.

[bwateratmsft]

I don't think this would be possible in the new SDK and we won't be investing enough in the old SDK to justify this.