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

Accept a list of modes to open db with nomutex option. #284

Merged
merged 5 commits into from
Apr 23, 2024

Conversation

ColaCheng
Copy link
Contributor

In our use case, we only read the database and never write the data. We found this flag can open db with no mutex. It should be safe to open db as read-only and no mutex.

Ref: https://www.sqlite.org/c3ref/open.html

@warmwaffles
Copy link
Member

@ColaCheng I haven't messed with readonly no mutex before. Why wouldn't this be the default for readonly?

@ColaCheng ColaCheng force-pushed the add_readonly_nomutex branch from 4931a54 to 6548984 Compare April 9, 2024 03:21
@ColaCheng
Copy link
Contributor Author

@ColaCheng I haven't messed with readonly no mutex before. Why wouldn't this be the default for readonly?

Thank you for review. I made readonly along with no mutex by default.

@ColaCheng ColaCheng force-pushed the add_readonly_nomutex branch from 6548984 to 9998c00 Compare April 9, 2024 03:52
@warmwaffles
Copy link
Member

I'll need to look at the documentation to figure out if no mutex should be the default. We are compiling sqlite with thread safety enabled, so this flag makes sense. But if we disabled thread safety on the compilation this would effectively be a no-op.

I'll accept this once I learn more

@ColaCheng ColaCheng changed the title Add readonly_nomutex mode. Add nomutex flag to readonly mode. Apr 15, 2024
@warmwaffles
Copy link
Member

@ColaCheng I think we'll need to modify mode to allow a list of modes added. After toying with this some, it's probably best to let the caller explicitly state the modes they want.

I'm thinking something like

mode: :readonly
mode: :readwrite
mode: [:readonly, :nomutex]

If the mode is just :nomutex we should raise an exception to the programmer to specify :readonly or :readwrite. Because technically, :readwrite is allowed with :nomutex although not recommended.

@ColaCheng ColaCheng changed the title Add nomutex flag to readonly mode. Accept a list of modes to open db with nomutex option. Apr 23, 2024
@ColaCheng ColaCheng force-pushed the add_readonly_nomutex branch from bc6e0ed to a91728d Compare April 23, 2024 06:56
@ColaCheng
Copy link
Contributor Author

Hi @warmwaffles, updated according to the comments. Please review it. Thanks!

@warmwaffles warmwaffles merged commit 8cd3e03 into elixir-sqlite:main Apr 23, 2024
17 of 19 checks passed
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