Break early and move around function definition:

shayonj · Jan 15, 2024 · cfd8b8e · cfd8b8e
1 parent fe993a4
commit cfd8b8e
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 26 deletions.
diff --git a/lib/pg_online_schema_change/orchestrate.rb b/lib/pg_online_schema_change/orchestrate.rb
@@ -88,7 +88,7 @@ def run!(options)
       def setup_signals!
         reader, writer = IO.pipe
 
-        ['TERM', 'QUIT', 'INT'].each { |sig| trap(sig) { writer.puts sig } }
+        %w[TERM QUIT INT].each { |sig| trap(sig) { writer.puts sig } }
 
         reader
       end
@@ -201,25 +201,6 @@ def run_alter_statement!
         Store.set(:renamed_columns_list, Query.renamed_columns(client))
       end
 
-      def log_progress
-        new_connection = client.checkout_connection
-        source_table_size = Query.get_table_size(new_connection, client.schema, client.table_name)
-
-        Thread.new do
-          loop do
-            sleep(TRACK_PROGRESS_INTERVAL) unless ENV["CI"]
-
-            shadow_table_size = Query.get_table_size(new_connection, client.schema, shadow_table)
-            progress = (shadow_table_size.to_f / source_table_size) * 100
-            logger.info("Estimated copy progress: #{progress.round(2)}% complete")
-
-            break if @copy_finished || progress >= 100
-          rescue StandardError => e
-            logger.info("Reporting progress failed: #{e.message}")
-          end
-        end
-      end
-
       def copy_data!
         # re-uses transaction with serializable
         # Begin the process to copy data into copy table
@@ -251,6 +232,26 @@ def copy_data!
         @copy_finished = true
       end
 
+      def log_progress
+        new_connection = client.checkout_connection
+        source_table_size = Query.get_table_size(new_connection, client.schema, client.table_name)
+
+        Thread.new do
+          loop do
+            break if @copy_finished
+
+            shadow_table_size = Query.get_table_size(new_connection, client.schema, shadow_table)
+            progress = (shadow_table_size.to_f / source_table_size) * 100
+            logger.info("Estimated copy progress: #{progress.round(2)}% complete")
+
+            break if @copy_finished || progress >= 100
+            sleep(TRACK_PROGRESS_INTERVAL) unless ENV["CI"]
+          rescue StandardError => e
+            logger.info("Reporting progress failed: #{e.message}")
+          end
+        end
+      end
+
       def replay_and_swap!
         Replay.begin!
       rescue CountBelowDelta

diff --git a/spec/lib/orchestrate_spec.rb b/spec/lib/orchestrate_spec.rb
@@ -1535,19 +1535,20 @@
         )
         allow(PgOnlineSchemaChange::Orchestrate).to receive(:logger).and_return(logger)
         allow(logger).to receive(:info)
-        allow(Thread).to receive(:new).and_yield
 
-        stub_const("PgOnlineSchemaChange::Orchestrate::TRACK_PROGRESS_INTERVAL", 0)
+        stub_const("PgOnlineSchemaChange::Orchestrate::TRACK_PROGRESS_INTERVAL", 0.1)
+        described_class.instance_variable_set(:@copy_finished, false)
       end
 
       it "logs the estimated copy progress" do
-        allow(client).to receive(:checkout_connection).and_return(client.connection)
+        expect(logger).to receive(:info).with(/Estimated copy progress: 39.2% complete/)
 
-        described_class.instance_variable_set(:@copy_finished, true)
+        thread = described_class.log_progress
 
-        expect(logger).to receive(:info).with(/Estimated copy progress: 39.2% complete/)
+        sleep(0.2)
 
-        described_class.log_progress
+        described_class.instance_variable_set(:@copy_finished, true)
+        thread.join
       end
     end
   end