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

WIP: Fix never expire current ViewVersion #12051

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

c-thiel
Copy link
Contributor

@c-thiel c-thiel commented Jan 22, 2025

WIP: For now this is just the breaking test which I believe should work.
The actual fix is missing.

Currently if more and more ViewVersion are added to the Metadata, eventually even the current version will expire. Currently expiration works as follows:

  1. All available view version ids are sorted
  2. The highest version ids are retained
  3. History / View log is cleaned up by removing all history items that came before any unknown version.

The problem occurs if we add view versions but never set them current. In this case the versions pile up and we eventually expire the whole view log as well as the current view version. I don't think this is desired behavior. This is a niche problem, as most clients create a view and set it active immediately.

I see two solutions:

  • Always keep the current version. Reduce the number of highest IDs to keep by 1.
  • Don't expire the highest view ids, but instead expire based on the view_log (previously current view versions). We would need another mechanism to expire the versions itself though. Is there a good use-case to keep a version that is neither current nor part of the view_log?

CC @nastra

@Fokko
Copy link
Contributor

Fokko commented Jan 23, 2025

Thanks for spotting this @c-thiel. I would be leaning towards:

  1. Always keep the current version.

Let's hear what Eduard thinks :)

@nastra
Copy link
Contributor

nastra commented Jan 23, 2025

Thanks for spotting this @c-thiel. I would be leaning towards:

  1. Always keep the current version.

Let's hear what Eduard thinks :)

Yep that's also what I'm leaning towards and what I implemented in #12067

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants