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.

Sign up for GitHub

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

Jump to bottom

[Session management] Update session cleanup PIT query to use partial search results #203440

Closed
SiddharthMantri opened this issue Dec 9, 2024 · 1 comment · Fixed by #203413
Closed

[Session management] Update session cleanup PIT query to use partial search results #203440

SiddharthMantri opened this issue Dec 9, 2024 · 1 comment · Fixed by #203413
Assignees
@SiddharthMantri
Labels
bug Fixes for quality problems that affect the customer experience Feature:Security/Session Management Platform Security - Session Management Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more!

Comments

@SiddharthMantri
Copy link
Contributor

We are seeing some error logs around session_cleanup, primarily in serverless.

The task uses a Point-in-time query to delete the sessions that require clean up. On investigating, it is possible that the PIT query is trying to run on shards that are unavailable or were first missing when the PIT was created. Relevant function:

https://github.com/elastic/kibana/blob/main/x-pack/plugins/security/server/session_management/session_index.ts#L829

The previous fix in #200912 added the flag only to the search query which seems to not have fixed the issue.

This fix is essential for ensuring more reliable session management and minimizing error log noise, especially in serverless environments.
@SiddharthMantri SiddharthMantri added bug Fixes for quality problems that affect the customer experience Feature:Security/Session Management Platform Security - Session Management Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! labels Dec 9, 2024
@SiddharthMantri SiddharthMantri self-assigned this Dec 9, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-security (Team:Security)

@SiddharthMantri SiddharthMantri mentioned this issue Dec 9, 2024
Merged
2 tasks
SiddharthMantri added a commit that referenced this issue Dec 10, 2024
@SiddharthMantri
[Session management] update cleanup query to allow partial search res…
91fec7a 
…ults for PIT query (#203413)

Closes #203440

### Summary
Update session cleanup task by adding the partial search results flag to
the PIT query as well and not just the search query.

#### Notes 
In the previous “fix”, the partial search results flag was incorrectly
added to the search query that depended on the PIT query. However, the
correct way is to set the flag when we openPointInTimeQuery which is
then used in the subsequent search query

### Release notes
Fixes error with opening point in time query for session deletion by now
accounting for partial results.


Check the PR satisfies following conditions. 

Reviewers should verify this PR satisfies this list as well.


- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] The PR description includes the appropriate Release Notes section,
and the correct `release_node:*` label is applied per the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Dec 10, 2024
@SiddharthMantri
[Session management] update cleanup query to allow partial search res…
0d6a76e 
…ults for PIT query (elastic#203413)

Closes elastic#203440

### Summary
Update session cleanup task by adding the partial search results flag to
the PIT query as well and not just the search query.

#### Notes
In the previous “fix”, the partial search results flag was incorrectly
added to the search query that depended on the PIT query. However, the
correct way is to set the flag when we openPointInTimeQuery which is
then used in the subsequent search query

### Release notes
Fixes error with opening point in time query for session deletion by now
accounting for partial results.

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] The PR description includes the appropriate Release Notes section,
and the correct `release_node:*` label is applied per the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

(cherry picked from commit 91fec7a)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Dec 10, 2024
@SiddharthMantri
[Session management] update cleanup query to allow partial search res…
1dac04c 
…ults for PIT query (elastic#203413)

Closes elastic#203440

### Summary
Update session cleanup task by adding the partial search results flag to
the PIT query as well and not just the search query.

#### Notes
In the previous “fix”, the partial search results flag was incorrectly
added to the search query that depended on the PIT query. However, the
correct way is to set the flag when we openPointInTimeQuery which is
then used in the subsequent search query

### Release notes
Fixes error with opening point in time query for session deletion by now
accounting for partial results.

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] The PR description includes the appropriate Release Notes section,
and the correct `release_node:*` label is applied per the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

(cherry picked from commit 91fec7a)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Dec 10, 2024
@SiddharthMantri
[Session management] update cleanup query to allow partial search res…
773286f 
…ults for PIT query (elastic#203413)

Closes elastic#203440

### Summary
Update session cleanup task by adding the partial search results flag to
the PIT query as well and not just the search query.

#### Notes
In the previous “fix”, the partial search results flag was incorrectly
added to the search query that depended on the PIT query. However, the
correct way is to set the flag when we openPointInTimeQuery which is
then used in the subsequent search query

### Release notes
Fixes error with opening point in time query for session deletion by now
accounting for partial results.

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] The PR description includes the appropriate Release Notes section,
and the correct `release_node:*` label is applied per the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

(cherry picked from commit 91fec7a)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Dec 10, 2024
@SiddharthMantri
[Session management] update cleanup query to allow partial search res…
6d641c1 
…ults for PIT query (elastic#203413)

Closes elastic#203440

### Summary
Update session cleanup task by adding the partial search results flag to
the PIT query as well and not just the search query.

#### Notes
In the previous “fix”, the partial search results flag was incorrectly
added to the search query that depended on the PIT query. However, the
correct way is to set the flag when we openPointInTimeQuery which is
then used in the subsequent search query

### Release notes
Fixes error with opening point in time query for session deletion by now
accounting for partial results.

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] The PR description includes the appropriate Release Notes section,
and the correct `release_node:*` label is applied per the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

(cherry picked from commit 91fec7a)
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this issue Dec 12, 2024
@SiddharthMantri @CAWilson94
[Session management] update cleanup query to allow partial search res…
5bb39b9 
…ults for PIT query (elastic#203413)

Closes elastic#203440

### Summary
Update session cleanup task by adding the partial search results flag to
the PIT query as well and not just the search query.

#### Notes 
In the previous “fix”, the partial search results flag was incorrectly
added to the search query that depended on the PIT query. However, the
correct way is to set the flag when we openPointInTimeQuery which is
then used in the subsequent search query

### Release notes
Fixes error with opening point in time query for session deletion by now
accounting for partial results.


Check the PR satisfies following conditions. 

Reviewers should verify this PR satisfies this list as well.


- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] The PR description includes the appropriate Release Notes section,
and the correct `release_node:*` label is applied per the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@SiddharthMantri SiddharthMantri

Labels
bug Fixes for quality problems that affect the customer experience Feature:Security/Session Management Platform Security - Session Management Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more!
Projects
None yet
Milestone
No milestone
2 participants
@SiddharthMantri @elasticmachine