Permalink
Browse files

Merge "Make release lock more granular in UpdateDeployment job."

  • Loading branch information...
Vadim Spivak Gerrit Code Review
Vadim Spivak authored and Gerrit Code Review committed Nov 13, 2012
2 parents 98b0563 + e4970ce commit df79707685367c1c097ae00a68268d97775c6654
@@ -5,6 +5,7 @@ module Bosh::Director
# about existing deployment and information from director DB
class DeploymentPlanCompiler
include DnsHelper
+ include LockHelper
include IpUtil
# @param [DeploymentPlan] deployment_plan Deployment plan
@@ -25,8 +26,10 @@ def bind_deployment
# Binds release DB record(s) to a plan
# @return [void]
def bind_releases
- @deployment_plan.releases.each do |release|
- release.bind_model
+ with_release_locks(@deployment_plan) do
+ @deployment_plan.releases.each do |release|
+ release.bind_model
+ end
end
end
@@ -187,15 +187,15 @@ def update_stemcell_references
def perform
with_deployment_lock(@deployment_plan) do
- with_release_locks(@deployment_plan) do
- logger.info("Preparing deployment")
- prepare
- begin
- deployment = @deployment_plan.model
- logger.info("Finished preparing deployment")
- logger.info("Updating deployment")
- update
-
+ logger.info("Preparing deployment")
+ prepare
+ begin
+ deployment = @deployment_plan.model
+ logger.info("Finished preparing deployment")
+ logger.info("Updating deployment")
+ update
+
+ with_release_locks(@deployment_plan) do
deployment.db.transaction do
deployment.remove_all_release_versions
# Now we know that deployment has succeeded and can remove
@@ -205,14 +205,14 @@ def perform
deployment.add_release_version(release.model)
end
end
-
- deployment.manifest = @manifest
- deployment.save
- logger.info("Finished updating deployment")
- "/deployments/#{deployment.name}"
- ensure
- update_stemcell_references
end
+
+ deployment.manifest = @manifest
+ deployment.save
+ logger.info("Finished updating deployment")
+ "/deployments/#{deployment.name}"
+ ensure
+ update_stemcell_references
end
end
ensure
@@ -1,5 +1,13 @@
module Bosh::Director
+ # Helper for managing BOSH locks.
module LockHelper
+
+ # Surround with deployment lock.
+ #
+ # @param [DeploymentPlan|String] deployment plan or name.
+ # @option opts [Number] timeout how long to wait before giving up
+ # @return [void]
+ # @yield [void] block to surround
def with_deployment_lock(deployment, opts = {})
if deployment.respond_to?(:name)
name = deployment.name
@@ -13,19 +21,38 @@ def with_deployment_lock(deployment, opts = {})
Lock.new("lock:deployment:#{name}", :timeout => timeout).lock { yield }
end
+ # Surround with stemcell lock.
+ #
+ # @param [String] name stemcell name.
+ # @param [String] version stemcell version.
+ # @option opts [Number] timeout how long to wait before giving up
+ # @return [void]
+ # @yield [void] block to surround
def with_stemcell_lock(name, version, opts = {})
timeout = opts[:timeout] || 10
Config.logger.info("Acquiring deployment lock on #{name}:#{version}")
Lock.new("lock:stemcells:#{name}:#{version}", :timeout => timeout).
lock { yield }
end
+ # Surround with release lock.
+ #
+ # @param [String] release name.
+ # @option opts [Number] timeout how long to wait before giving up
+ # @return [void]
+ # @yield [void] block to surround
def with_release_lock(release, opts = {})
timeout = opts[:timeout] || 10
Config.logger.info("Acquiring deployment lock on #{release}")
Lock.new("lock:release:#{release}", :timeout => timeout).lock { yield }
end
+ # Surround with deployment releases lock.
+ #
+ # @param [DeploymentPlan] deployment plan.
+ # @option opts [Number] timeout how long to wait before giving up
+ # @return [void]
+ # @yield [void] block to surround
def with_release_locks(deployment_plan, opts = {})
timeout = opts[:timeout] || 10
release_names = deployment_plan.releases.map do |release|
@@ -45,5 +72,21 @@ def with_release_locks(deployment_plan, opts = {})
locks.reverse_each { |lock| lock.release }
end
end
+
+ # Surround with compile lock.
+ #
+ # @param [String|Number] package_id package id.
+ # @param [String|Number] stemcell_id stemcell id.
+ # @option opts [Number] timeout how long to wait before giving up
+ # @return [void]
+ # @yield [void] block to surround
+ def with_compile_lock(package_id, stemcell_id, opts = {})
+ timeout = opts[:timeout] || 15 * 60 # 15 minutes
+
+ Config.logger.info("Acquiring compile lock on " +
+ "#{package_id} #{stemcell_id}")
+ Lock.new("lock:compile:#{package_id}:#{stemcell_id}",
+ :timeout => timeout).lock { yield }
+ end
end
-end
+end
@@ -2,6 +2,7 @@
module Bosh::Director
class PackageCompiler
+ include LockHelper
# TODO Support nested dependencies
# TODO Decouple tsort from the actual compilation
@@ -215,31 +216,37 @@ def compile_package(task)
package = task.package
stemcell = task.stemcell
- build = generate_build_number(package, stemcell)
- agent_task = nil
-
- prepare_vm(stemcell) do |vm_data|
- agent_task =
- vm_data.agent.compile_package(package.blobstore_id,
- package.sha1, package.name,
- "#{package.version}.#{build}",
- task.dependency_spec)
- end
+ with_compile_lock(package.id, stemcell.id) do
+ # Check if the package was compiled in a parallel deployment
+ compiled_package = find_compiled_package(task)
+ if compiled_package.nil?
+ build = generate_build_number(package, stemcell)
+ agent_task = nil
+
+ prepare_vm(stemcell) do |vm_data|
+ agent_task =
+ vm_data.agent.compile_package(package.blobstore_id,
+ package.sha1, package.name,
+ "#{package.version}.#{build}",
+ task.dependency_spec)
+ end
- task_result = agent_task["result"]
+ task_result = agent_task["result"]
- compiled_package = Models::CompiledPackage.create do |p|
- p.package = package
- p.stemcell = stemcell
- p.sha1 = task_result["sha1"]
- p.build = build
- p.blobstore_id = task_result["blobstore_id"]
- p.dependency_key = task.dependency_key
- end
+ compiled_package = Models::CompiledPackage.create do |p|
+ p.package = package
+ p.stemcell = stemcell
+ p.sha1 = task_result["sha1"]
+ p.build = build
+ p.blobstore_id = task_result["blobstore_id"]
+ p.dependency_key = task.dependency_key
+ end
- @counter_mutex.synchronize { @compilations_performed += 1 }
+ @counter_mutex.synchronize { @compilations_performed += 1 }
+ end
- task.use_compiled_package(compiled_package)
+ task.use_compiled_package(compiled_package)
+ end
end
def enqueue_unblocked_tasks(task)
@@ -31,6 +31,7 @@
r1.should_receive(:bind_model)
r2.should_receive(:bind_model)
+ compiler.should_receive(:with_release_locks).and_yield
compiler.bind_releases
end
@@ -187,10 +187,10 @@
job = Bosh::Director::Jobs::UpdateDeployment.new(@manifest_file.path)
job.should_receive(:with_deployment_lock).with(@deployment_plan).
and_yield.ordered
- job.should_receive(:with_release_locks).with(@deployment_plan).
- and_yield.ordered
job.should_receive(:prepare).ordered
job.should_receive(:update).ordered
+ job.should_receive(:with_release_locks).with(@deployment_plan).
+ and_yield.ordered
job.should_receive(:update_stemcell_references).ordered
deployment.should_receive(:add_release_version).
@@ -108,4 +108,18 @@ class TestClass
end
end
-end
+ describe :with_compile_lock do
+ it "should support a package and stemcell id" do
+ lock = mock(:lock)
+ BD::Lock.stub!(:new).with("lock:compile:3:4", {:timeout=>900}).
+ and_return(lock)
+ lock.should_receive(:lock).and_yield
+
+ called = false
+ @test_instance.with_compile_lock(3, 4) do
+ called = true
+ end
+ called.should be_true
+ end
+ end
+end
@@ -205,6 +205,16 @@ def prepare_samples
end
end
+ @package_set_a.each do |package|
+ compiler.should_receive(:with_compile_lock).
+ with(package.id, @stemcell_a.model.id).and_yield
+ end
+
+ @package_set_b.each do |package|
+ compiler.should_receive(:with_compile_lock).
+ with(package.id, @stemcell_b.model.id).and_yield
+ end
+
@j_dea.should_receive(:use_compiled_package).exactly(6).times
@j_router.should_receive(:use_compiled_package).exactly(5).times
@@ -296,6 +306,12 @@ def prepare_samples
@director_job.should_receive(:task_checkpoint).once
compiler = make(@plan)
+
+ @package_set_a.each do |package|
+ compiler.should_receive(:with_compile_lock).
+ with(package.id, @stemcell_a.model.id).and_yield
+ end
+
compiler.compile
compiler.compilations_performed.should == 6
@@ -304,4 +320,29 @@ def prepare_samples
end
end
+ it "should make sure a parallel deployment did not compile a " +
+ "package already" do
+
+ package = BDM::Package.make
+ stemcell = BDM::Stemcell.make
+
+ task = BD::CompileTask.new(package, stemcell)
+ task.dependency_key = "[]"
+
+ compiler = make(@plan)
+ callback = nil
+ compiler.should_receive(:with_compile_lock).
+ with(package.id, stemcell.id) do |&block|
+ callback = block
+ end
+ compiler.compile_package(task)
+
+ compiled_package = BDM::CompiledPackage.make(
+ :package => package, :stemcell => stemcell, :dependency_key => "[]")
+
+ callback.call
+
+ task.compiled_package.should == compiled_package
+ end
+
end

0 comments on commit df79707

Please sign in to comment.