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

[Reporting] adjust esquery refreshes settings #157323

Merged
merged 5 commits into from
May 17, 2023

Conversation

tsullivan
Copy link
Member

@tsullivan tsullivan commented May 10, 2023

Summary

https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-refresh.html

The goal is to use a refresh value of "wait_for" or false for Reporting's Index, Update, Delete, and Bulk operations, to limit negative impacts to search and indexing performance.

Checklist

Delete any items that are not applicable to this PR.

Risk Matrix

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk Probability Severity Mitigation/Notes
Unexpected behavior in report job listing Low Medium Users possibly may see report jobs out-of-sync with their state in the backend. This possibly will not be noticeable unless the user is watching the server logs in real-time while waiting for the report download to be available.

@tsullivan tsullivan self-assigned this May 10, 2023
@tsullivan tsullivan added Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) enhancement New value added to drive a business result ci:cloud-deploy Create or update a Cloud deployment ci:cloud-redeploy Always create a new Cloud deployment labels May 10, 2023
@tsullivan tsullivan force-pushed the reporting/no-refresh-true branch from 9e88585 to 4559065 Compare May 10, 2023 22:49

// Since the contents of the table have changed, we must reset the pagination
// and re-fetch. Otherwise, the Nth page we are on could be empty of jobs.
this.setState(() => ({ page: 0 }), this.fetchJobs);
Copy link
Member Author

Choose a reason for hiding this comment

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

Without refresh after delete, this fetchJobs tends to predicable stale report artifacts for recently deleted reoprts in the list.

@tsullivan tsullivan force-pushed the reporting/no-refresh-true branch from 4559065 to cb41969 Compare May 12, 2023 22:42
@tsullivan tsullivan marked this pull request as ready for review May 12, 2023 22:42
@tsullivan tsullivan requested a review from a team as a code owner May 12, 2023 22:42
@elasticmachine
Copy link
Contributor

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

@tsullivan tsullivan added the release_note:skip Skip the PR/issue when compiling release notes label May 16, 2023
@kibana-ci
Copy link
Collaborator

kibana-ci commented May 16, 2023

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
reporting 59.2KB 59.1KB -46.0B
Unknown metric groups

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
securitySolution 400 404 +4
total +6

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
securitySolution 480 484 +4
total +6

History

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

cc @tsullivan

Copy link
Contributor

@rshen91 rshen91 left a comment

Choose a reason for hiding this comment

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

tested locally with sample data - reporting LGTM!

@tsullivan tsullivan merged commit 74195d5 into elastic:main May 17, 2023
@kibanamachine kibanamachine added v8.9.0 backport:skip This commit does not require backporting labels May 17, 2023
@tsullivan tsullivan changed the title [Reporting] set esquery refreshes to wait_for [Reporting] adjust esquery refreshes settings May 17, 2023
@tsullivan tsullivan deleted the reporting/no-refresh-true branch May 17, 2023 21:11
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 ci:cloud-deploy Create or update a Cloud deployment ci:cloud-redeploy Always create a new Cloud deployment enhancement New value added to drive a business result release_note:skip Skip the PR/issue when compiling release notes Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) v8.9.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants