Review comments

Author: Michael Kipper

README.md

@@ -471,8 +471,10 @@ You should run a simulation with your workloads to determine an efficient
 scaling factor that will produce a time-to-open reduction but isn't too
 sensitive.
 
-To disable host-based circuits, set the environment variable
-`SEMIAN_CIRCUIT_BREAKER_IMPL` to `ruby`.
+The circuit breaker implementation is based on the environment variable
+`SEMIAN_CIRCUIT_BREAKER_IMPL`. Set it to `worker` to disable sharing circuit
+state and `host` to enable host-based circuits. The default is `worker` for
+backward compatibility.
 
 ### Bulkheading
 

ext/semian/semian.c

@@ -81,9 +81,9 @@ use_c_circuits() {
     fprintf(stderr, "Warning: Defaulting to worker-based circuit breaker implementation\n");
     return 0;
   } else {
-    if (!strcmp(circuit_impl, "ruby") || !strcmp(circuit_impl, "worker")) {
+    if (!strcmp(circuit_impl, "worker")) {
       return 0;
-    } else if (!strcmp(circuit_impl, "c") || !strcmp(circuit_impl, "host")) {
+    } else if (!strcmp(circuit_impl, "host")) {
       return 1;
     } else {
       fprintf(stderr, "Warning: Unknown circuit breaker implementation: '%s'\n", circuit_impl);

test/simple_integer_test.rb

@@ -43,20 +43,24 @@ def test_reset
       assert_equal(0, @integer.value)
     end
 
-    if ENV['SEMIAN_CIRCUIT_BREAKER_IMPL'] != 'ruby'
-      # Without locks, this only passes around 1 in every 5 runs
-      def test_increment_race
-        process_count = 255
-        100.times do
-          process_count.times do
-            fork do
-              value = @integer.increment(1)
-              exit!(value)
-            end
+    # Without locks, this only passes around 1 in every 5 runs
+    def test_increment_race
+      process_count = 255
+      100.times do
+        process_count.times do
+          fork do
+            value = @integer.increment(1)
+            exit!(value)
           end
-          exit_codes = Process.waitall.map { |_, status| status.exitstatus }
-          # No two processes should exit with the same exit code
+        end
+        exit_codes = Process.waitall.map { |_, status| status.exitstatus }
+
+        if ENV['SEMIAN_CIRCUIT_BREAKER_IMPL'] == 'host'
+          # Host-based circuits: No two processes should exit with the same exit code
           assert_equal(process_count, exit_codes.uniq.length)
+        else
+          # Worker-based circuits: All the processes should exit with the same code
+          assert_equal(1, exit_codes.uniq.length)
         end
       end
     end

test/simple_sliding_window_test.rb

@@ -52,7 +52,7 @@ def test_sliding_window_reject
   end
 
   def test_sliding_window_reject_failure
-    skip if ENV['SEMIAN_CIRCUIT_BREAKER_IMPL'] == 'ruby'
+    skip if ENV['SEMIAN_CIRCUIT_BREAKER_IMPL'] == 'worker'
     @sliding_window << 0 << 1 << 2 << 3 << 4 << 5 << 6 << 7
     assert_equal(6, @sliding_window.size)
     assert_sliding_window(@sliding_window, [2, 3, 4, 5, 6, 7], 6)