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

Auto refresh is no longer working #20813

Closed
timroes opened this issue Jul 16, 2018 · 4 comments
Closed

Auto refresh is no longer working #20813

timroes opened this issue Jul 16, 2018 · 4 comments
Labels
blocker bug Fixes for quality problems that affect the customer experience Feature:Visualizations Generic visualization features (in case no more specific feature label is available) regression v6.4.0

Comments

@timroes
Copy link
Contributor

timroes commented Jul 16, 2018

Enabling auto refresh won't cause any additional requests to be send periodically anymore (this issue is not yet released).

This was caused by #20295

@timroes timroes added bug Fixes for quality problems that affect the customer experience blocker Feature:Visualizations Generic visualization features (in case no more specific feature label is available) v6.4.0 labels Jul 16, 2018
@timroes timroes changed the title Auto refresh is no longer working in visualize app Auto refresh is no longer working Jul 16, 2018
@nreese
Copy link
Contributor

nreese commented Jul 16, 2018

Talked to @spalger about moving all auto refresh timeout management logic into timefilter. Spencer suggested that it did not make sense and the better long term solution would be creating a new service to handle auto refresh state and timeout logic.

SearchPoll throttles the refresh interval timeout to the speed that courier can execute when the interval is too fast. This logic should be pulled out of Courier and put into the auto refresh service so that the timeout logic can be throttled to the speed for the slowest request when the interval is too fast.

We talked about just having SearchPoll emit an event on timefilter that could be subscribed to as a short term solution for 6.4. I have put up a WIP PR demonstrating this solution.

cc @stacey-gammon @markov00 @cjcenizal

@LeeDr
Copy link

LeeDr commented Jul 23, 2018

I guess this means there aren't any tests that verify auto refresh? Would that have to be a UI functional test or is there some more efficient way?

@stacey-gammon
Copy link
Contributor

Nope 😭 A glaring gap that I've been aware of for awhile but never found time to cover. Actually you can see this comment in our team meeting notes from back in April:

I'd like to add some auto refresh tests and time zone tests first. This will also be important for the search source clean up since that is very fragile code.

If I recall correctly I did add some timezone tests in there but never got around to the auto refresh ones.

I think this would have to be functional because of all the moving parts and the interaction with courier.

@timroes
Copy link
Contributor Author

timroes commented Jul 24, 2018

Fixed by #20863 - will create a follow up ticket for a proper solution as outlined by Nathan, that will introduce a new service

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker bug Fixes for quality problems that affect the customer experience Feature:Visualizations Generic visualization features (in case no more specific feature label is available) regression v6.4.0
Projects
None yet
Development

No branches or pull requests

4 participants