[
https://issues.apache.org/jira/browse/GEODE-8099?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17144222#comment-17144222
]
ASF GitHub Bot commented on GEODE-8099:
---------------------------------------
jchen21 commented on a change in pull request #5285:
URL: https://github.com/apache/geode/pull/5285#discussion_r445095363
##########
File path:
geode-gfsh/src/main/java/org/apache/geode/management/cli/SingleGfshCommand.java
##########
@@ -39,7 +42,11 @@
* return value of your command method.
* @return a boolean indicating whether a change to the cluster
configuration was persisted.
*/
- public boolean updateConfigForGroup(String group, CacheConfig config, Object
configObject) {
- return false;
+ public abstract boolean updateConfigForGroup(String group, CacheConfig
config,
Review comment:
This changes the public API, which could affect those gfsh commands that
extend this class.
##########
File path:
geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessor.java
##########
@@ -53,14 +57,13 @@
private SecurityService securityService;
- private InternalCache cache;
-
public OnlineCommandProcessor() {}
@VisibleForTesting
- public OnlineCommandProcessor(Properties cacheProperties, SecurityService
securityService,
- CommandExecutor commandExecutor, InternalCache cache) {
- this.gfshParser = new GfshParser(new CommandManager(cacheProperties,
cache));
+ public OnlineCommandProcessor(GfshParser gfshParser,
Review comment:
This constructor is used only by tests. e.g. `MemberCommandServiceTest`.
This also leads to adding a new constructor to a deprecated class
`MemberCommandService`. And then changes to `MemberCommandServiceTest`. I
wonder if it is necessary to make this change.
##########
File path:
geode-gfsh/src/main/java/org/apache/geode/management/cli/SingleGfshCommand.java
##########
@@ -39,7 +42,11 @@
* return value of your command method.
* @return a boolean indicating whether a change to the cluster
configuration was persisted.
*/
- public boolean updateConfigForGroup(String group, CacheConfig config, Object
configObject) {
- return false;
+ public abstract boolean updateConfigForGroup(String group, CacheConfig
config,
+ Object configObject);
+
+ @Override
+ public boolean affectsClusterConfiguration() {
Review comment:
This changes the public API, which could affect those gfsh commands that
extend this class.
----------------------------------------------------------------
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)
