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

Support env_stat #97

Closed
Keats opened this issue Jan 25, 2021 · 5 comments
Closed

Support env_stat #97

Keats opened this issue Jan 25, 2021 · 5 comments

Comments

@Keats
Copy link

Keats commented Jan 25, 2021

Taken from the python bindings docs for LMDB: https://lmdb.readthedocs.io/en/release/#lmdb.Environment.stat

Would it be possible to expose that function?

@Kerollmops
Copy link
Member

Kerollmops commented Jan 25, 2021

Hey @Keats,

I believe it is easy to add two new methods:

  • The Env::stat method that just calls mdb_env_stat with the env pointer.
  • The Database::stat method that calls the mdb_env_stat with the database integer.

Do be able to use these mdb_env_stat and mdb_stat functions, we must make sure we export them from the lmdb_ffi and mdbx_ffi modules first.

However, I would prefer that we return two different structs for the two methods, I don't find the Mdb_val struct clear enough, what do you think. And two kill two birds with one stone, we could fix #56 by using these stats.

@Keats
Copy link
Author

Keats commented Jan 25, 2021

I don't find the Mdb_val struct clear enough

What would you change?

The main thing I wanted the DB stat struct for is the number of entries to (so it's not slow like right now, solving #56 as you point out) and to calculate the space used (https://wiki.samba.org/index.php/Using_the_lmdb_database_backend#Calculating_space_used). I don't think I would need the env stat struct myself but it could definitely be added for the sake of completeness.

@Kerollmops
Copy link
Member

Kerollmops commented Jan 26, 2021

What would you change?

First thing I don't quite like is the documentation and those other fields that are not that obvious.

I don't think I would need the env stat struct myself but it could definitely be added for the sake of completeness.

Maybe you could first fix #56 by extracting the number of entries from the Mdb_stat struct, this way you don't expose it, then we will introduce the new methods which return two new structs with explicit fields and maybe one that contains the space used?

I will create a 0.10 branch to be able to update the currently published crate version as I am currently doing a lot of changes for a 0.11 version right now. But don't worry I will ask you to change the base branch directly on GitHub if you publish a PR on master.

@Keats
Copy link
Author

Keats commented Jan 28, 2021

PR for #56: #99

The rest is not that important to me so if you're doing a lot of changes, I'll do another PR with the db stat structs later once you're done (unless you beat me to it).

@Kerollmops
Copy link
Member

Closed by #100.

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