Skip to content

Commit 099f991

Browse files
authored
[#545] Add GroupManager writeLock performance (#551)
1 parent 8b0ef86 commit 099f991

2 files changed

Lines changed: 78 additions & 37 deletions

File tree

opendj-server-legacy/src/main/java/org/opends/server/core/GroupManager.java

Lines changed: 24 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
*
1414
* Copyright 2007-2010 Sun Microsystems, Inc.
1515
* Portions Copyright 2011-2016 ForgeRock AS.
16+
* Portions Copyright 2025 3A Systems,LLC.
1617
*/
1718
package org.opends.server.core;
1819

@@ -726,48 +727,34 @@ private void doPostModify(PluginOperation modifyOperation,
726727
return;
727728
}
728729

730+
Group<?> group =null;
729731
lock.readLock().lock();
730-
try
731-
{
732-
if (!groupInstances.containsKey(oldEntry.getName()))
733-
{
734-
// If the modified entry is not in any group instance, it's probably
735-
// not a group, exit fast
736-
return;
737-
}
732+
try{
733+
group = groupInstances.get(oldEntry.getName());
738734
}
739735
finally
740736
{
741-
lock.readLock().unlock();
742-
}
743-
744-
lock.writeLock().lock();
745-
try
746-
{
747-
Group<?> group = groupInstances.get(oldEntry.getName());
748-
if (group != null)
749-
{
750-
if (!oldEntry.getName().equals(newEntry.getName())
751-
|| !group.mayAlterMemberList()
752-
|| updatesObjectClass(modifications))
753-
{
754-
groupInstances.remove(oldEntry.getName());
755-
// This updates the refreshToken
756-
createAndRegisterGroup(newEntry);
757-
}
758-
else
759-
{
760-
group.updateMembers(modifications);
737+
lock.readLock().unlock();
738+
}
739+
if (group!=null) {
740+
try {
741+
if (!oldEntry.getName().equals(newEntry.getName())
742+
|| !group.mayAlterMemberList()
743+
|| updatesObjectClass(modifications)) {
744+
lock.writeLock().lock();
745+
try {
746+
groupInstances.remove(oldEntry.getName());
747+
// This updates the refreshToken
748+
createAndRegisterGroup(newEntry);
749+
} finally {
750+
lock.writeLock().unlock();
751+
}
752+
} else {
753+
group.updateMembers(modifications);
754+
}
755+
} catch (UnsupportedOperationException | DirectoryException e) {
756+
logger.traceException(e);
761757
}
762-
}
763-
}
764-
catch (UnsupportedOperationException | DirectoryException e)
765-
{
766-
logger.traceException(e);
767-
}
768-
finally
769-
{
770-
lock.writeLock().unlock();
771758
}
772759
}
773760

opendj-server-legacy/src/test/java/org/opends/server/core/GroupManagerTestCase.java

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,17 @@
1313
*
1414
* Copyright 2008-2010 Sun Microsystems, Inc.
1515
* Portions Copyright 2011-2016 ForgeRock AS.
16+
* Portions Copyright 2025 3A Systems, LLC
1617
*/
1718
package org.opends.server.core;
1819

1920
import java.util.LinkedHashSet;
2021
import java.util.List;
2122
import java.util.Set;
23+
import java.util.concurrent.ExecutorService;
24+
import java.util.concurrent.Executors;
25+
import java.util.concurrent.TimeUnit;
26+
import java.util.concurrent.atomic.AtomicInteger;
2227

2328
import org.forgerock.opendj.ldap.DN;
2429
import org.forgerock.opendj.ldap.ResultCode;
@@ -2292,6 +2297,55 @@ public void testSubtreeModify() throws Exception {
22922297
TestCaseUtils.clearBackend("userRoot");
22932298
}
22942299

2300+
@Test
2301+
public void test_issue_535() throws Exception {
2302+
TestCaseUtils.clearBackend("userRoot", "dc=example,dc=com");
2303+
TestCaseUtils.addEntries(
2304+
"dn: ou=Users,dc=example,dc=com",
2305+
"objectClass: organizationalUnit",
2306+
"objectClass: top",
2307+
"ou: Users",
2308+
"",
2309+
"dn: ou=Groups,dc=example,dc=com",
2310+
"objectClass: organizationalUnit",
2311+
"objectClass: top",
2312+
"ou: Groups",
2313+
"",
2314+
"dn: cn=Test User,ou=Users,dc=example,dc=com",
2315+
"objectClass: inetOrgPerson",
2316+
"objectClass: organizationalPerson",
2317+
"objectClass: person",
2318+
"objectClass: top",
2319+
"uid: testuser",
2320+
"cn: Test User",
2321+
"sn: User",
2322+
"userPassword: password123",
2323+
"",
2324+
"dn: cn=Level1,ou=Groups,dc=example,dc=com",
2325+
"objectClass: groupOfNames",
2326+
"objectClass: top",
2327+
"cn: Level1",
2328+
"member: cn=Test User,ou=Users,dc=example,dc=com",
2329+
"",
2330+
"dn: cn=Level2,ou=Groups,dc=example,dc=com",
2331+
"objectClass: groupOfNames",
2332+
"objectClass: top",
2333+
"cn: Level2",
2334+
"member: cn=Level1,ou=Groups,dc=example,dc=com",
2335+
""
2336+
);
2337+
ExecutorService executor = Executors.newFixedThreadPool(100);
2338+
for (int i = 0; i < 10000; i++) {
2339+
executor.submit(() -> {
2340+
final ModifyRequest modifyRequest = newModifyRequest(DN.valueOf("cn=Level2,ou=Groups,dc=example,dc=com"));
2341+
modifyRequest.addModification(REPLACE, "member", "cn=Test User,ou=Users,dc=example,dc=com");
2342+
ModifyOperation modifyOperation = getRootConnection().processModify(modifyRequest);
2343+
assertEquals(modifyOperation.getResultCode(), ResultCode.SUCCESS);
2344+
});
2345+
}
2346+
executor.shutdown();
2347+
assertTrue(executor.awaitTermination(1, TimeUnit.MINUTES));
2348+
}
22952349
/**
22962350
* Adds nested group entries.
22972351
*

0 commit comments

Comments
 (0)