Remove incomplete reload/keyed reconciliation. (#73)

Author: samuel-williams-shopify

lib/async/container/controller.rb

@@ -194,32 +194,6 @@ def restart
 				end
 			end
 
-			# Reload the existing container. Children instances will be reloaded using `SIGHUP`.
-			def reload
-				@notify&.reloading!
-				
-				Console.info(self){"Reloading container: #{@container}..."}
-				
-				begin
-					self.setup(@container)
-				rescue
-					raise SetupError, container
-				end
-				
-				# Wait for all child processes to enter the ready state.
-				Console.info(self, "Waiting for startup...")
-				@container.wait_until_ready
-				Console.info(self, "Finished startup.")
-				
-				if @container.failed?
-					@notify.error!("Container failed to reload!")
-					
-					raise SetupError, @container
-				else
-					@notify&.ready!(size: @container.size, status: "Running with #{@container.size} children.")
-				end
-			end
-			
 			# Enter the controller run loop, trapping `SIGINT` and `SIGTERM`.
 			def run
 				@notify&.status!("Initializing controller...")

lib/async/container/generic.rb

@@ -8,7 +8,6 @@
 require "async/clock"
 
 require_relative "group"
-require_relative "keyed"
 require_relative "statistics"
 require_relative "policy"
 
@@ -79,7 +78,7 @@ def to_s
 			# Look up a child process by key.
 			# A key could be a symbol, a file path, or something else which the child instance represents.
 			def [] key
-				@keyed[key]&.value
+				@keyed[key]
 			end
 
 			# Statistics relating to the behavior of children instances.
@@ -210,13 +209,13 @@ def stop(timeout = true)
 			# Spawn a child instance into the container.
 			# @parameter name [String] The name of the child instance.
 			# @parameter restart [Boolean] Whether to restart the child instance if it fails.
-			# @parameter key [Symbol] A key used for reloading child instances.
+			# @parameter key [Symbol] An optional key used to look up (via {[]}) and reuse the child instance.
 			# @parameter health_check_timeout [Numeric | Nil] The maximum time a child instance can run without updating its state, before it is terminated as unhealthy.
 			# @parameter startup_timeout [Numeric | Nil] The maximum time a child instance can run without becoming ready, before it is terminated as unhealthy.
 			def spawn(name: nil, restart: false, key: nil, health_check_timeout: nil, startup_timeout: nil, &block)
 				name ||= UNNAMED
 
-				if mark?(key)
+				if reuse?(key)
 					Console.debug(self, "Reusing existing child.", child: {key: key, name: name})
 					return false
 				end
@@ -326,32 +325,22 @@ def async(**options, &block)
 				end
 			end
 
-			# Reload the container's keyed instances.
+			# Re-run the given block against the container.
+			#
+			# Existing keyed children are reused (see {spawn}), so re-running setup will not
+			# duplicate them. Reconciliation of children whose keys are no longer configured
+			# (i.e. stopping obsolete children) is not currently supported and will be revisited.
 			def reload
-				@keyed.each_value(&:clear!)
-				
 				yield
-				
-				dirty = false
-				
-				@keyed.delete_if do |key, value|
-					value.stop? && (dirty = true)
-				end
-				
-				return dirty
 			end
 
-			# Mark the container's keyed instance which ensures that it won't be discarded.
-			def mark?(key)
+			# Whether a child instance already exists for the given key, in which case it can be reused rather than spawned again.
+			def reuse?(key)
 				if key
-					if value = @keyed[key]
-						value.mark!
-						
-						return true
-					end
+					@keyed.key?(key)
+				else
+					false
 				end
-				
-				return false
 			end
 
 			# Whether a child instance exists for the given key.
@@ -366,7 +355,7 @@ def key?(key)
 			# Register the child (value) as running.
 			def insert(key, child)
 				if key
-					@keyed[key] = Keyed.new(key, child)
+					@keyed[key] = child
 				end
 
 				state = {}

lib/async/container/keyed.rb

@@ -1,5 +1,9 @@
 # Releases
 
+## Unreleased
+
+  - **Removed** `Controller#reload` and the container-level keyed reconciliation (`Keyed`, mark/sweep, and stopping of obsolete keyed children on reload). This functionality was incomplete and not correctly wired, and will be revisited with a simpler design. Keyed `spawn(key:)` registration and `container[key]` lookup are retained, and `Container#reload` now simply re-runs the given block (reusing existing keyed children).
+
 ## v0.37.0
 
   - Rename `ASYNC_CONTAINER_GRACEFUL_TIMEOUT` to `ASYNC_CONTAINER_GRACEFUL_STOP` and apply it at the controller level as `GRACEFUL_STOP`. `Group#stop` now only applies the shutdown policy it is given.

releases.md

@@ -44,46 +44,6 @@ def read_graceful_stop(value)
 		end
 	end
 
-	with "#reload" do
-		it "can reuse keyed child" do
-			input, output = IO.pipe
-			
-			controller.instance_variable_set(:@output, output)
-			
-			def controller.setup(container)
-				container.spawn(key: "test") do |instance|
-					instance.ready!
-					
-					@output.write(".")
-					@output.flush
-					
-					sleep(0.2)
-				end
-				
-				container.spawn do |instance|
-					instance.ready!
-					
-					sleep(0.1)
-					
-					@output.write(",")
-					@output.flush
-				end
-			end
-			
-			controller.start
-			
-			expect(controller.state_string).to be == "running"
-			
-			expect(input.read(2)).to be == ".,"
-			
-			controller.reload
-			
-			expect(input.read(1)).to be == ","
-			
-			controller.wait
-		end
-	end
-	
 	with "#restart" do
 		it "replaces the running container with a new one" do
 			def controller.setup(container)
@@ -127,46 +87,6 @@ def controller.setup(container)
 		end
 	end
 
-	with "notify" do
-		before do
-			@notify_server = Async::Container::Notify::Server.open
-			@notify_client = Async::Container::Notify::Socket.new(@notify_server.path)
-			@notify = @notify_server.bind
-		end
-		
-		after do
-			@notify&.close
-		end
-		
-		let(:controller) {subject.new(notify: @notify_client)}
-		
-		it "sends status with ready notification on reload" do
-			def controller.setup(container)
-				container.spawn do |instance|
-					instance.ready!
-					sleep(0.1)
-				end
-			end
-			
-			controller.start
-			
-			# Drain the start ready message:
-			@notify.wait_until_ready
-			
-			controller.reload
-			
-			# Capture messages until we find the reload ready notification:
-			while message = @notify.receive
-				break if message[:ready]
-			end
-			
-			expect(message).to have_keys(
-				ready: be == true,
-				status: be =~ /Running/
-			)
-		end
-	end
-	
 	with "#start" do
 		it "can start up a container" do
 			expect(controller).to receive(:setup)