jasonk000 commented on a change in pull request #12096:
URL: https://github.com/apache/druid/pull/12096#discussion_r775046521
##########
File path:
indexing-service/src/main/java/org/apache/druid/indexing/overlord/ZkWorker.java
##########
@@ -63,6 +67,26 @@ public ZkWorker(Worker worker, PathChildrenCache
statusCache, final ObjectMapper
this.statusCache = statusCache;
this.cacheConverter = (ChildData input) ->
JacksonUtils.readValue(jsonMapper, input.getData(),
TaskAnnouncement.class);
+ this.taskIdExtractor = createTaskIdExtractor(jsonMapper);
+ }
+
+ static java.util.function.Function<ChildData, String>
createTaskIdExtractor(final ObjectMapper jsonMapper)
+ {
+ return (ChildData input) -> {
+ try {
+ JsonParser parser =
jsonMapper.getFactory().createParser(input.getData());
+ while (parser.nextToken() != JsonToken.END_OBJECT) {
+ if ("id".equals(parser.getCurrentName())) {
+ parser.nextToken();
+ return parser.getText();
+ }
+ }
+ return null;
Review comment:
It will not result in an exception in `getRunningTaskIds`, but it would
allow `null` to be in the result set. I think from this class perspective is
valid (if a task exists with a null id, then `getRunningTaskIds` should include
null). And, `TaskAnnouncement` currently would (should?) exhibit this behaviour
on deserialization.
However, I am not clear on the right system-wide behaviour here. If all
tasks should have valid Ids, then I think the clearer least-surprising
behaviour is to explicitly throw an exception, or maybe filter `null` elements
out of the return value from `getRunningTaskIds`.
What do you think?
--
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]
