Menachem Shapira created GUACAMOLE-2316:
-------------------------------------------
Summary: Allow the number of display encoder worker threads per
connection to be limited
Key: GUACAMOLE-2316
URL: https://issues.apache.org/jira/browse/GUACAMOLE-2316
Project: Guacamole
Issue Type: Improvement
Components: guacd, libguac
Affects Versions: 1.6.1 [staging]
Reporter: Menachem Shapira
*Problem*
{{guac_display_alloc()}} sizes its encoder worker thread pool from the number
of available processors:
{{display->worker_thread_count = cpu_count * GUAC_DISPLAY_CPU_THREAD_FACTOR;}}
Each connection gets its own {{{}guac_display{}}}, so the total number of
encoder threads in a guacd deployment grows as {{{}concurrent_connections ×
available_processors{}}}. There is no upper bound.
On modern high-core-count servers this scales badly. On a 128-core host, 50
concurrent connections request on the order of 6,400 encoder threads. Well
before that, the host hits per-process/per-user thread limits
({{{}RLIMIT_NPROC{}}}, {{pids.max}} in containers, {{{}threads-max{}}}), thread
stack memory, and scheduler overhead. The failure is not graceful:
{{pthread_create()}} return values are not currently checked, so exhaustion
surfaces as unpredictable behavior rather than a clean error.
The parallelism is also largely wasted. A single display rarely produces enough
concurrently-encodable work to occupy that many processors, so the marginal
thread contributes contention rather than throughput.
*Proposal*
Make the per-connection encoder worker thread count configurable, with the
limit owned by the {{guac_client}} and read at display allocation time.
* Add {{int max_display_worker_threads}} to {{guac_client}} (appended to the
struct; {{0}} = no limit beyond available processors, the current behavior).
* {{guac_display_alloc()}} caps its computed worker count against the owning
client's value. The cap can only reduce the count, never increase it, and is
applied before the worker array is allocated and before any
{{pthread_create()}} call.
* guacd exposes the limit as {{max_worker_threads}} in {{guacd.conf}} and
{{-w}} on the command line, carries it through connection routing, and assigns
it to {{proc->client}} in {{guacd_create_proc()}} before {{{}fork(){}}}.
* The guacd Docker image exposes it as the {{MAX_WORKER_THREADS}} environment
variable.
Storing the limit on the client rather than in process-global libguac state
means the value transports naturally across the guacd → protocol-plugin
boundary (guacd does not itself call {{{}guac_display_alloc(){}}}; the RDP/VNC
implementations do), independent clients in one process can use different
limits, there is no non-thread-safe public setter and no data race, and no
existing public signature changes.
*Scope / non-goals*
This is a *per-connection* cap, not a daemon-wide thread budget. It changes
growth from {{connections × detected_cpus}} to {{{}connections ×
configured_limit{}}}; it does not impose a hard ceiling on total encoder
threads across all connections. A true global budget would require a shared
worker pool or admission control and is a separate, substantially larger design.
Also out of scope: resizing an existing display's worker pool at runtime, and
configuration reload.
*Open decision: default value*
Two defensible policies, and this should be an explicit maintainer decision
rather than an implicit one:
* _Compatibility-first_ (what the current patch does): default {{0}} /
unlimited. Zero regression risk, but the hazard remains for every deployment
that doesn't discover the setting — the change is a mitigation operators must
opt into, not a fix.
* {_}Safety-first{_}: a small finite default (e.g. 4), with {{0}} explicitly
opting back into legacy CPU-count behavior. Fixes existing deployments on
upgrade, but needs benchmark evidence and release notes since some workloads
may regress.
Suggest benchmarking effective counts of 1, 2, 4, 8, and CPU-count across
representative RDP/VNC workloads (typical resolutions, high-motion content)
under both single-connection and high-concurrency load before choosing.
*Acceptance criteria*
# {{guacd.conf}} {{max_worker_threads}} and {{-w}} accept a non-negative
integer; non-numeric, trailing-garbage, negative, and out-of-range values are
rejected with a clear error at startup.
# A configured limit lower than the detected processor count produces exactly
that many worker threads per display.
# A configured limit higher than the detected count does not increase the
thread count above the CPU-derived value.
# A limit of {{1}} produces exactly one worker thread.
# {{0}} (and unset) preserves current behavior exactly.
# Two clients in one process with different limits produce different worker
counts; a configured client does not affect a client using the default.
# The limit is read at {{guac_display_alloc()}} time; changing it afterward
does not mutate an already-allocated display.
# A libguac consumer that never sets the field gets the default — verify
{{guac_client_alloc()}} zero-initializes it.
# Man pages ({{{}guacd.8{}}}, {{{}guacd.conf.5{}}}) document that the limit is
per connection, not daemon-wide.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
