-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[Uptime] Fix race on overview page query #67843
Conversation
Fixes elastic#67842 by requerying during the refine phase to see if a newer matching doc has come in.
Pinging @elastic/uptime (Team:uptime) |
}, | ||
}, | ||
latestMatching: { | ||
'@timestamp': '2020-06-04T12:39:54.698-0500', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't you try increasing the timestamp of latestMatching to cover the use case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I kept this case and added one for a future 'latest' timestamp
x-pack/plugins/uptime/server/lib/requests/search/refine_potential_matches.ts
Show resolved
Hide resolved
x-pack/plugins/uptime/server/lib/requests/search/refine_potential_matches.ts
Show resolved
Hide resolved
@@ -71,6 +71,7 @@ export default function ({ getService }: FtrProviderContext) { | |||
const url = getBaseUrl(dateRangeStart, dateRangeEnd) + `&filters=${filters}`; | |||
const apiResponse = await supertest.get(url); | |||
const nonSummaryRes = apiResponse.body; | |||
// expect(JSON.stringify(nonSummaryRes)).to.eql("foo"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if it's not applicable anymore, it's better to remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My mistake, left over from debugging!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shahzad31 I believe I've addressed all feedback here. |
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM !!
Fixes elastic#67842 by requerying during the refine phase to see if a newer matching doc has come in.
Fixes elastic#67842 by requerying during the refine phase to see if a newer matching doc has come in.
* master: (22 commits) Partial revert of "Sync Kerberos + Anonymous access tests with the latest `security/_authenticate` API (user roles now include roles of anonymous user)." (elastic#68624) adapt some snapshot test (elastic#68489) [APM] Service maps - Fix missing ML status for services with jobs but no anomalies (elastic#68486) [skip test] apis Kerberos security Kerberos authentication finishing SPNEGO should properly set cookie and authenticate user [SIEM][Exceptions] - ExceptionsViewer UI component part 2 (elastic#68294) Surface data streams in Index Management. (elastic#67806) Fix edit datasource not working following changes in elastic#67234 (elastic#68583) [Logs + Metrics UI] Clean up async plugin initialization (elastic#67654) APM Storybook fixes (elastic#68671) Upgrade EUI to v24.1.0 (elastic#68141) [ML] DF Analytics: Creation wizard part 2 (elastic#68462) [Uptime] Fix race on overview page query (elastic#67843) Prefer using npm_execpath when spawning Yarn (elastic#68673) [Security] [Cases] Attach timeline to existing case (elastic#68580) Use Search API in Vega (elastic#68257) [Component templates] Table view (elastic#68031) [Uptime] Added relative date info in cert status column (elastic#67612) [Endpoint] Re-enable Functional test case for Endpoint related pages (elastic#68445) run page_load_metrics tests in visual regresssion jobs (elastic#68570) Enable exhaustive-deps; correct any lint warnings (elastic#68453) ...
Fixes #67842 by re-querying during the refine phase to see if a newer matching doc has come in. This also simplifies the first 'potential' phase to only return monitor IDs. Getting the top check groups is no longer necessary since we determine this value in the 'refine' phase now.
Checklist
Delete any items that are not applicable to this PR.