Skip to content

Commit

Permalink
[8.17] [Session management] update cleanup query to allow partial sea…
Browse files Browse the repository at this point in the history
…rch results for PIT query (elastic#203413) (elastic#203539)

# Backport

This will backport the following commits from `main` to `8.17`:
- [[Session management] update cleanup query to allow partial search
results for PIT query
(elastic#203413)](elastic#203413)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT
[{"author":{"name":"Sid","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-12-10T11:01:57Z","message":"[Session
management] update cleanup query to allow partial search results for PIT
query (elastic#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 (elastic#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 (elastic#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 <[email protected]>
  • Loading branch information
kibanamachine and SiddharthMantri authored Dec 10, 2024
1 parent 76c5c4d commit 8d3d226
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,7 @@ describe('Session index', () => {
{
index: aliasName,
keep_alive: '5m',
allow_partial_search_results: true,
},
{ ignore: [404], meta: true }
);
Expand All @@ -454,6 +455,7 @@ describe('Session index', () => {
{
index: aliasName,
keep_alive: '5m',
allow_partial_search_results: true,
},
{ meta: true }
);
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
);
Expand All @@ -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 }
));
Expand All @@ -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) {
Expand Down

0 comments on commit 8d3d226

Please sign in to comment.