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

Etcd cleanup #5065

Merged
merged 12 commits into from
Oct 15, 2024
Merged

Etcd cleanup #5065

merged 12 commits into from
Oct 15, 2024

Conversation

ManishaKumari295
Copy link

@ManishaKumari295 ManishaKumari295 commented Sep 26, 2024

closed #5063

Description
The user sessions gets piled up in etcd occupying upto 2bg space .

Change in behavior
Deleting the etcd user_sesssions them after 6 minutes of creation ( Access token of session gets expired in 5 mnts) This change will make sure the memory occupancy uptil 1.8 GB will be eradicated

Added
Added TTL time of 6 minutes in session.go file for each entry regarding user-sessions.

Fixed
This fixes the etcd space occupancy issue .

Change verification
Do sensuctl configure.
Check respective entry in etcd ar /sensu.io/user-sessions/admin/xxyyzz[session_id]
this entry will be deleted automatically after 11 minutes if the backend keep .
I have the video of default behaviour ( which caused the issue) and the video of fixed behaviour with TTL=2mins , unfortunately cannot be uploaded due to big size

Signed-off-by: manisha kumari <[email protected]>
Signed-off-by: manisha kumari <[email protected]>
Signed-off-by: manisha kumari <[email protected]>
Signed-off-by: manisha kumari <[email protected]>
Signed-off-by: manisha kumari <[email protected]>
Signed-off-by: manisha kumari <[email protected]>
if _, err := s.client.Put(ctx, userSessionPath(username, sessionID), state); err != nil {

leaseDuration := jwt.DefaultAccessTokenLifespan
ttl := int64(leaseDuration.Minutes()+1) * 60

Choose a reason for hiding this comment

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

Any reason to adding a minute to the lease?

Copy link
Author

Choose a reason for hiding this comment

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

This is to keep a buffer in deletion, no other reason.

CHANGELOG-6.md Outdated

### Changed
- ADD TTL to each user session in the etcd data store to prevent leak.
- TTl value is DefaultAccessTokenLifeSpan + 1 minute
Copy link
Contributor

Choose a reason for hiding this comment

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

Just one comment per change is required. The second entry can be removed.

Copy link
Author

Choose a reason for hiding this comment

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

Addressed

Signed-off-by: manisha kumari <[email protected]>
Copy link
Contributor

@fguimond fguimond left a comment

Choose a reason for hiding this comment

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

LGTM

@chavakula chavakula self-requested a review October 12, 2024 05:04
Signed-off-by: manisha kumari <[email protected]>
@ManishaKumari295
Copy link
Author

Merging to develop/6

@ManishaKumari295 ManishaKumari295 merged commit 613fc29 into develop/6 Oct 15, 2024
61 checks passed
@ManishaKumari295 ManishaKumari295 deleted the etcdCleanup branch October 15, 2024 05:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants