You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "Pochatkin (via GitHub)" <gi...@apache.org> on 2023/06/22 14:45:47 UTC
[GitHub] [ignite-3] Pochatkin opened a new pull request, #2238: IGNITE-19694 Introduce deployment unit operation ID
Pochatkin opened a new pull request, #2238:
URL: https://github.com/apache/ignite-3/pull/2238
https://issues.apache.org/jira/browse/IGNITE-19694
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] PakhomovAlexander commented on a diff in pull request #2238: IGNITE-19694 Introduce deployment unit operation ID
Posted by "PakhomovAlexander (via GitHub)" <gi...@apache.org>.
PakhomovAlexander commented on code in PR #2238:
URL: https://github.com/apache/ignite-3/pull/2238#discussion_r1243355558
##########
modules/code-deployment/src/main/java/org/apache/ignite/internal/deployunit/metastore/DeploymentUnitFailover.java:
##########
@@ -113,12 +116,43 @@ private void processStatus(UnitClusterStatus unitClusterStatus, UnitNodeStatus u
case REMOVING:
deploymentUnitStore.getAllNodeStatuses(id, version)
.thenAccept(nodes -> {
- UnitNodeStatus status = new UnitNodeStatus(id, version, REMOVING, nodeName);
+ UnitNodeStatus status = new UnitNodeStatus(id, version, REMOVING, unitClusterStatus.opId(), nodeName);
nodeEventCallback.onUpdate(status, nodes);
});
break;
default:
break;
}
}
+
+ private boolean checkClusterNullStatus(String id, Version version, UnitClusterStatus clusterStatus) {
Review Comment:
the method name does not really explain what is going on inside. I would like to ask you to do the null check on the higher level but do the undeploy in the method body. So, the name will be changed as well like `undeploy`
##########
modules/code-deployment/src/main/java/org/apache/ignite/internal/deployunit/metastore/DeploymentUnitFailover.java:
##########
@@ -113,12 +116,43 @@ private void processStatus(UnitClusterStatus unitClusterStatus, UnitNodeStatus u
case REMOVING:
deploymentUnitStore.getAllNodeStatuses(id, version)
.thenAccept(nodes -> {
- UnitNodeStatus status = new UnitNodeStatus(id, version, REMOVING, nodeName);
+ UnitNodeStatus status = new UnitNodeStatus(id, version, REMOVING, unitClusterStatus.opId(), nodeName);
nodeEventCallback.onUpdate(status, nodes);
});
break;
default:
break;
}
}
+
+ private boolean checkClusterNullStatus(String id, Version version, UnitClusterStatus clusterStatus) {
+ if (clusterStatus == null) {
+ deployer.undeploy(id, version)
+ .thenAccept(success -> {
+ if (success) {
+ deploymentUnitStore.removeNodeStatus(nodeName, id, version);
+ }
+ });
+ return true;
+ }
+ return false;
+ }
+
+ private boolean checkAbaProblem(UnitClusterStatus clusterStatus, UnitNodeStatus nodeStatus) {
+ String id = nodeStatus.id();
+ Version version = nodeStatus.version();
+ if (clusterStatus.opId() != nodeStatus.opId()) {
+ if (nodeStatus.status() == DEPLOYED) {
+ deployer.undeploy(id, version).thenAccept(success -> {
+ if (success) {
+ deploymentUnitStore.removeNodeStatus(nodeName, id, version);
+ }
+ });
+ } else {
+ deploymentUnitStore.removeNodeStatus(nodeName, id, version);
Review Comment:
Will be the status removed if `nodeStatus.status` has been changed to `DEPLOYED` since the last check at 145 line?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] Pochatkin commented on a diff in pull request #2238: IGNITE-19694 Introduce deployment unit operation ID
Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.
Pochatkin commented on code in PR #2238:
URL: https://github.com/apache/ignite-3/pull/2238#discussion_r1243469230
##########
modules/code-deployment/src/main/java/org/apache/ignite/internal/deployunit/metastore/DeploymentUnitFailover.java:
##########
@@ -113,12 +116,43 @@ private void processStatus(UnitClusterStatus unitClusterStatus, UnitNodeStatus u
case REMOVING:
deploymentUnitStore.getAllNodeStatuses(id, version)
.thenAccept(nodes -> {
- UnitNodeStatus status = new UnitNodeStatus(id, version, REMOVING, nodeName);
+ UnitNodeStatus status = new UnitNodeStatus(id, version, REMOVING, unitClusterStatus.opId(), nodeName);
nodeEventCallback.onUpdate(status, nodes);
});
break;
default:
break;
}
}
+
+ private boolean checkClusterNullStatus(String id, Version version, UnitClusterStatus clusterStatus) {
+ if (clusterStatus == null) {
+ deployer.undeploy(id, version)
+ .thenAccept(success -> {
+ if (success) {
+ deploymentUnitStore.removeNodeStatus(nodeName, id, version);
+ }
+ });
+ return true;
+ }
+ return false;
+ }
+
+ private boolean checkAbaProblem(UnitClusterStatus clusterStatus, UnitNodeStatus nodeStatus) {
+ String id = nodeStatus.id();
+ Version version = nodeStatus.version();
+ if (clusterStatus.opId() != nodeStatus.opId()) {
+ if (nodeStatus.status() == DEPLOYED) {
+ deployer.undeploy(id, version).thenAccept(success -> {
+ if (success) {
+ deploymentUnitStore.removeNodeStatus(nodeName, id, version);
+ }
+ });
+ } else {
+ deploymentUnitStore.removeNodeStatus(nodeName, id, version);
Review Comment:
This `nodeStatus` can't be changed, because failover start to download unit content after this ABA check
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] PakhomovAlexander merged pull request #2238: IGNITE-19694 Introduce deployment unit operation ID
Posted by "PakhomovAlexander (via GitHub)" <gi...@apache.org>.
PakhomovAlexander merged PR #2238:
URL: https://github.com/apache/ignite-3/pull/2238
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org