gianm commented on code in PR #18121:
URL: https://github.com/apache/druid/pull/18121#discussion_r2178272920
##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/exec/ControllerContext.java:
##########
@@ -33,17 +33,23 @@
import org.apache.druid.server.DruidNode;
import java.io.File;
+import java.util.Map;
/**
* Context used by multi-stage query controllers. Useful because it allows
test fixtures to provide their own
* implementations.
*/
public interface ControllerContext
{
+ /**
+ * Query ID for this context.
+ */
+ String queryId();
Review Comment:
> we already have a lot of queryId-s this method will just increase the
confusion - one implementation of this method returns DART_QUERY_ID and the
other something else
From my point of view both implementations return the same thing: they both
return the unique identifier which globally identifies the query. For the task
it's the controller task ID and for Dart it's the `dartQueryId`. They don't
source the ID from the same place, but that's the point of the context anyway.
It abstracts over differences in execution model.
We can explain this better in the javadoc, using something like:
```
/**
* Globally unique identifier for the query handled by this controller. This
is used to set
* {@link QueryDefinition#getQueryId}. Must be globally unique because this
is used for
* identifying workers, naming temporary files, etc.
*/
String queryId();
```
It might be too disruptive to change the naming now, but I wonder would this
concept seem more OK to you if it was called something else, like `jobId`?
> right now all callsites seem to be inside the implementations of this
interface.
This is true, although I think it would make sense to have `ControllerImpl`
get the query ID from here, rather than have it passed in the constructor.
--
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]
