replace Padrino::Cache::Store with Moneta

[minad]

As an alternative to #1017 I replaced Padrino::Cache::Store completely with Moneta.

[skade]

See #1017 (comment) in #1017 for my opinion on this. I see more hard third-party dependencies as problematic. Vendoring might be another way to go.

Still, I don't want to block this change - Moneta is a solution fitting the problem and allows us to deprecate some of our stores in exchange for depending on another third-party. Given that we are the "rack lovers" framework, its integration into Rack is very nice.

What I do want to block: the change is needlessly backwards-incompatible (fine for a discussion). If accepted, an API change with deprecation notice is necessary. But this is not alot of work.

@padrino/ecore-members , especially those who wrote Cache and use it alot: is the gained flexibily and ease of development worth throwing out Cache completely? And how do we allow users to plugin in something different?

[minad]

@skade: I agree, this fits well.

Backward incompatibility: What is missing to make it compatible? As I see it the only thing missing is the constructors Padrino::Cache::Store::Memory.new etc. The #get, #set and #flush methods are provided by the LegacyStore mixin.

Plugging in something different: This is really easy. You only have to write something with a Moneta compatible interface. I also try to keep Moneta very up-to-date, this means that it should support virtually all available key/value stores. So I think the case that someone needs something else would seldomly come up, but then I would be happy to merge the adapter into Moneta.

[minad]

And towards your argument against third-party-code - If someone of you @padrino/core-members wants to contribute to Moneta, I will happily provide commit access. The only problem remaining is that the releases are not synchronized.

[dariocravero]

I use cache quite a lot and haven't found any big issues with the way Padrino is managing that so far. Having say that, I reckon that using a gem like Moneta, which is highly specialised on the topic perhaps makes more sense. At the end of the day it will allow us to grow our cache-stores offer without the overhead of having to implement them. That's a big plus to me.

To sum up, I'm fine with this implementation which still keeps the helpers but replaces the underlaying caching implementation.

@DAddYE @nesquena?

[nesquena]

On the one hand, Moneta does remind me of rack and tilt and in cases of clean, well-scoped standard libraries there can be a big advantage to using those or wrapping them instead of reinventing. On the other, as @skade mentioned it does introduce a hard dependency and another place users have to go to read documentation (altho we can mitigate some of that).

Looking at the code, I also would want to make sure we don't make things needlessly backwards incompatible. From the commit it looks like all the helpers and cache interface would stay the same (with some deprecation notices) and the only serious backwards incompatible change is when you set the cache adapter (Padrino::Cache::Store::Memcache.new becomes Moneta.new)?

What happens when someone upgrades their application and they haven't changed the caching adapter? I guess they would be greeted with the error that the class doesn't exist. Do we think this is fine for a major (0.11.0) release?

I am often glad to be able to remove code to maintain as part of Padrino especially because when moneta itself has no additional hard dependencies. It does seem to cleanup a lot of code that we no longer have to maintain. I think it's a positive change. Thoughts? /cc @achiu @DAddYE

[minad]

Are you already working on 1.0 or do you want to keep this open until then?

If you want I could also make it completly backwards compatible with some minor effort (adding the missing classes/fake constructors).

[nesquena]

We actually want to put this in possibly pre-1.0 but after the very next release which we want to cut soon. Leave this open for now, and we will soon merge in post 0.11.0 (0.12.0? @DAddYE). Thanks

[minad]

Ok, cool 👍

[skade]

This sounds like a good direction.

[DAddYE]

We should discuss a bit more about that, @ujifgc and @skade can we check how much ram we need for that? Performances?

Our goal for next version is keep things simple, clean and concise.

Using an external gem our code definitely will look clear but people to understand what we are doing should browse moneta sources which actually is great but are a huge amount of files ... 😸

So instead, I propose:

• Keep ours padrino-cache
• Simplify wherever we can it
• Add an api (@minad can do that) for a super simple interface to moneta.

Make sense? Thoughts ?

[ujifgc]

moneta uses ruby autoload to require it's backends, so, it's very tidy and uses no excess memory over padrino-cache.

[DAddYE]

yep but autoload is not threadsafe and dead in ruby 2.0 :(

[minad]

@DAddYE

Your proposol: Well, then you could just take the other pull request.

About your criticism of Moneta:

• Moneta uses autoload yes, but this can be changed as soon as ruby 3.0 is there. Ruby 2.0 will still support it. And when it is really removed I will just implement autoloading manually or use a pattern that other libraries use in the same case.
• Performance/Memory: Well I cannot say much about that, but if you only load moneta only a few constants will be created. The performance of the key/value stores is ensured by continous benchmarking on travis-ci. The moneta adapters itself are very thin and have an implementation similar to yours.
• Moneta has many classes. This is true, but most of the classes are just adapters for the different backends. Moneta is also very feature rich, supports configurable value compression and serialization. This is all hidden behind the simple Moneta.new interface, but a power user could access all these niceties. I would also say that the design is quite clean since a lot of things are handled in different layers, similar to rack middlewares.

Moneta does also have a very extensive test and benchmarking suite. So this pull request is not only about reducing code, but also about ensuring code quality, adding many backends and other powerful features, ...

Maybe some words about my motiviation for this request: I would really like projects to profit from each other. I think this could be the case here. And as I said I am open to any suggestions and you can also contribute at the Moneta project (commit rights etc) if you want to.

So think about it :)

[skade]

I think the big advantage of using moneta is one of seperation of concerns. autoload is problematic (and for that reason, Rails just removed it, as it has bitten them), but when we adopt moneta, thats monetas problem :). Then again, my main complaint here is overreliance on constant names:

A fix for this problem would be not to rely on the constants directly on the Padrino side and let moneta handle that. E.g.:

set :cache, :adapter => :redis, :db => 1 #....
set :cache_instance, Proc.new { @instance ||= Moneta.new(cache.delete(:adapter), cache) } #maybe simplify that?

And a similar interface on the Moneta side. This allows Moneta to load the constant on first access without having to rely on magically appearing constants. Such interfaces are generally preferable, as they allow lazy loading in a much more decoupled manner.

About Memory footprint: we should bench that. I think the moneta project is also interested in getting better in that regard if possible.

[minad]

I don't understand what you mean my overreliance on constant names (:Redis vs :redis?). If you implement autoloading manually (not relying on"magically appearing constants", you have to deal with the same thread-safety problems. If you have suggestion on how to improve the moneta api, don't hesitate to create an issue.

But autoloading or not is really Monetas problem and your Store uses autoload too. Therefore this whole autoload discussion here doesn't make much sense, but the concern is noted.

Thread safety for autoloading the cache stores is not a big problem since it is done at startup (similar to new rails eager autoloading)

[skade]

@minad I think there has been a misunderstanding: I meant the current API, which requires users to directly access constants, which doesn't allow for changing the loading in the back. Thats why I'd be wary with using Moneta.new directly, as it means that you cannot break API there.

About autoload in Moneta itself: I really don't care what color the bikeshed is, as long as its not mine and it harbors bikes :).

Concerning loading at bootstrap time: yes, thats precisely what I prefer and how its implemented is at the discretion of the library in use.

[minad]

@skade There are two api entry points to create moneta stores, but one could overwrite the proc :cache_instance in your suggestion.

Moneta.new(:Memory, :expires => true)

Moneta.build do
  use :Expires
  adapter :Memory
end

the second one is to add special middlewares.

[minad]

I added a memusage script to Moneta: https://github.com/minad/moneta/blob/master/script/memusage

Example output on 1.9.3:

# 29772K +29772K
require 'moneta'
# 29772K +0K
Moneta.new(:Memory)
# 30828K +1056K
Moneta.new(:File, :dir => 'filestore')
# 34940K +4112K
Moneta.new(:MemcachedNative)
# 68616K +33676K
Moneta.new(:MemcachedDalli)
# 84580K +15964K

[ujifgc]

@minad what platform is it? 32 or 64 bit?

[minad]

linux 64 bit

[minad]

@ujifgc You might want to test it with padrino loaded and see if it makes a difference.

[minad]

Will this go in at some point?

[nesquena]

I would definitely like it to, I think it will make the codebase for cache much leaner and easier to maintain by relying on moneta and not reinventing the wheel. The only question is of when in my mind. It is going to be a fairly breaking change, so I would say at this point on the next major point release i.e 0.12.0. Thoughts @padrino/core-members ?

[dariocravero]

👍!

[skade]

Next Major was also what I thought of.

Am 09.04.2013 um 23:52 schrieb Nathan Esquenazi notifications@github.com:

I would definitely like it to, I think it will make the codebase for cache much leaner and easier to maintain by relying on moneta and not reinventing the wheel. The only question is of when in my mind. It is going to be a fairly breaking change, so I would say at this point on the next major point release i.e 0.12.0. Thoughts @padrino/core-members ?

—
Reply to this email directly or view it on GitHub.

[minad]

I rebased the pull request on your master and added a few improvements. As discussed before the only difference to your implementation is the creation of the stores. Padrino::Cache::Store::Memory.new(50) is now Padrino::Cache.new(:Memory). You can also use Moneta.new(:Memory) directly but then you don't get the legacy interface.

I have seen that you had problems with encoding and marshalling (#821, #1128). The key/value transformation feature is also provided by Moneta directly.

[nesquena]

This is great, thanks for rebasing. Will be a part of 0.12

[dariocravero]

👍

[minad]

Compare https://github.com/minad/padrino-framework/tree/moneta-without-deprecated and https://github.com/minad/padrino-framework/tree/moneta-replace to check the backward compatibility layer.