mjsax commented on PR #20570: URL: https://github.com/apache/kafka/pull/20570#issuecomment-3368609854
I see your points... That's a beast... I was digging into the code a little bit more, and we actually need the "partial initialization" only for `StandbyTasks`, but not for `StreamsTasks`, right? So I am wondering if adding the new `assignThreads()` method to `Task` interface is actually the right thing to do? Should we instead add a `preInitilize()` (or something like this) method only to `StandbyTasks`? And also add a `StandbyTask#completeInitialization()` that we use to set the thread and init the metrics correctly? -- And we might also fix the state transition, and keep the `StandbyTask` in CREATED state (when calling `preInitilize()`) -- it's not correct that the task going into RUNNING right away, as it does now? And only transit to `RUNNING` inside `completeInitialization()`. We still need to keep `initializeIfNeeded` and it might call `preInitilize()` and `completeInitlize()` internally to avoid code duplication? This would imply to change `StateDirectory` to use `StandbyTask` (not `Task` to handle startup tasks, but I don't need any disadvantage doing it -- contrary: it seems to be provide better type safety in any case, to use `StandbyTasks` as all startup tasks are `StandbyTask, and thus there is no good reason to use `Task` interface), and only call the new `preInitilize()` method? Would this work? Btw: your code is still adding `assignThread()` to `StateStore` what we cannot do w/o a KIP (or we piggy-back it on KIP-1035...) -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
