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

Moneta#new doesn't support thread-safe YAML::Store via positional arguments to the backend #231

Open
todd-a-jacobs opened this issue Jul 19, 2022 · 2 comments

Comments

@todd-a-jacobs
Copy link

Given the following initialization with the documented :threadsafe keyword set to true doesn't yield a YAML::Store backend where @thread_safe=true. If I were calling YAML::Store#new directly, I would invoke:

require "yaml/store"

YAML::Store.new "store.yml", true

In Moneta, I can't seem to do the equivalent with either Moneta#new or Moneta::Adapters::YAML#new. With Moneta#new, I get the following, where the backend shows @thread_safe=false. Perhaps Moneta::Lock is handling the thread-safety on behalf of YAML::Store, but this is an assumption rather than something I was able to find documented. Consider the following:

m = Moneta.new :YAML, file: "store.yml", threadsafe: true, value_serializer: :yaml
m
#=> 
#<Moneta::Transformer::MarshalPrefixKeyYamlValue:0x00000001100d3520                           
 @adapter=                     
  #<Moneta::Lock:0x00000001100d8430
   @adapter=                   
    #<Moneta::Adapters::YAML:0x00000001100d9010
     @backend=
      #<Psych::Store:0x00000001100d8a48
       @abort=false,
       @filename="store.yml",
       @lock=#<Thread::Mutex:0x00000001100d87f0>,
       @opt={},
       @thread_safe=false,
       @ultra_safe=false>,
     @config=nil,
     @id="Moneta::Adapters::PStore(312140)">,
   @config=nil,
   @lock=#<Thread::Mutex:0x00000001100d3f98>>,
 @config=nil,
 @prefix="">

This seems to be fixable on the fly with:

m.adapter.adapter.backend.instance_variable_set :@thread_safe, true
m
#=> 
#<Moneta::Transformer::MarshalPrefixKeyYamlValue:0x00000001100d3520                                                   
 @adapter=                                                                                                            
  #<Moneta::Lock:0x00000001100d8430                                                                                   
   @adapter=                                                                                                          
    #<Moneta::Adapters::YAML:0x00000001100d9010                                                                       
     @backend=                                                                                                        
      #<Psych::Store:0x00000001100d8a48                                                                               
       @abort=false,                                                                                                  
       @filename="store.yml",                                                                                         
       @lock=#<Thread::Mutex:0x00000001100d87f0>,                                                                     
       @opt={},                                                                                                       
       @thread_safe=true,                                                                                             
       @ultra_safe=false>,                                                                                   
     @config=nil,                                                                                            
     @id="Moneta::Adapters::PStore(312140)">,                                                                
   @config=nil,                                                                                              
   @lock=#<Thread::Mutex:0x00000001100d3f98>>,                                                               
 @config=nil,
 @prefix="">

However, unless Moneta::Lock is handling the thread-safety on behalf of YAML::Store (which doesn't seen to be explicit in the documentation for Moneta or the YAML adapter) it seems as if the standard constructor doesn't have a way to pass other positional arguments to the :YAML backend. This may simply be a documentation issue, or possibly a bug. Either way, it should probably be more obvious about what arguments a given adapter can accept, or how (or even if) YAML::Store and other adapters can receive positional arguments during initialization.

@asppsa
Copy link
Collaborator

asppsa commented Jul 24, 2022

Hi,

Most adapters allow you to pass a backend option if you need to configure the underlying store - so you can do this:

store = Moneta.new(:YAML, { backend: YAML::Store.new('store.yml', true) })
store['x'] = 1 # etc

This seems to work for now - I'll keep this ticket open though and we can make this an explicit option too.

@asppsa
Copy link
Collaborator

asppsa commented Mar 13, 2023

Hi again, with the 1.6.0 release you can now do:

store = Moneta.build do
  adapter :YAML, file: 'store.yml', threadsafe: true
end

That initialises a store that doesn't use mutex locking, but does initialise the YAML backend with thread-safety.

As you said in your OP, the threadsafe option passed to Moneta.new does indeed insert a Moneta::Lock into the store - this is a general option (not particular to YAML).

Docs for both features:

If you wanted both the lock and the adapter to be threadsafe, you could do:

store = Moneta.build do
  use :Lock
  adapter :YAML, file: 'store.yml', threadsafe: true
end

... though I believe this won't be necessary.

The longer-term option for supporting this better through Moneta.new is probably to make thread-safety a feature that adapters can advertise, and to only use the lock middleware if its not a feature of the adapter (similar to how we do things like expiry). I'll keep this ticket open while we consider that possibility.

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