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: Better Backend Support #528

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

Conversation

arthurschreiber
Copy link
Member

Initial backend support was added by @charypar as part of #410.

Unfortunately, the implementation is a bit convoluted: Every backend implementation needs to include <rugged.h> to get the definition of rugged_backend. Also, the way backends are implemented right now, they always need to provide a refdb and odb backend together, there's no way to use e.g. one backend type for the odb and another backend type for the refdb. Also, config backends were missing completely.

This PR is aimed at fixing those shortcomings and make backend support first class.

It introduces a few new classes that wrap existing functionality from libgit2, provides abstract implementations of custom refdb, odb and config backends and exposes new APIs on the Repository object to make it easier to initialize repositories with custom backends.


  • Add Rugged::Refdb to wrap git_refdb functionality from libgit2.
  • Add Rugged::Refdb::Backend::FileSystem to expose git_refdb_backend_fs and to serve as an example on how to wrap git_refdb_backend.
  • Add Rugged::Refdb::Backend::Custom as an abstract base class that can be extended to provide new refdb backends that are defined using Ruby code.
  • Add Rugged::Odb to wrap git_odb functionality from libgit2.
  • Add Rugged::Odb::Backend::Loose, Rugged::Odb::Backend::Packed and Rugged::Odb::Backend::Single to expose git_odb_backend_loose, git_odb_backend_packed and git_odb_backend_single and to serve as examples on how to wrap git_odb_backend.
  • Add Rugged::Odb::Backend::Custom as an abstract base class that can be extended to provide new odb backends that are defined using Ruby code.
  • Add Rugged::Config::Backend::Custom as an abstract base class that can be extended to provide new config backends that are defined using Ruby code.

@arthurschreiber
Copy link
Member Author

@tpickett66 would you mind taking a look? This is obviously still WIP, but I think this should support all the your use cases once ready.

@tpickett66
Copy link
Contributor

Wow, I wasn't expecting such a big response on this idea. I'm excited to take a look first thing in the morning!

@tpickett66
Copy link
Contributor

Ok, I lied; I was too excited to wait. I have to say this is almost exactly what I was wanting to build when I started but at the time wasn't sure such a major rework on the backend system would be looked kindly upon by someone who just popped up. How can I help round this work out? I'd be happy to start filling in some of the various C -> Ruby bridge methods or build a reference backend with documentation or really any other task that would be useful.

@arthurschreiber
Copy link
Member Author

Hey @tpickett66

Glad to hear this is shaping out to do exactly what you were missing. I'll work on filling in most of the missing pieces here and there over the course of the week.

Existing native backends like rugged-redis would need to be upgraded to be compatible with this new API, and having an example custom backend with documentation would also be highly appreciated.

One thing that I wanted to let you know, and we also should mention in the docs for alternative backends (mostly for odb backends) is that they all have pretty abysmal performance when compared with the default filesystem based backends. This is mostly due to the amount of operations that need to happen when reading and writing objects into the odb.

@tpickett66
Copy link
Contributor

@arthurschreiber Thanks for the heads up on the poor performance on ODB backends, after digging into how the ODB works it totally makes sense that performance would be terrible for non-fs backends. Since this project is an experiment it won't be the end of the world if performance is terrible but hopefully we can get it to a level that's acceptable for the interactions we're needing.

@charypar
Copy link
Contributor

charypar commented Oct 5, 2015

Great to see this being extended! Looks good.

@arthurschreiber How do you plan to implement the support for backends implemented in ruby? I believe that support was originally included in Rugged (I can try and dig out an old commit I found last time I looked), but was later removed for performance reasons – there was too much context switching (too many indirections) between ruby and C on every read and write to make backends written in Ruby practical (e.g. for Redis, it would be something like Client ruby code -> Rugged -> C rugged -> libgit -> abstract backend -> rugged backend -> ruby backend implementation -> ruby redis client -> redis native lib).

That's also the reason why I've done the backends the way they are implemented, because they only get injected from Ruby code, but the reads and writes are all performed in native land as if you were using libgit and the backend library directly.

Did you come up with a better solution for this?

@arthurschreiber
Copy link
Member Author

@charypar I'll factor out the Ruby backend implementations into a separate gem and add a big scary warning that'll say "DANGERZONE - DO NOT USE IN PRODUCTION".

On another note, how did you even use the redis backend? I wanted to open a PR against your repo to switch it over to the new backend API and ran into the issue that the sharedConnection functionality does not work in the slightest and will cause freed connections to be reused and whatnot.

@charypar
Copy link
Contributor

charypar commented Oct 7, 2015

@arthurschreiber That's a bit strange! we used it in [https://github.com/bskyb/colonel] as a storage backend. The sharedConnection was meant to prevent opening too many Redis connections as RedisCloud which we used charges for connections and there was no need for each backend to have its own. It should basically open one connection and them use it for the duration of the process. What were the problems you ran into with it?

As an aside, I personally haven't used rugged-redis or the Colonel in about half a year, maybe @jonsharratt or @JoeStanton could provide some more recent insights?

@arthurschreiber
Copy link
Member Author

The way the shared connection thing is implemented right now can not really work with multiple backend instances. If you have two redis backends open, and close one, this will cause the connection to be freed and the second backend will become unusable.

Also, the shared connection thing prevents two backends using two different redis hosts.

@charypar
Copy link
Contributor

charypar commented Oct 7, 2015

@arthurschreiber I think the reason it doesn't support it is we never needed multiple backend instances at the same time. I couldn't think of a case when you'd need it and what the situation would be, so I skipped it (would be interested in hearing about some).

If you can get it not to waste connections inside one backend instance and still allow multiple to exist, great, I'm all for it. 👍

@arthurschreiber
Copy link
Member Author

I pulled out the code that allows defining backends in ruby into a separate repository, rugged-ruby. That will serve as a good example of how new backends can be exposed to rugged.

@arthurschreiber arthurschreiber force-pushed the arthur/better-backend-support branch from de4191e to c937735 Compare October 13, 2015 20:35
@charypar
Copy link
Contributor

@arthurschreiber do you have any sort of idea of how much defining them in ruby affects performance yet? Just out of interest

@arthurschreiber
Copy link
Member Author

@charypar No idea, except that it'll probably be pretty bad. I can run some benchmarks against the "native" rugged-redis once everything is ready here.

@charypar
Copy link
Contributor

@arthurschreiber awesome, will keep an eye out for those 👍

@arthurschreiber arthurschreiber force-pushed the arthur/better-backend-support branch from 4ec3211 to 248eae9 Compare October 22, 2015 21:06
@arthurschreiber
Copy link
Member Author

@tpickett66 Did you have any change to take a look over https://github.com/arthurschreiber/rugged-ruby? I started implementing a refdb and config db backend, and would appreciate any contributions.

@charypar Is there anyone maintaining rugged-redis?

@thedanielhanke
Copy link

hi, any news on this?

@ioquatix
Copy link
Contributor

I'd really like to see this implemented too.

@ioquatix
Copy link
Contributor

Any chance we can update and/or merge this? I'd also like to see support for the in-memory backend, which would be super useful for testing.

@carlosmn how could I help get this over the finish line?

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.

5 participants