Skip to content

Adds Redis stream trimming by time(stream ID)#3710

Merged
yaron2 merged 8 commits into
dapr:mainfrom
elena-kolevska:redis-trim
May 6, 2025
Merged

Adds Redis stream trimming by time(stream ID)#3710
yaron2 merged 8 commits into
dapr:mainfrom
elena-kolevska:redis-trim

Conversation

@elena-kolevska

@elena-kolevska elena-kolevska commented Mar 19, 2025

Copy link
Copy Markdown
Contributor

Description

Adding the option to trim Redis streams based on timestamp. When minIDApprox is specified in the component file entries with IDs smaller than the specified threshold are evicted, keeping the stream trimmed to only contain entries with IDs greater than or equal to this value.
This can be used for time-based eviction since Redis stream IDs are in the format {timestamp-sequence}, where timestamp is in milliseconds since epoch.

Issue reference

#3664

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

yaron2
yaron2 previously approved these changes Apr 24, 2025

@yaron2 yaron2 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@JoshVanL

Copy link
Copy Markdown
Contributor

We should use a TTL time duration value as part of the component field, not a timestamp- the timestamp should be computed at runtime.

Component field values should be evergreen.

@elena-kolevska

Copy link
Copy Markdown
Contributor Author

We should use a TTL time duration value as part of the component field, not a timestamp- the timestamp should be computed at runtime.

Absolutely! That was my initial idea when i started doing this, I don't remember how it changed by the time I implemented it. I was just looking into it last week. I'll update the pr.

@elena-kolevska

Copy link
Copy Markdown
Contributor Author

Updated

Comment on lines +135 to +137
if s.StreamTTL == 0 {
return ""
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
if s.StreamTTL == 0 {
return ""
}
if s.StreamTTL <= 0 {
return ""
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think we shouldn't fail silently, and it would be too late to return the error here (we use this on message send). We should probably catch it on init.

@yaron2
yaron2 merged commit 5f17025 into dapr:main May 6, 2025
jjcollinge pushed a commit to jjcollinge/components-contrib that referenced this pull request May 12, 2025
Signed-off-by: Elena Kolevska <[email protected]>
Co-authored-by: Cassie Coyle <[email protected]>
Co-authored-by: Nelson Parente <[email protected]>
Co-authored-by: Yaron Schneider <[email protected]>
Co-authored-by: Bernd Verst <[email protected]>
passuied pushed a commit to passuied/components-contrib that referenced this pull request Feb 14, 2026
Signed-off-by: Elena Kolevska <[email protected]>
Co-authored-by: Cassie Coyle <[email protected]>
Co-authored-by: Nelson Parente <[email protected]>
Co-authored-by: Yaron Schneider <[email protected]>
Co-authored-by: Bernd Verst <[email protected]>
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.

7 participants