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

Pause refresh when page is hidden #177693

Merged
merged 2 commits into from
Mar 4, 2024

Conversation

Dosant
Copy link
Contributor

@Dosant Dosant commented Feb 23, 2024

Summary

close #1878

Pauses auto-refresh when a page is not visible.
I tested and didn't notice any issues, but looking for more testing help

Release Notes

Auto-refresh pauses when the page is not visible.

@Dosant
Copy link
Contributor Author

Dosant commented Feb 23, 2024

/ci

@Dosant Dosant changed the title pause refresh when hidden Pause refresh when page is hidden Feb 23, 2024
@Dosant Dosant added Feature:Unified search Unified search related tasks release_note:enhancement Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas Team:Visualizations Visualization editors, elastic-charts and infrastructure Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) labels Feb 23, 2024
@Dosant Dosant marked this pull request as ready for review February 23, 2024 13:43
@Dosant Dosant requested review from a team as code owners February 23, 2024 13:43
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-visualizations (Team:Visualizations)

@elasticmachine
Copy link
Contributor

Pinging @elastic/appex-sharedux (Team:SharedUX)

@timductive
Copy link
Member

Nice work @Dosant can we also track impact of this in our cloud telemetry after release? Ideally we would see a reduction in proxy logs or something like that.

Copy link
Member

@lukasolson lukasolson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I'm noticing in Discover (not in Lens) we still seem to get refreshes even when the page is not visible. I've set the refresh interval to 5s and when I switch tabs I'm still getting refreshes every 30s or so.

After putting in some log statements it looks like these refreshes for some reason are not initiated by this code (I'm not getting any events from tickWhenVisible$), so I think we are still good to move forward with this change... But we will want to do some additional investigation on the Discover side as to why these search requests are still being fired.

Anyway, changes LGTM!

Copy link
Contributor

@mbondyra mbondyra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noticed the same thing as @lukasolson, but it is a great improvement for Lens and Dashboard so approving! Hopefully we'll fix the discover case too 😊

@Dosant
Copy link
Contributor Author

Dosant commented Mar 4, 2024

can we also track impact of this in our cloud telemetry after release? Ideally we would see a reduction in proxy logs or something like that.

@timductive, I doubt we will be able to see the impact. I am not sure how we can only filter in the excessive traffic caused by auto-refresh while in the background to measure its impact

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
data 495 496 +1

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
data 415.4KB 415.7KB +287.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@Dosant
Copy link
Contributor Author

Dosant commented Mar 4, 2024

@lukasolson, we with @mbondyra found the possible reason for the refetches in the background:

Looks like there are coming from this snippet

const schedulePollSearches = () => {
return timer(KEEP_ALIVE_COMPLETED_SEARCHES_INTERVAL).pipe(
mergeMap(() => {
const searchesToKeepAlive = this.state.get().trackedSearches.filter(
(s) =>
!s.searchMeta.isStored &&
s.state === TrackedSearchState.Completed &&
s.searchMeta.lastPollingTime.getTime() < Date.now() - 5000 // don't poll if was very recently polled
);
return from(
Promise.all(
searchesToKeepAlive.map((s) =>
s.searchDescriptor.poll().catch((e) => {
// eslint-disable-next-line no-console
console.warn(
`Error while polling search to keep it alive. Considering that it is no longer possible to extend a session.`,
e
);
if (this.isCurrentSession(sessionId)) {
this._disableSaveAfterSearchesExpire$.next(true);
}
})
)
)
);
}),
repeat(),
takeUntil(this.disableSaveAfterSearchesExpire$.pipe(filter((disable) => disable)))
);
};
where we're keeping the async searches alive for 5 minutes with 30s interval.
Btw, the same should happen on a dashboard with an async search panel (e.g lens)

So these refreshes are expected and we won't get rid of them without disabling search sessions

@Dosant Dosant merged commit b635674 into elastic:main Mar 4, 2024
18 checks passed
@Dosant Dosant self-assigned this Mar 4, 2024
@kibanamachine kibanamachine added v8.14.0 backport:skip This commit does not require backporting labels Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Unified search Unified search related tasks release_note:enhancement Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.14.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Disable Refresh while not in focus
7 participants