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

idea: require backing module for content all the time #6630

Open
chu11 opened this issue Feb 13, 2025 · 3 comments
Open

idea: require backing module for content all the time #6630

chu11 opened this issue Feb 13, 2025 · 3 comments

Comments

@chu11
Copy link
Member

chu11 commented Feb 13, 2025

There is a lot of special case code for dealing with the "none" backing module (i.e. is a backing module loaded or not). It may make things simpler if we simply require a backing module all of the time.

Note that we still want to support "no real backing module" for performance (especially for sub-instances), so hypothetically there could be a special "memory only" backing module. or perhaps sqlite run in memory only mode. or dumbly just a backing module called "none" that does absolutely nothing.

@garlick
Copy link
Member

garlick commented Feb 13, 2025

Yeah, I think it suffices to say that we have options if we don't want to make content persistent, and also that it's not a situation that's come up IRL recently, so fine to nix it in the name of simplifying code.

It looks like the backing modules all register the content-backing service already so the main thing is eliminating the content.register-backing RPCs and loading the backing module before the cache module. Plus removing the "none" special cases from rc1, not routing the checkpoint stuff through the cache, eliminating code to let the memory cache grow when backing is unavailable, etc..

@chu11
Copy link
Member Author

chu11 commented Feb 13, 2025

Yeah, I think it suffices to say that we have options if we don't want to make content persistent, and also that it's not a situation that's come up IRL recently, so fine to nix it in the name of simplifying code.

The performance issue I'm medium concerned with is we will generate unnecessary RPCs. i.e. the content wants to flush to the backing store that does nothing, so it's just a wasted RPC. For sub-instances that don't need a backing store, we are introducing some extra traffic and being busy.

I'm trying to think if there's some "trick" we could do here, but a super clean one isn't coming to mind. One possibly not great idea is to just special case the word "none" as a backing. If "none" is the backing module, content.flush doesn't work and maybe some other RPCs won't. Or perhaps the backing modules will advertise "we can't handle flushes" and the content module can adjust accordingly.

@garlick
Copy link
Member

garlick commented Feb 14, 2025

Performance benefits were not compelling as I recall. I'd say just eliminate that option and take the win.

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

No branches or pull requests

2 participants