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

Adds TTL support to SQL server #3277

Merged
merged 3 commits into from
Mar 29, 2023
Merged

Conversation

JoshVanL
Copy link
Contributor

Implemented here.

Signed-off-by: joshvanl <me@joshvanl.dev>

- 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.

Copy link
Contributor

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.

Copy link
Contributor Author

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?

Copy link
Contributor

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.

JoshVanL and others added 2 commits March 20, 2023 10:57
…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>
@github-actions
Copy link

Stale PR, paging all reviewers

@github-actions github-actions bot added the stale label Mar 26, 2023
@ItalyPaleAle
Copy link
Contributor

@dapr/approvers-docs CC

@github-actions github-actions bot removed the stale label Mar 29, 2023
Copy link
Collaborator

@hhunter-ms hhunter-ms left a comment

Choose a reason for hiding this comment

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

lgtm

@hhunter-ms hhunter-ms merged commit 8f4fb1c into dapr:v1.10 Mar 29, 2023
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