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 2022/06/24 15:06:02 UTC
[GitHub] [brooklyn-server] ahgittin opened a new pull request, #1325: Fix persistence deletion race
ahgittin opened a new pull request, #1325:
URL: https://github.com/apache/brooklyn-server/pull/1325
Build on #1307, fixing the races that could cause persistence files for unmanaged items. Expands test to cover enrichers also. Several routes identified and fixed, plus better warnings, per the individual commits.
--
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] ahgittin commented on a diff in pull request #1325: Fix persistence deletion race
Posted by GitBox <gi...@apache.org>.
ahgittin commented on code in PR #1325:
URL: https://github.com/apache/brooklyn-server/pull/1325#discussion_r907462662
##########
core/src/main/java/org/apache/brooklyn/core/entity/Entities.java:
##########
@@ -948,10 +955,22 @@ public static ManagementContext startManagement(Application app, ManagementConte
}
}
+ premanageRecursively((ManagementContextInternal)mgmt, app);
+
mgmt.getEntityManager().manage(app);
return mgmt;
}
+ private static void premanageRecursively(ManagementContextInternal mgmt, Entity e) {
+ if (Entities.isUnmanagingOrNoLongerManaged(e) || Entities.isManagedActive(e)) {
+ log.warn("Skipping premanagement for "+e+", as it has some form of known management");
+ // skip for active and unmanaged entities
+ return;
+ }
+ mgmt.prePreManage(e);
Review Comment:
this method is called `premanageRecursively` so i think the answer is yes. this is only needed for legacy code (tests), which does want the recursive behaviour.
--
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] asfgit merged pull request #1325: Fix persistence deletion race
Posted by GitBox <gi...@apache.org>.
asfgit merged PR #1325:
URL: https://github.com/apache/brooklyn-server/pull/1325
--
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 diff in pull request #1325: Fix persistence deletion race
Posted by GitBox <gi...@apache.org>.
algairim commented on code in PR #1325:
URL: https://github.com/apache/brooklyn-server/pull/1325#discussion_r907355718
##########
core/src/main/java/org/apache/brooklyn/core/entity/Entities.java:
##########
@@ -948,10 +955,22 @@ public static ManagementContext startManagement(Application app, ManagementConte
}
}
+ premanageRecursively((ManagementContextInternal)mgmt, app);
+
mgmt.getEntityManager().manage(app);
return mgmt;
}
+ private static void premanageRecursively(ManagementContextInternal mgmt, Entity e) {
+ if (Entities.isUnmanagingOrNoLongerManaged(e) || Entities.isManagedActive(e)) {
+ log.warn("Skipping premanagement for "+e+", as it has some form of known management");
+ // skip for active and unmanaged entities
+ return;
+ }
+ mgmt.prePreManage(e);
Review Comment:
Can it be that this condition is required for a particular entity, but not for its children?
--
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 diff in pull request #1325: Fix persistence deletion race
Posted by GitBox <gi...@apache.org>.
algairim commented on code in PR #1325:
URL: https://github.com/apache/brooklyn-server/pull/1325#discussion_r907357281
##########
core/src/main/java/org/apache/brooklyn/core/entity/Entities.java:
##########
@@ -948,10 +955,22 @@ public static ManagementContext startManagement(Application app, ManagementConte
}
}
+ premanageRecursively((ManagementContextInternal)mgmt, app);
+
mgmt.getEntityManager().manage(app);
return mgmt;
}
+ private static void premanageRecursively(ManagementContextInternal mgmt, Entity e) {
+ if (Entities.isUnmanagingOrNoLongerManaged(e) || Entities.isManagedActive(e)) {
+ log.warn("Skipping premanagement for "+e+", as it has some form of known management");
+ // skip for active and unmanaged entities
+ return;
+ }
+ mgmt.prePreManage(e);
Review Comment:
In other words: should we proceed recursively with children?
--
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