Skip to content

Commit

Permalink
Minimum changes to avoid unnecessary DB lock on read
Browse files Browse the repository at this point in the history
With accompanying tests to prove it.
  • Loading branch information
atz committed Feb 23, 2017
1 parent fb7a478 commit 4c41497
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 7 deletions.
17 changes: 10 additions & 7 deletions lib/active_fedora/noid/minter/db.rb
Expand Up @@ -6,14 +6,14 @@ module Noid
module Minter
class Db < Base
def read
filtered_hash = instance.as_json.select { |key| %w(template counters seq rand namespace).include?(key) }
filtered_hash = instance.as_json.slice(*%w(template counters seq rand namespace))
filtered_hash['counters'] = JSON.parse(filtered_hash['counters'], symbolize_names: true) if filtered_hash['counters']
filtered_hash.symbolize_keys
end

def write!(minter)
# namespace and template are the same, now update the other attributes
instance.update_attributes!(
instance(true).update_attributes!(
seq: minter.seq,
counters: JSON.generate(minter.counters),
rand: Marshal.dump(minter.instance_variable_get(:@rand))
Expand All @@ -26,7 +26,7 @@ def write!(minter)
# Should be fast enough to avoid terrible deadlock.
# Must lock because of multi-connection context! (transaction is per connection -- not enough)
# The DB table will only ever have at most one row per namespace.
# The 'default' namespace row is inserted by `rails generate active_fedora:noid:seed`.
# The 'default' namespace row is inserted by `rails generate active_fedora:noid:seed` or autofilled by instance below.
# If you want another namespace, edit your config initialzer to something like:
# ActiveFedora::Noid.config.namespace = 'druid'
# ActiveFedora::Noid.config.template = '.reeedek'
Expand All @@ -43,10 +43,13 @@ def next_id
id
end

def instance
MinterState.lock.find_by!(
namespace: ActiveFedora::Noid.config.namespace,
template: ActiveFedora::Noid.config.template
# @param [Boolean] lock whether the returned object (row) should be locked for update or not
# @return [MinterState]
def instance(lock = false)
MinterState.find_by!(
{ namespace: ActiveFedora::Noid.config.namespace,
template: ActiveFedora::Noid.config.template },
lock: lock
)
rescue ActiveRecord::RecordNotFound
MinterState.seed!(
Expand Down
8 changes: 8 additions & 0 deletions spec/unit/db_minter_spec.rb
Expand Up @@ -43,6 +43,14 @@
subject { db_minter.read }

context 'when the database has been initialized' do
it 'does not lock the DB' do
expect(MinterState).not_to receive(:lock)
expect(MinterState).to receive(:find_by!).with(
anything,
hash_excluding(lock: true)
).and_call_original
subject # have to call it to trigger above
end
it 'has the expected namespace and template' do
expect(subject).to include(namespace: ActiveFedora::Noid.config.namespace,
template: ActiveFedora::Noid.config.template)
Expand Down

0 comments on commit 4c41497

Please sign in to comment.