Skip to content

Commit

Permalink
fix: Remove thread pool executor logic until we get a better handle o…
Browse files Browse the repository at this point in the history
…n what's causing thread pool hangs. refs graphiti-api#469

revert: "thread pool scope and mutex need to be global across all instances of Scope for it to be a global thread pool (graphiti-api#471)"
revert: "add thread pool and concurrency_max_threads configuration option (graphiti-api#470)"

This reverts commit 51fb51c.
This reverts commit 697d761.
  • Loading branch information
jkeen committed Mar 26, 2024
1 parent 8f46cf3 commit 7941b6f
Show file tree
Hide file tree
Showing 3 changed files with 1 addition and 48 deletions.
15 changes: 0 additions & 15 deletions lib/graphiti/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,6 @@ class Configuration
# Defaults to false OR if classes are cached (Rails-only)
attr_accessor :concurrency

# This number must be considered in accordance with the database
# connection pool size configured in `database.yml`. The connection
# pool should be large enough to accommodate both the foreground
# threads (ie. web server or job worker threads) and background
# threads. For each process, Graphiti will create one global
# executor that uses this many threads to sideload resources
# asynchronously. Thus, the pool size should be at least
# `thread_count + concurrency_max_threads + 1`. For example, if your
# web server has a maximum of 3 threads, and
# `concurrency_max_threads` is set to 4, then your pool size should
# be at least 8.
# @return [Integer] Maximum number of threads to use when fetching sideloads concurrently
attr_accessor :concurrency_max_threads

attr_accessor :respond_to
attr_accessor :context_for_endpoint
attr_accessor :links_on_demand
Expand All @@ -40,7 +26,6 @@ class Configuration
def initialize
@raise_on_missing_sideload = true
@concurrency = false
@concurrency_max_threads = 4
@respond_to = [:json, :jsonapi, :xml]
@links_on_demand = false
@pagination_links_on_demand = false
Expand Down
19 changes: 1 addition & 18 deletions lib/graphiti/scope.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,6 @@ module Graphiti
class Scope
attr_accessor :object, :unpaginated_object
attr_reader :pagination

@@thread_pool_executor_mutex = Mutex.new

def self.thread_pool_executor
return @thread_pool_executor if @thread_pool_executor

concurrency = Graphiti.config.concurrency_max_threads || 4
@@thread_pool_executor_mutex.synchronize do
@@thread_pool_executor ||= Concurrent::ThreadPoolExecutor.new(
min_threads: 0,
max_threads: concurrency,
max_queue: concurrency * 4,
fallback_policy: :caller_runs
)
end
end

def initialize(object, resource, query, opts = {})
@object = object
@resource = resource
Expand Down Expand Up @@ -66,7 +49,7 @@ def resolve_sideloads(results)
@resource.adapter.close if concurrent
}
if concurrent
promises << Concurrent::Promise.execute(executor: self.class.thread_pool_executor, &resolve_sideload)
promises << Concurrent::Promise.execute(&resolve_sideload)
else
resolve_sideload.call
end
Expand Down
15 changes: 0 additions & 15 deletions spec/configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -150,21 +150,6 @@
end
end

describe "#concurrency_max_threads" do
include_context "with config", :concurrency_max_threads

it "defaults" do
expect(Graphiti.config.concurrency_max_threads).to eq(4)
end

it "is overridable" do
Graphiti.configure do |c|
c.concurrency_max_threads = 1
end
expect(Graphiti.config.concurrency_max_threads).to eq(1)
end
end

describe "#raise_on_missing_sideload" do
include_context "with config", :raise_on_missing_sideload

Expand Down

0 comments on commit 7941b6f

Please sign in to comment.