Streaming: support streaming and a javascript test datasource (v2 observer)

[ryantxu]

This PR makes streaming support a core feature. Rather than:

It now looks something like:

Diagram: https://www.draw.io/#G19wNQ1N141Unx9v8NBF2UV88_UT82UDRL

[torkelo]

Rather than returning a result that the runner subscribes to, this passes an observer with the datasource query. This greatly simplifies the implementation overhead:

For the data source implementer or the code in the PanelQueryState? Passing in the observer looks clean but having to also return a streams events with the shutdown makes it feel a bit disjointed. But I do see that some of the OpenStreams logic is now gone in this version.

Which one do you prefer?

[torkelo]

the naming here feels very confusing, the obserer takes events, but the request method should return expected events? Is this more like a DataStreamState rather than an event?

[ryantxu]

This is the part I am least happy with -- yes DataStreamState is better.

The key issues we need to solve are:

• keeping consistent order as stream events arrive (otherwise the series colors change!)
• surviving a refresh event intelligently
• closing streams reliably -- when done or query changes

In the approach above, we:

1. return a list of DataStreamState from the ds.query() -- close open streams that don't match open keys.
2. as events arrive, it checks that they match a key from the latest response, otherwise close them

I would much rather keep the ds.query() result unchanged, and somehow manage order and stale streams with just the callbacks and query method. I think its possible, but have not figured it out yet :)

[ryantxu]

Which one do you prefer?

v2 is easier for most datasource implementations: you get passed a function and start writing values to it. However the semantics of how you shutdown/unsubscribe the server are still akward.

The v1 approach is easier if the datasource is backed by a rxjs Subject, but it adds extra management overhead to simple cases where there is really just one observer (the PanelQueryRunner).

I am focused on v2 now since it feels simpler/cleaner, but have left v1 because it works 😄and is closest to the existing streaming support.

[ryantxu]

@torkelo -- I now feel much better about the state of this PR! The main trick was hiding the aggregation details in PanelQueryState#getPanelData() -- it takes a simple path when not streaming and a more complicated one when it is.

I think we should:

1. do whatever minor cleanup/changes you think make sense
2. merge
3. focus on: MetricsPanelCtrl: use shared queryRunner to support query execution #16659

Meanwhile, I will:

• I will look at how to improve/refactor testdata/StreamHandler.ts to be generally useful. This is pretty close to the fetch streaming data source I have been working with
• I will look at MixedDatasource: stream results and keep track each sub-request #16676 for async query support. This is a variation of streaming we should make sure works -- essentially running multiple queries and updating them as they finish.

[torkelo]

• Looks like it has problems when switch dashboard and come back, streaming does not resume. Might be a problem in StreamHandler or in the closing logic. Will investigate later today I hope
• Looks like the streaming triggers renders for non visible panels (results in Graph errors when you have 2 panels and edit one of them and the other is streaming and triggers renders). Will try to investigate later.

[torkelo]

wonder if streams would work better as object map (by key) instead of an array. Maybe makes other stuff clunky, not sure.

[ryantxu]

I tried map... But consistent order is important. And we have to loop on the array anyway. When you add to theap it should not change the order of existing series

[torkelo]

ah, makes sense, array it will be then 👍

[torkelo]

can't find this used anywhere, there is a requestId prop on QueryResultMeta as well

[ryantxu]

Good catch -- i just removed it. This is an artifact of maintaining #16676

I'll revist this later -- the meta.requestId (not request!) gets used to pick the right sub-request to show in the QueryEditor -- it shows thing like how long it has been running etc.

[torkelo]

Looks like it has problems when switch dashboard and come back, streaming does not resume. Might be a problem in StreamHandler or in the closing logic. Will investigate later today I hope

Fixed this problem, was a StreamHandler that tried to reuse workers that had been unsubscribed (so had no observer).

Looks like the streaming triggers renders for non visible panels (results in Graph errors when you have 2 panels and edit one of them and the other is streaming and triggers renders). Will try to investigate later.

This problem is a bit trickier. We have to somehow block renders in PanelChrome when receiving data updates. One solution was to skip setState when not visible but save state and do it on next render (render triggeres when exiting fullscreen / edit mode)

[ryantxu]

After #15554, we could consider adding something like:
panel.getQueryRunner().enableStreamEvents( this.props.isInView ) when isInView changes:
https://github.com/grafana/grafana/pull/15554/files#diff-791865e8ee91eb8f3e2cfa67eff71ce5R96

[torkelo]

what if the panel query runner is shared with another panel that is visible? :)

[ryantxu]

better idea!

We just unsubscribe() when isInView becomes false, and re-subscribe when isInView becomes true. subscribe() immediately updates the state with any changes that happened while disconnected.

As implemented, the streams stay open even if everyone has disconnected. They only get closed if the next query/refresh does not match any open streams.

Lets wait till #15554 for this change since that PR makes visibility part of the state :)

[ryantxu]

I made one more change -- I did not like that the check for isRunning was based on the state variable. The usage is to check if we have an active open Promise... so I changed it to check that explicitly.

[torkelo]

Argh, so close to merge this but a few things with the PanelQueryRunner and PanelQueryState is bugging me, feel it very hard to read and understand some parts of this code.

1. Complex call chains. PanelQueryState has the dataStreamObserver that calls this.streamCallback that is set externally form PanelQueryRunner that points to throttled function that then calls back into PanelQueryState.getPanelData() (and then publish the result on the subject). Since the PanelQueryRunner owns the subject this roundabout is needed.
2. PanelQueryState.getPanelData() has side effects (updates the stream array). Maybe this can be moved from getPanelData to dataStreamObserver?

[ryantxu]

1. Complex call chains.
   The main issue here is needing to process all requests from the datasource, but throttle the updates to any subscribers.

The streamCallback is weird, but makes this distinction easier to support

2. dataStreamObserver is only called when it gets data from a stream -- getPanelData is called whenever you want data. so it makes sure the data is valid then returns it. Importantly, this happens after a query executes and within the throttled query callback. I can rename it to validateStreamsAndGetData() (or something similarly wordy) and maybe that will help explain the state manipulation in the getter

[ryantxu]

feel it very hard to read and understand some parts of this code.

Ha! it was hard to understand and write parts of this code :)