clintropolis commented on code in PR #13930:
URL: https://github.com/apache/druid/pull/13930#discussion_r1140697270
##########
indexing-service/src/main/java/org/apache/druid/indexing/overlord/hrtr/HttpRemoteTaskRunner.java:
##########
@@ -565,13 +566,26 @@ public void nodeViewInitialized()
private Worker toWorker(DiscoveryDruidNode node)
{
+ final WorkerNodeService workerNodeService =
node.getService(WorkerNodeService.DISCOVERY_SERVICE_KEY,
WorkerNodeService.class);
+ if (workerNodeService == null) {
+ // this shouldn't typically happen, but just in case it does, make a
dummy worker to allow the callbacks to
+ // continue since addWorker/removeWorker only need worker.getHost()
Review Comment:
Yeah, I would agree it seems a bit fragile, I was trying to avoid a larger
refactor since the code flow here is a bit complicated. The thing behind both
`addWorker` and `removeWorker` (and the analog methods on
`HttpServerInventoryView`, `serverAdded` and `serverRemoved`) is combining
tasks (or segments) discovered and the worker/server that is serving them to
track what is where, so theoretically any workers/servers that are ..
"service-less" should just hang out harmlessly in the collection of known hosts
since no segments or tasks should be associated with them, and the host part is
the only thing used when adding them or removing them from the collection.
It would probably be worth refactoring to modify these views to just
silently ignore these servers instead of adding them to the collection, but
seemed a bit higher risk than I was hoping to take on at the moment, so I was
trying to add these guards here "just in case" because the consequences of a
failure in these callbacks is pretty bad. Maybe its not as hard as I'm making
it out to be though, I'll have a look to see how complicated it would be to
just ignore nodes that are not `WorkerNodeService` or `DataNodeService`.
--
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]
