NealSun96 commented on a change in pull request #1076:
URL: https://github.com/apache/helix/pull/1076#discussion_r437785603
##########
File path:
helix-core/src/main/java/org/apache/helix/controller/stages/TaskGarbageCollectionStage.java
##########
@@ -23,34 +27,53 @@ public AsyncWorkerType getAsyncWorkerType() {
}
@Override
- public void execute(ClusterEvent event) {
+ public void process(ClusterEvent event) throws Exception {
WorkflowControllerDataProvider dataProvider =
event.getAttribute(AttributeName.ControllerDataProvider.name());
+ event.addAttribute(AttributeName.WORKFLOW_CONFIG_MAP.name(),
+ dataProvider.getWorkflowConfigMap());
Review comment:
So you're right that looping is done twice, but there is no exact
duplication for sure. My point is that TaskSchedulingStage is doing a lot of
things, to a point where I think it's better for the code base if we allow the
other stage to do work just to improve clarity. Is the delay of running a loop
a matter of concern? If so we should do it in the TaskSchedulingStage loop.
About your second thought: 1. even if we do the check in async thread, same
job name is still an issue because the operation is not atomic. It could be
recreated just before deletion; 2. if this is a concern for jobs, this should
also be a concern for workflow? If we need to do the check for workflow in
async thread, we face the same problem again.
----------------------------------------------------------------
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]
