-
Notifications
You must be signed in to change notification settings - Fork 733
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
Adds TTL support to SQL server #3277
Conversation
Signed-off-by: joshvanl <me@joshvanl.dev>
daprdocs/content/en/reference/components-reference/supported-state-stores/setup-sqlserver.md
Outdated
Show resolved
Hide resolved
|
||
- Longer intervals require less frequent scans for expired rows, but can require storing expired records for longer, potentially requiring more storage space. If you plan to store many records in your state table, with short TTLs, consider setting `cleanupIntervalInSeconds` to a smaller value - for example, `300` (300 seconds, or 5 minutes). | ||
- If you do not plan to use TTLs with Dapr and the SQL Server state store, you should consider setting `cleanupIntervalInSeconds` to a value <= 0 (e.g. `0` or `-1`) to disable the periodic cleanup and reduce the load on the database. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you also add a note saying that the column doesn't have an index, and the consequences of that (garbage collectors perform full-table scans).
Whether an index is required or not depends on the use case. A table with lots of rows (hundreds of thousands) where only some have a TTL could benefit from an index as FTS in this case would be expensive and would remove few rows only. A table with not many rows and/or where most rows have a TTL (like, tables used for caching) would probably benefit from NOT having an index. An index makes queries faster but requires more memory, more storage, and (slightly) slows down inserts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ItalyPaleAle!
I disagree with this statement though:
and/or where most rows have a TTL (like, tables used for caching) would probably benefit from NOT having an index
Surely an index is beneficial because the index orders the ExpireDate
and therefore exits sooner when performing the <
condition on the scan?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uhmmm it may really depend on how many rows are in the table, and how many need to be deleted on the iteration. I guess we could benchmark that, but I don't care enough 😅 Point is, users may choose to add an index, or not, depending on their specific patterns.
…tate-stores/setup-sqlserver.md Co-authored-by: Alessandro (Ale) Segala <43508+ItalyPaleAle@users.noreply.github.com> Signed-off-by: Josh van Leeuwen <me@joshvanl.dev>
Signed-off-by: joshvanl <me@joshvanl.dev>
Stale PR, paging all reviewers |
@dapr/approvers-docs CC |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Implemented here.