golden-yang commented on a change in pull request #9671:
URL: https://github.com/apache/pulsar/pull/9671#discussion_r588840142



##########
File path: 
pulsar-functions/worker/src/main/java/org/apache/pulsar/functions/worker/FunctionRuntimeManager.java
##########
@@ -452,25 +444,43 @@ public synchronized void restartFunctionInstances(String 
tenant, String namespac
                         }
                         continue;
                     }
-
-                    ComponentType componentType = 
assignment.getInstance().getFunctionMetaData().getFunctionDetails().getComponentType();
-
-                    if (ComponentType.SOURCE == componentType) {
-                        this.functionAdmin.sources().restartSource(tenant, 
namespace, functionName,
-                            assignment.getInstance().getInstanceId());
-                    } else if (ComponentType.SINK == componentType) {
-                        this.functionAdmin.sinks().restartSink(tenant, 
namespace, functionName,
-                            assignment.getInstance().getInstanceId());
-                    } else {
-                        this.functionAdmin.functions().restartFunction(tenant, 
namespace, functionName,
-                            assignment.getInstance().getInstanceId());
-                    }
+                    restartFunctionUsingPulsarAdmin(assignment, tenant, 
namespace, functionName, false);
                 }
             }
         }
         return;
     }
 
+    /**
+     * Restart the entire function or restart a single instance of the function
+     */
+    private void restartFunctionUsingPulsarAdmin(Assignment assignment, String 
tenant, String namespace,

Review comment:
       Thank you for your advice.
   I realized that in my previous implementation, using the boolean value 
`restartEntireFunction ` to control function behavior was absolutely bad code.
   
   How about split it into two methods, 'internalrestartinstance' and 
'internalrestartfunction', will it be more intuitive?
   // restarts a particular instance of the function
   `private void internalRestartInstance(Assignment assignment, String tenant, 
String namespace, Integer instanceId){}`
   
   // restarts the whole function
   `private void internalRestartFunction(Assignment assignment, String tenant, 
String namespace) {}`
   
   Do you think it's necessary? Or just take your advice directly.
   @jerrypeng 




----------------------------------------------------------------
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]


Reply via email to