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

Add openEnvironmentWithFlags #2

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

Conversation

mihaigiurgeanu
Copy link

Add a new function, openEnvironmentWithFlags, to pass various flags when opening an environment. Useful if you want to use things like MDB_NOSYNC optimization.

@verement
Copy link
Owner

Thanks for this proposal.

There are several reasons I hesitate to merge this as-is. In general, I think it works against the theme of keeping things simple; I would prefer not to directly expose any of the lower-level API. But there are also hidden traps: some flag combinations can be dangerous or lead to unexpected consequences. For example:

  • MDB_WRITEMAP is incompatible with nested transactions, and all processes operating on the same environment must agree on whether this flag is used to avoid defeating durability.
  • MDB_FIXEDMAP can only be specified when initially creating an environment, and is stored persistently with it. It is also considered experimental and may not always work.
  • MDB_NOMEMINIT is likely redundant and unnecessary given the way the library is written to use MDB_RESERVE.
  • It doesn’t make a lot of sense to expose MDB_NOSUBDIR or MDB_RDONLY since the library already handles these internally; using MDB_RDONLY to create a ReadWrite environment can only lead to confusion.
  • MDB_NOLOCK means the user becomes responsible for all concurrency considerations.
  • MDB_NOSYNC can lead to database corruption upon a system crash.

I think I would prefer to provide a safer API for any of the flags that may actually be useful. For example, the MDB_NOMETASYNC and MDB_NOSYNC flags can be changed after the environment is opened, but to be most useful a function to call mdb_env_sync (and/or mdb_env_sync_flush) should also be provided.

Can you tell me which flags in particular you think are useful to modulate?

@mihaigiurgeanu
Copy link
Author

Well, I really needed to use the MDB_NOSYNC flag for performance reasons, so I made this quick hack. For me it is not important if the database would become invalid in the case of a system crush, because the database can be restored, but it is important to have high speed for database writes. It is also a feature I am using in a prototype, maybe it will not be used in the production version, but for this prototype it is crucial for me to get the performance given by the MDB_NOSYNC flag.

The idea is that, in the rare occasions when you need one of these flags, if this library does not allow you to use them you would have the option to use the lower level api. But you would miss all the nice things in this library.

Would you consider making this function part of a module named with unsafe, like Database.LMDB.Simple.Unsafe? Or to include the unsafe word in the function name, for example openEnvironmentUnsafe and to document it in a special Unsafe functions section? Or to have only the flags that make sense exposed?

@verement
Copy link
Owner

I’m thinking perhaps there could be an API like this:

delaySync     :: Environment ReadWrite  IO a  IO a
delayMetaSync :: Environment ReadWrite  IO a  IO a

These would each set the MDB_NOSYNC or MDB_NOMETASYNC flags, respectively, in the environment for the duration of the IO action, then call mdb_env_sync_flush before returning.

Do you think this is a good idea and would it satisfy your needs?

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