]> git.openstreetmap.org Git - chef.git/commitdiff
Attempt to fix changeset replication lock queueing.
authorMatt Amos <zerebubuth@gmail.com>
Tue, 7 Jul 2020 18:42:31 +0000 (19:42 +0100)
committerMatt Amos <zerebubuth@gmail.com>
Tue, 7 Jul 2020 18:42:31 +0000 (19:42 +0100)
When the changeset replication runs slow, a lot of processes queue up trying to get the lock. I'm not totally sure, but it looks like they then get woken up in a random order, resulting in weird out-of-order behaviour.

This patch simplifies that process in two ways:

1. If the lock isn't acquired, the process exits. This means much less (perhaps zero) lock queueing.
2. Using a separate `lockfile`, rather than the current state file. Since the new state file is moved into place over the old one, it was effectively unlocking at that point for _new_ processes. But would also unblock an old process which still had the old (now unlinked) file descriptor open.

Hopefully between these two changes, it resolves some of the brokenness that plagues changeset replication! :-)

cookbooks/planet/files/default/replication-bin/replicate-changesets
cookbooks/planet/recipes/replication.rb
cookbooks/planet/templates/default/changesets.conf.erb

index 29131b2edd27c2394fb5d83f76157912d90a58ec..77dc83312e946354415b09112f1952ebffa550d7 100755 (executable)
@@ -261,8 +261,13 @@ class Replicator
 
   # saves new state (including the changeset dump xml)
   def save!
 
   # saves new state (including the changeset dump xml)
   def save!
-    File.open(@config["state_file"], "r") do |fl|
-      fl.flock(File::LOCK_EX)
+    File.open(@config["lock_file"], File::RDWR | File::CREAT, 0o600) do |fl|
+      # take the lock in non-blocking mode. if this process doesn't get the lock
+      # then another will be run from cron shortly. this prevents a whole bunch
+      # of processes queueing on the lock and causing weirdness if/when they
+      # get woken up in a random order.
+      got_lock = fl.flock(File::LOCK_EX | File::LOCK_NB)
+      return unless got_lock
 
       # try and write the files to tmp locations and then
       # move them into place later, to avoid in-progress
 
       # try and write the files to tmp locations and then
       # move them into place later, to avoid in-progress
index b23c00f36bacacdc65ef03b4ec303e247ee56afc..f1be1d0d0b45fe8676d1c8b3ad37b5854b85b851 100644 (file)
@@ -127,6 +127,12 @@ directory "/etc/replication" do
   mode 0o755
 end
 
   mode 0o755
 end
 
+directory "/var/run/lock/changeset-replication/" do
+  owner "planet"
+  group "planet"
+  mode 0o750
+end
+
 template "/etc/replication/auth.conf" do
   source "replication.auth.erb"
   user "root"
 template "/etc/replication/auth.conf" do
   source "replication.auth.erb"
   user "root"
index 309f253b49638720077e3f01bfba988567a8fd1c..73fc83e17ecba370a9c4bd24d7348eee3ea784e1 100644 (file)
@@ -1,3 +1,4 @@
 state_file: /store/planet/replication/changesets/state.yaml
 db: host=<%= node[:web][:database_host] %> dbname=openstreetmap user=planetdiff password=<%= @password %>
 data_dir: /store/planet/replication/changesets
 state_file: /store/planet/replication/changesets/state.yaml
 db: host=<%= node[:web][:database_host] %> dbname=openstreetmap user=planetdiff password=<%= @password %>
 data_dir: /store/planet/replication/changesets
+lock_file: /var/run/lock/changeset-replication/lockfile