Remove per-bind global lock contention in AuthenticatedUsers#660
Open
vharseko wants to merge 2 commits into
Open
Remove per-bind global lock contention in AuthenticatedUsers#660vharseko wants to merge 2 commits into
vharseko wants to merge 2 commits into
Conversation
The `snmp` profile assembles the server template config.ldif in two antrun steps: `copy-config-ldif` copies the pristine resource/config/config.ldif into target/template/config, and `generate-config-ldif` appends config.snmp.ldif to it with `concat append="true"`. The copy used Ant's default `overwrite="false"`, so on an incremental build (without `clean`) the already-populated target file was newer than the source and was not refreshed, while the concat appended the SNMP fragment again on every build. Repeated builds therefore accumulated several `cn=SNMP Connection Handler,cn=Connection Handlers,cn=config` entries, and `setup` then failed with "Entry Already Exists ... multiple times". Add `overwrite="true"` to the config.ldif copy so the SNMP fragment is always appended to a freshly copied base file, making config.ldif generation idempotent regardless of incremental builds.
Every successful bind (and unbind) registers/deregisters the client
connection in the global AuthenticatedUsers map via
ClientConnection.setAuthenticationInfo(). The map was a DITCacheMap guarded
by a single ReentrantReadWriteLock, so put()/remove() serialized on one
write lock that is taken on the hot path of every authentication.
Under high concurrency this is a real bottleneck: with 200 connections
binding as different users, roughly half of the worker threads were parked
acquiring that lock (AuthenticatedUsers.put/remove -> WriteLock.lock),
confirmed by jstack sampling, even though the connections target different
users and never actually conflict.
Replace the DITCacheMap + global lock with a
ConcurrentHashMap<DN, CopyOnWriteArraySet<ClientConnection>>:
- put/remove/get are now lock-free at the map level (bin-granularity locking
via computeIfAbsent/computeIfPresent); binds for different users no longer
serialize.
- The post-response handlers that react to changes of authenticated user
entries are adjusted accordingly:
- modify uses an exact-DN lookup (O(1)) - a modify never affects
descendants, so no subtree scan is needed;
- delete and modify-DN keep subtree semantics via a single key-set pass
(removeSubtree), guarded by an isEmpty() fast-exit; the redundant
pre-check method operationDoesNotTargetAuthenticatedUser is removed.
Public behaviour and the get() return type (CopyOnWriteArraySet) are
unchanged.
Measured on a packaged JE server (5000 users, 200 threads, persistent
connections rebinding as random distinct users):
throughput 18.4k -> ~23k binds/s (+~25%)
mean 11.0 -> ~8.5 ms
p50 9.5 -> ~6.5 ms
workers parked on AuthenticatedUsers lock: ~half -> 0
Add BindLatencyBenchmarkTestCase: an opt-in concurrent BIND latency /
throughput benchmark (N threads, distinct users, persistent connections)
used to capture the before/after numbers. It is disabled by default and only
runs with -Dbind.bench=true, so it never slows the normal test suite.
maximthomas
approved these changes
Jun 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Removes a global lock that every LDAP BIND (and unbind) takes on the hot
path, which serialized authentications under concurrency. Replaces the
AuthenticatedUsersmap (aDITCacheMapguarded by a singleReentrantReadWriteLock) with aConcurrentHashMap, so registering /deregistering a connection no longer contends on one global write lock.
Test PR vs Release: https://github.com/vharseko/OpenDJ/actions/runs/28219689094
Problem
Every successful bind registers the connection in the server-wide
AuthenticatedUsersmap viaClientConnection.setAuthenticationInfo()(
puton bind,removeon unbind / re-bind). The map was backed by aDITCacheMapprotected by a singleReentrantReadWriteLock, soput()andremove()took one global write lock on the hot path of everyauthentication — even though connections authenticating as different users
never actually conflict.
Under load this serializes binds. With 200 connections binding as different
users, jstack sampling showed roughly half of the worker threads parked
acquiring that lock:
Change
AuthenticatedUsersnow usesConcurrentHashMap<DN, CopyOnWriteArraySet<ClientConnection>>.put/remove/getare lock-free at the map level (bin-granularity locking viacomputeIfAbsent/computeIfPresent); binds for different users run inparallel.
changes to their entries are updated:
O(1)(a modify never affects descendants,so no subtree scan is needed);
pass (
removeSubtree) with anisEmpty()fast-exit; the redundantpre-check
operationDoesNotTargetAuthenticatedUseris removed.get()return type (CopyOnWriteArraySet) areunchanged.
Benchmark
Packaged server,
jebackend, 5000 users (uid=user.N,userPasswordhashed
{SSHA}), 200 client threads on persistent connections eachre-binding as a random distinct user, JDK 11.
At moderate (non-saturating) concurrency the win is larger and tail latency
also improves; at 200-thread saturation p99 is GC-bound at the higher
operating point (no longer the bind bookkeeping).
Testing
Adds
BindLatencyBenchmarkTestCase— an opt-in concurrent BINDlatency/throughput benchmark (N threads, distinct users, persistent
connections). Disabled by default; it only runs with
-Dbind.bench=true,so the normal test suite is unaffected. Example:
Existing bind /
AuthenticatedUsersregression tests continue to pass;main and test sources compile (JDK 11, target 11).
Files
opendj-server-legacy/src/main/java/org/opends/server/core/AuthenticatedUsers.javaopendj-server-legacy/src/test/java/org/opends/server/core/BindLatencyBenchmarkTestCase.java