[
https://issues.apache.org/jira/browse/GEODE-8099?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17126995#comment-17126995
]
ASF GitHub Bot commented on GEODE-8099:
---------------------------------------
kirklund commented on a change in pull request #5188:
URL: https://github.com/apache/geode/pull/5188#discussion_r436081052
##########
File path:
geode-core/src/integrationTest/java/org/apache/geode/distributed/internal/InternalLocatorClusterManagementServiceIntegrationTest.java
##########
@@ -56,13 +57,38 @@ public void tearDown() {
}
}
+ @Before
+ public void setup() throws URISyntaxException {
Review comment:
I recommend moving all the field and var creations at the top, then move
all the when blocks together, order both blocks in some way (I alphabetize),
and then consider moving the subject(s) under test to the bottom of the setup
method.
I also recommend cleaning up any statics in tearDown that you've configured
in setUp. For example: how to undo this statement in tearDown:
```
BaseManagementService.setManagementService(cache, managementService);
```
The statics will still have this configured after each test completes.
##########
File path:
geode-core/src/main/java/org/apache/geode/management/internal/api/LocatorClusterManagementService.java
##########
@@ -114,6 +122,7 @@
private final MemberValidator memberValidator;
private final CommonConfigurationValidator commonValidator;
private final InternalCache cache;
+ private DistributedLockService cmsDlockService;
Review comment:
Let's move away from lazy-initializing and mutable fields and just make
it final.
I'm assuming you made it mutable for testing. This is where you typically
want to chain more than one constructor. One of the constructors needs
`@VisibleForTesting` and pass in the DistributedLockService (as a mock or
whatever). The main public constructor can then create the
DistributedLockService and pass it as a parameter to the next constructor. The
test would use the next constructor that accepts a DistributedLockService.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Make ClusterConfiguration Service thread safe
> ---------------------------------------------
>
> Key: GEODE-8099
> URL: https://issues.apache.org/jira/browse/GEODE-8099
> Project: Geode
> Issue Type: Bug
> Components: configuration
> Reporter: Anilkumar Gingade
> Priority: Major
> Labels: GeodeOperationAPI
> Fix For: 1.14.0
>
>
> When multiple cluster configuration clients (multiple rest clients) connects
> and issue configuration commands, the cluster configuration should handle
> concurrent request and modify/save/execute the configuration definition in
> thread safe manner.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
