mirror of
https://github.com/elastic/kibana.git
synced 2025-04-24 01:38:56 -04:00
[8.16] [Session management] update cleanup query to allow partial search results for PIT query (#203413) (#203538)
# Backport This will backport the following commits from `main` to `8.16`: - [[Session management] update cleanup query to allow partial search results for PIT query (#203413)](https://github.com/elastic/kibana/pull/203413) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Sid","email":"siddharthmantri1@gmail.com"},"sourceCommit":{"committedDate":"2024-12-10T11:01:57Z","message":"[Session management] update cleanup query to allow partial search results for PIT query (#203413)\n\nCloses https://github.com/elastic/kibana/issues/203440\r\n\r\n### Summary\r\nUpdate session cleanup task by adding the partial search results flag to\r\nthe PIT query as well and not just the search query.\r\n\r\n#### Notes \r\nIn the previous “fix”, the partial search results flag was incorrectly\r\nadded to the search query that depended on the PIT query. However, the\r\ncorrect way is to set the flag when we openPointInTimeQuery which is\r\nthen used in the subsequent search query\r\n\r\n### Release notes\r\nFixes error with opening point in time query for session deletion by now\r\naccounting for partial results.\r\n\r\n\r\nCheck the PR satisfies following conditions. \r\n\r\nReviewers should verify this PR satisfies this list as well.\r\n\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n- [x] The PR description includes the appropriate Release Notes section,\r\nand the correct `release_node:*` label is applied per the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"91fec7a69b4db5c1d5835add148b86c3732b02a7","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:Security","v9.0.0","Feature:Security/Session Management","backport:prev-major"],"title":"[Session management] update cleanup query to allow partial search results for PIT query","number":203413,"url":"https://github.com/elastic/kibana/pull/203413","mergeCommit":{"message":"[Session management] update cleanup query to allow partial search results for PIT query (#203413)\n\nCloses https://github.com/elastic/kibana/issues/203440\r\n\r\n### Summary\r\nUpdate session cleanup task by adding the partial search results flag to\r\nthe PIT query as well and not just the search query.\r\n\r\n#### Notes \r\nIn the previous “fix”, the partial search results flag was incorrectly\r\nadded to the search query that depended on the PIT query. However, the\r\ncorrect way is to set the flag when we openPointInTimeQuery which is\r\nthen used in the subsequent search query\r\n\r\n### Release notes\r\nFixes error with opening point in time query for session deletion by now\r\naccounting for partial results.\r\n\r\n\r\nCheck the PR satisfies following conditions. \r\n\r\nReviewers should verify this PR satisfies this list as well.\r\n\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n- [x] The PR description includes the appropriate Release Notes section,\r\nand the correct `release_node:*` label is applied per the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"91fec7a69b4db5c1d5835add148b86c3732b02a7"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/203413","number":203413,"mergeCommit":{"message":"[Session management] update cleanup query to allow partial search results for PIT query (#203413)\n\nCloses https://github.com/elastic/kibana/issues/203440\r\n\r\n### Summary\r\nUpdate session cleanup task by adding the partial search results flag to\r\nthe PIT query as well and not just the search query.\r\n\r\n#### Notes \r\nIn the previous “fix”, the partial search results flag was incorrectly\r\nadded to the search query that depended on the PIT query. However, the\r\ncorrect way is to set the flag when we openPointInTimeQuery which is\r\nthen used in the subsequent search query\r\n\r\n### Release notes\r\nFixes error with opening point in time query for session deletion by now\r\naccounting for partial results.\r\n\r\n\r\nCheck the PR satisfies following conditions. \r\n\r\nReviewers should verify this PR satisfies this list as well.\r\n\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n- [x] The PR description includes the appropriate Release Notes section,\r\nand the correct `release_node:*` label is applied per the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"91fec7a69b4db5c1d5835add148b86c3732b02a7"}}]}] BACKPORT--> Co-authored-by: Sid <siddharthmantri1@gmail.com>
This commit is contained in:
parent
30e04df353
commit
4fa7135580
2 changed files with 10 additions and 6 deletions
|
@ -446,6 +446,7 @@ describe('Session index', () => {
|
|||
{
|
||||
index: aliasName,
|
||||
keep_alive: '5m',
|
||||
allow_partial_search_results: true,
|
||||
},
|
||||
{ ignore: [404], meta: true }
|
||||
);
|
||||
|
@ -454,6 +455,7 @@ describe('Session index', () => {
|
|||
{
|
||||
index: aliasName,
|
||||
keep_alive: '5m',
|
||||
allow_partial_search_results: true,
|
||||
},
|
||||
{ meta: true }
|
||||
);
|
||||
|
@ -473,7 +475,6 @@ describe('Session index', () => {
|
|||
expect(mockElasticsearchClient.search).toHaveBeenCalledTimes(1);
|
||||
expect(mockElasticsearchClient.search).toHaveBeenCalledWith({
|
||||
_source_includes: 'usernameHash,provider',
|
||||
allow_partial_search_results: true,
|
||||
sort: '_shard_doc',
|
||||
track_total_hits: false,
|
||||
search_after: undefined,
|
||||
|
@ -556,7 +557,6 @@ describe('Session index', () => {
|
|||
expect(mockElasticsearchClient.search).toHaveBeenCalledTimes(1);
|
||||
expect(mockElasticsearchClient.search).toHaveBeenCalledWith({
|
||||
_source_includes: 'usernameHash,provider',
|
||||
allow_partial_search_results: true,
|
||||
sort: '_shard_doc',
|
||||
track_total_hits: false,
|
||||
search_after: undefined,
|
||||
|
@ -651,7 +651,6 @@ describe('Session index', () => {
|
|||
expect(mockElasticsearchClient.search).toHaveBeenCalledTimes(1);
|
||||
expect(mockElasticsearchClient.search).toHaveBeenCalledWith({
|
||||
_source_includes: 'usernameHash,provider',
|
||||
allow_partial_search_results: true,
|
||||
sort: '_shard_doc',
|
||||
track_total_hits: false,
|
||||
search_after: undefined,
|
||||
|
@ -740,7 +739,6 @@ describe('Session index', () => {
|
|||
expect(mockElasticsearchClient.search).toHaveBeenCalledTimes(1);
|
||||
expect(mockElasticsearchClient.search).toHaveBeenCalledWith({
|
||||
_source_includes: 'usernameHash,provider',
|
||||
allow_partial_search_results: true,
|
||||
sort: '_shard_doc',
|
||||
track_total_hits: false,
|
||||
search_after: undefined,
|
||||
|
@ -854,7 +852,6 @@ describe('Session index', () => {
|
|||
expect(mockElasticsearchClient.search).toHaveBeenCalledTimes(1);
|
||||
expect(mockElasticsearchClient.search).toHaveBeenCalledWith({
|
||||
_source_includes: 'usernameHash,provider',
|
||||
allow_partial_search_results: true,
|
||||
sort: '_shard_doc',
|
||||
track_total_hits: false,
|
||||
search_after: undefined,
|
||||
|
|
|
@ -830,6 +830,10 @@ export class SessionIndex {
|
|||
{
|
||||
index: this.aliasName,
|
||||
keep_alive: SESSION_INDEX_CLEANUP_KEEP_ALIVE,
|
||||
// @ts-expect-error client support this option, but it is not documented and typed yet.
|
||||
// once support added we should remove this expected type error
|
||||
// https://github.com/elastic/elasticsearch-specification/issues/3144
|
||||
allow_partial_search_results: true,
|
||||
},
|
||||
{ ignore: [404], meta: true }
|
||||
);
|
||||
|
@ -841,6 +845,10 @@ export class SessionIndex {
|
|||
{
|
||||
index: this.aliasName,
|
||||
keep_alive: SESSION_INDEX_CLEANUP_KEEP_ALIVE,
|
||||
// @ts-expect-error client support this option, but it is not documented and typed yet.
|
||||
// once support added we should remove this expected type error
|
||||
// https://github.com/elastic/elasticsearch-specification/issues/3144
|
||||
allow_partial_search_results: true,
|
||||
},
|
||||
{ meta: true }
|
||||
));
|
||||
|
@ -857,7 +865,6 @@ export class SessionIndex {
|
|||
size: SESSION_INDEX_CLEANUP_BATCH_SIZE,
|
||||
sort: '_shard_doc',
|
||||
track_total_hits: false, // for performance
|
||||
allow_partial_search_results: true,
|
||||
});
|
||||
const { hits } = searchResponse.hits;
|
||||
if (hits.length > 0) {
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue