FI-2801: Fix validator service race condition (#498)

* bump sidekiq * add separate queue/capsule for invoking validator sessions * enable hl7 validator service by default * Update docker-compose.background.yml Co-authored-by: Dylan Hall <dehall@mitre.org> --------- Co-authored-by: Dylan Hall <dehall@mitre.org>
inferno-framework · May 31, 2024 · b51b9bc · b51b9bc
1 parent 85183c0
commit b51b9bc
Show file tree

Hide file tree

Showing 7 changed files with 44 additions and 35 deletions.
diff --git a/.env b/.env
@@ -24,3 +24,8 @@ FHIR_RESOURCE_VALIDATOR_URL=http://localhost/hl7validatorapi
 # values.
 # MAX_DB_CONNECTION_ATTEMPTS=10
 # DB_CONNECTION_RETRY_DELAY=5
+
+# Set how many validator sessions to create in parallel at startup. Session
+# creation in the validator is not completely thread safe, so values greater
+# than 1 could cause issues.
+# VALIDATOR_SESSIONS_CONCURRENCY=1
diff --git a/Gemfile.lock b/Gemfile.lock
@@ -23,7 +23,7 @@ PATH
       puma (~> 5.6.7)
       rake (~> 13.0)
       sequel (~> 5.42.0)
-      sidekiq (~> 6.5.6)
+      sidekiq (~> 7.2.4)
       sqlite3 (~> 1.4)
       thor (~> 1.2.1)
       tty-markdown (~> 0.7.1)
@@ -211,12 +211,13 @@ GEM
     puma (5.6.8)
       nio4r (~> 2.0)
     racc (1.7.3)
-    rack (2.2.8.1)
+    rack (2.2.9)
     rack-test (1.1.0)
       rack (>= 1.0, < 3)
     rainbow (3.1.1)
     rake (13.2.1)
-    redis (4.8.1)
+    redis-client (0.22.2)
+      connection_pool
     regexp_parser (2.2.0)
     reline (0.3.2)
       io-console (~> 0.5)
@@ -260,10 +261,11 @@ GEM
     ruby-progressbar (1.11.0)
     ruby2_keywords (0.0.5)
     sequel (5.42.0)
-    sidekiq (6.5.12)
-      connection_pool (>= 2.2.5, < 3)
-      rack (~> 2.0)
-      redis (>= 4.5.0, < 5)
+    sidekiq (7.2.4)
+      concurrent-ruby (< 2)
+      connection_pool (>= 2.3.0)
+      rack (>= 2.2.4)
+      redis-client (>= 0.19.0)
     simplecov (0.21.2)
       docile (~> 1.1)
       simplecov-html (~> 0.11)

diff --git a/config/nginx.background.conf b/config/nginx.background.conf
@@ -83,22 +83,20 @@ http {
       proxy_pass http://validator_service:4567/;
     }
 
-# To enable the HL7 Validator Wrapper, both the section below and
-# the section in docker-compose.background.yml need to be uncommented
-#    location /hl7validatorapi/ {
-#      proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
-#      proxy_set_header Host $http_host;
-#      proxy_set_header X-Forwarded-Proto $scheme;
-#      proxy_set_header X-Forwarded-Port $server_port;
-#      proxy_redirect off;
-#      proxy_set_header Connection '';
-#      proxy_http_version 1.1;
-#      chunked_transfer_encoding off;
-#      proxy_buffering off;
-#      proxy_cache off;
-#      proxy_read_timeout 600s;
-#
-#      proxy_pass http://hl7_validator_service:3500/;
-#    }
+   location /hl7validatorapi/ {
+     proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
+     proxy_set_header Host $http_host;
+     proxy_set_header X-Forwarded-Proto $scheme;
+     proxy_set_header X-Forwarded-Port $server_port;
+     proxy_redirect off;
+     proxy_set_header Connection '';
+     proxy_http_version 1.1;
+     chunked_transfer_encoding off;
+     proxy_buffering off;
+     proxy_cache off;
+     proxy_read_timeout 600s;
+
+     proxy_pass http://hl7_validator_service:3500/;
+   }
   }
 }
diff --git a/docker-compose.background.yml b/docker-compose.background.yml
@@ -28,13 +28,11 @@ services:
     volumes:
       - ./data/redis:/data
     command: redis-server --appendonly yes
-  # To enable the HL7 Validator Wrapper, both the section below and
-  # the section in nginx.background.conf need to be uncommented
-  # hl7_validator_service:
-  #   image: infernocommunity/inferno-resource-validator
-  #   # Update this path to match your directory structure
-  #   volumes:
-  #     - ./igs:/home/igs
-  #     # To let the service share your local FHIR package cache,
-  #     # uncomment the below line
-  #     # - ~/.fhir:/home/ktor/.fhir
+  hl7_validator_service:
+    image: infernocommunity/inferno-resource-validator
+    # Update this path to match your directory structure
+    volumes:
+      - ./igs:/app/igs
+      # To let the service share your local FHIR package cache,
+      # uncomment the below line
+      # - ~/.fhir:/home/ktor/.fhir
diff --git a/inferno_core.gemspec b/inferno_core.gemspec
@@ -32,7 +32,7 @@ Gem::Specification.new do |spec|
   spec.add_runtime_dependency 'puma', '~> 5.6.7'
   spec.add_runtime_dependency 'rake', '~> 13.0'
   spec.add_runtime_dependency 'sequel', '~> 5.42.0'
-  spec.add_runtime_dependency 'sidekiq', '~> 6.5.6'
+  spec.add_runtime_dependency 'sidekiq', '~> 7.2.4'
   spec.add_runtime_dependency 'sqlite3', '~> 1.4'
   spec.add_runtime_dependency 'thor', '~> 1.2.1'
   spec.add_runtime_dependency 'tty-markdown', '~> 0.7.1'

diff --git a/lib/inferno/config/boot/sidekiq.rb b/lib/inferno/config/boot/sidekiq.rb
@@ -5,6 +5,11 @@
     if Inferno::Application['async_jobs']
       Sidekiq.configure_server do |config|
         config.redis = { url: ENV.fetch('REDIS_URL', 'redis://127.0.0.1:6379/0') }
+
+        config.capsule('validator_sessions') do |cap|
+          cap.concurrency = ENV.fetch('VALIDATOR_SESSIONS_CONCURRENCY', '1').to_i
+          cap.queues = ['validator_sessions']
+        end
       end
     end
   end

diff --git a/lib/inferno/jobs/invoke_validator_session.rb b/lib/inferno/jobs/invoke_validator_session.rb
@@ -2,6 +2,7 @@ module Inferno
   module Jobs
     class InvokeValidatorSession
       include Sidekiq::Worker
+      sidekiq_options queue: 'validator_sessions'
 
       def perform(suite_id, validator_name, validator_index)
         suite = Inferno::Repositories::TestSuites.new.find suite_id