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

Scd subscriptions ttl #1134

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

callumdmay
Copy link

There has been prior effort in addressing dangling subscriptions in #1109 and #1088, however in our load testing efforts on v0.18.0 we have observed that some expired subscriptions are not being cleaned up. The root cause of this should be identified eventually, but as a mitigation we can add a row-level ttl to the table to prevent unbounded table growth.

Copy link

linux-foundation-easycla bot commented Nov 25, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@barroco
Copy link
Contributor

barroco commented Nov 27, 2024

Hi @callumdmay, thank you for the contribution. v0.19.0 is going to introduce a command to evict expired subscriptions. See https://github.com/interuss/dss/blob/master/cmds/db-manager/cleanup/README.md for instructions and #1116 for the implementation. In the discussion #1074 leading to this feature, we have left the approach to scheduling open. I just created this issue to make a decision #1135 .
If you are interested in the evict command, you can already test it with v0.19.0-rc2. If you need a stable release, please, let us know.

@callumdmay
Copy link
Author

Hey @barroco, what's the advantage of using the command over enforcing a ttl? That would do the cleanup automatically. Happy to adjust the ttl window as well if we would like to keep around expired subscriptions for longer.

@barroco
Copy link
Contributor

barroco commented Dec 4, 2024

Hi @callumdmay, thank you for your patience. we discussed during yesterday's contributors sync this topic and we would welcome this contribution in addition to what has been done in #1116. We just have concerns on the following points:

  1. Is it possible that deleting subscriptions like this could create inconsistencies on other entities?
  2. Since we plan to rollout this feature to all deployment, we would want to have it tested.
  3. The standard do not prevent a managing USS to extend an expired subscription. Therefore, it would be useful to extend the ttl window as you suggested.

what's the advantage of using the command over enforcing a TTL ?

The key point at this stage is that Yugabyte (YSQL mode) does not support the TTL feature.

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.

2 participants