jiajunwang commented on a change in pull request #1520:
URL: https://github.com/apache/helix/pull/1520#discussion_r522356659
##########
File path: helix-core/src/main/java/org/apache/helix/task/TaskUtil.java
##########
@@ -1142,6 +1142,35 @@ public static void workflowGarbageCollection(final
Set<String> toBePurgedWorkflo
}
}
+ /**
+ * The function that removes IdealStates and job contexts of the jobs that
need to be
+ * deleted.
+ * @param toBePurgedJobs
+ * @param manager
+ */
+ public static void jobGarbageCollection(final Set<String> toBePurgedJobs,
Review comment:
I feel purgeExpiredJobs has a more complete logic to process if the
removal fails. Why not using it directly. In this case, you won't need 2 lists
in the event object. You can merge the job with no config to the expired job
for purging.
Or is there any conflict logic there?
##########
File path: helix-core/src/main/java/org/apache/helix/task/TaskUtil.java
##########
@@ -1142,6 +1142,35 @@ public static void workflowGarbageCollection(final
Set<String> toBePurgedWorkflo
}
}
+ /**
+ * The function that removes IdealStates and job contexts of the jobs that
need to be
+ * deleted.
+ * @param toBePurgedJobs
+ * @param manager
+ */
+ public static void jobGarbageCollection(final Set<String> toBePurgedJobs,
+ final HelixManager manager) {
+ HelixDataAccessor accessor = manager.getHelixDataAccessor();
+ HelixPropertyStore<ZNRecord> propertyStore =
manager.getHelixPropertyStore();
+
+ for (String jobName : toBePurgedJobs) {
+ LOG.warn(
+ "JobContext exists for job {}. However, job Config is missing!
Deleting the JobContext and IdealState!!",
+ jobName);
+
+ // TODO: We dont need this in the future when TF is not relying on IS/EV
anymore.
+ if (!cleanupJobIdealStateExtView(accessor, jobName)) {
+ LOG.warn("Error occurred while trying to remove job
idealstate/externalview for {}.",
+ jobName);
+ continue;
+ }
+
+ if (!removeJobContext(propertyStore, jobName)) {
+ LOG.warn("Error occurred while trying to remove job context for {}.",
jobName);
Review comment:
This section is almost identical as removeJob(), could you please merge?
##########
File path:
helix-core/src/main/java/org/apache/helix/controller/stages/AttributeName.java
##########
@@ -48,5 +48,7 @@
// This attribute should only be used in TaskGarbageCollectionStage, misuse
could cause race conditions.
TO_BE_PURGED_WORKFLOWS,
// This attribute should only be used in TaskGarbageCollectionStage, misuse
could cause race conditions.
+ JOBS_WITHOUT_CONFIG,
Review comment:
One minor question, why not call it TO_BE_PURGED_JOB directly?
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
