kfaraz commented on PR #16328: URL: https://github.com/apache/druid/pull/16328#issuecomment-2075054859
As I see it, pros of this approach: - It doesn't require changing the way a `CliPeon` is launched. - Allows specific task implementations such as `MsqControllerTask` to give dynamic implementations of their `getLookupsToLoad()` methods, thus allowing them to load any list of tasks. Even with the other approach, this method would be needed at some point down the line. - Doesn't require a new enum. Cons of this approach: - We need to attach a special meaning to `null`. In other words, `null` here means `Load Everything`, which is not too great. This can always be remedied though by returning a wrapper object instead of just a `List<String>`. The wrapper object can have a boolean or enum flag to indicate the tristate: ALL, NONE, REQUIRED. - Based on this comment (https://github.com/apache/druid/issues/13324#issuecomment-1321483228), it seems that MSQ workers know about the lookups they need to load only after they start running (or maybe I am reading it wrong). The approach in this PR injects the list of lookups to load at start-up itself. Is the list of lookups required for a worker included in its task payload? -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
