You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by GitBox <gi...@apache.org> on 2021/06/08 15:45:07 UTC
[GitHub] [brooklyn-server] zan-mateusz opened a new pull request #1184: code for functionality to remove a particular node from persistence via API
zan-mateusz opened a new pull request #1184:
URL: https://github.com/apache/brooklyn-server/pull/1184
WIP Introducing API to remove a particular node from persistence (might need to be limited to terminated nodes only as removing an active nodes might cause some potential issues).
This change will be linked to corresponding UI changes covering changing status and priority of nodes and removing terminated nodes, so via UI it would only be possible to remove terminated nodes anyway, but there is currently no check for node status on server side
--
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:
users@infra.apache.org
[GitHub] [brooklyn-server] algairim commented on a change in pull request #1184: code for functionality to remove a particular node from persistence via API
Posted by GitBox <gi...@apache.org>.
algairim commented on a change in pull request #1184:
URL: https://github.com/apache/brooklyn-server/pull/1184#discussion_r669537927
##########
File path: core/src/main/java/org/apache/brooklyn/core/mgmt/ha/HighAvailabilityManagerImpl.java
##########
@@ -1004,6 +1019,15 @@ public ManagementPlaneSyncRecord loadManagementPlaneSyncRecord(boolean useLocalK
return record;
}
+ private boolean updateLastManagementPlaneSyncRecordWithLocalKnowledge() {
+ if (lastSyncRecord != null) {
+ lastSyncRecord = updateManagementPlaneSyncRecordWithLocalKnowledge(lastSyncRecord);
+ return true;
+ } else {
+ return false;
+ }
+ }
Review comment:
Do we need this method at all? Dose not look like return value is used anywhere.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-server] algairim commented on a change in pull request #1184: code for functionality to remove a particular node from persistence via API
Posted by GitBox <gi...@apache.org>.
algairim commented on a change in pull request #1184:
URL: https://github.com/apache/brooklyn-server/pull/1184#discussion_r669536725
##########
File path: core/src/main/java/org/apache/brooklyn/core/mgmt/ha/HighAvailabilityManagerImpl.java
##########
@@ -1072,6 +1081,26 @@ private ManagementPlaneSyncRecord loadManagementPlaneSyncRecordInternal(boolean
throw new IllegalStateException(message, lastException);
}
+ private ManagementPlaneSyncRecord updateManagementPlaneSyncRecordWithLocalKnowledge(ManagementPlaneSyncRecord result) {
Review comment:
I would add javadoc saying that this method re-assigns the value of the supplied parameter. Otherwise, make it final if you prefer to return a new value from this method.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-server] iuliana merged pull request #1184: code for functionality to remove a particular node from persistence via API
Posted by GitBox <gi...@apache.org>.
iuliana merged pull request #1184:
URL: https://github.com/apache/brooklyn-server/pull/1184
--
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.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-server] algairim commented on pull request #1184: code for functionality to remove a particular node from persistence via API
Posted by GitBox <gi...@apache.org>.
algairim commented on pull request #1184:
URL: https://github.com/apache/brooklyn-server/pull/1184#issuecomment-880515818
Looks good now, build failure is not related to these changes.
Unit-test to follow in a separate PR.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-server] algairim commented on a change in pull request #1184: code for functionality to remove a particular node from persistence via API
Posted by GitBox <gi...@apache.org>.
algairim commented on a change in pull request #1184:
URL: https://github.com/apache/brooklyn-server/pull/1184#discussion_r669527429
##########
File path: core/src/main/java/org/apache/brooklyn/core/mgmt/ha/HighAvailabilityManagerImpl.java
##########
@@ -1072,6 +1081,26 @@ private ManagementPlaneSyncRecord loadManagementPlaneSyncRecordInternal(boolean
throw new IllegalStateException(message, lastException);
}
+ private ManagementPlaneSyncRecord updateManagementPlaneSyncRecordWithLocalKnowledge(ManagementPlaneSyncRecord result) {
+ // Report this node's most recent state, and detect AWOL nodes
+ ManagementNodeSyncRecord me = BasicManagementNodeSyncRecord.builder()
+ .from(result.getManagementNodes().get(ownNodeId), true)
+ .from(createManagementNodeSyncRecord(false), true)
+ .build();
+ Iterable<ManagementNodeSyncRecord> allNodes = result.getManagementNodes().values();
+ if (me.getRemoteTimestamp()!=null)
+ allNodes = Iterables.transform(allNodes, new MarkAwolNodes(me));
+ Builder builder = ManagementPlaneSyncRecordImpl.builder()
+ .masterNodeId(result.getMasterNodeId())
+ .nodes(allNodes);
+ builder.node(me);
+ if (getTransitionTargetNodeState() == ManagementNodeState.MASTER) {
+ builder.masterNodeId(ownNodeId);
+ }
+ result = builder.build();
+ return result;
Review comment:
Remove the `return` condition from the method signature. It is not required.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-server] algairim commented on a change in pull request #1184: code for functionality to remove a particular node from persistence via API
Posted by GitBox <gi...@apache.org>.
algairim commented on a change in pull request #1184:
URL: https://github.com/apache/brooklyn-server/pull/1184#discussion_r669457493
##########
File path: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ServerResource.java
##########
@@ -476,6 +477,16 @@ public Response clearHighAvailabilityPlaneStates() {
return Response.ok().build();
}
+ @Override
+ public Response clearHighAvailabilityPlaneStates(String nodeId) {
Review comment:
Unit-test for this in `ServerResourceTest` please.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org