You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by "gianm (via GitHub)" <gi...@apache.org> on 2023/07/06 03:43:40 UTC
[PR] HttpRemoteTaskRunner: Fix markLazyWorkers for maxLazyWorkers == 0. (druid)
gianm opened a new pull request, #14532:
URL: https://github.com/apache/druid/pull/14532
`RemoteTaskRunner` already handles this. Use a similar approach in `HttpRemoteTaskRunner`.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] HttpRemoteTaskRunner: Fix markLazyWorkers for maxLazyWorkers == 0. (druid)
Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm merged PR #14532:
URL: https://github.com/apache/druid/pull/14532
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] HttpRemoteTaskRunner: Fix markLazyWorkers for maxLazyWorkers == 0. (druid)
Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm commented on code in PR #14532:
URL: https://github.com/apache/druid/pull/14532#discussion_r1253902802
##########
indexing-service/src/main/java/org/apache/druid/indexing/overlord/WorkerTaskRunner.java:
##########
@@ -48,8 +48,11 @@ enum ActionType
/**
* Check which workers can be marked as lazy
+ *
+ * @param isLazyWorker predicate that checks if a worker is lazy
+ * @param maxLazyWorkers maximum number of lazy workers to return
*/
- Collection<Worker> markWorkersLazy(Predicate<ImmutableWorkerInfo> isLazyWorker, int maxWorkers);
+ Collection<Worker> markWorkersLazy(Predicate<ImmutableWorkerInfo> isLazyWorker, int maxLazyWorkers);
Review Comment:
I agree the name is funny, but I didn't want to rename it since it's an interface method.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] HttpRemoteTaskRunner: Fix markLazyWorkers for maxLazyWorkers == 0. (druid)
Posted by "kfaraz (via GitHub)" <gi...@apache.org>.
kfaraz commented on code in PR #14532:
URL: https://github.com/apache/druid/pull/14532#discussion_r1253894162
##########
indexing-service/src/main/java/org/apache/druid/indexing/overlord/WorkerTaskRunner.java:
##########
@@ -48,8 +48,11 @@ enum ActionType
/**
* Check which workers can be marked as lazy
+ *
+ * @param isLazyWorker predicate that checks if a worker is lazy
+ * @param maxLazyWorkers maximum number of lazy workers to return
*/
- Collection<Worker> markWorkersLazy(Predicate<ImmutableWorkerInfo> isLazyWorker, int maxWorkers);
+ Collection<Worker> markWorkersLazy(Predicate<ImmutableWorkerInfo> isLazyWorker, int maxLazyWorkers);
Review Comment:
Nit: This method name is probably not accurate anymore, because it does not mark workers as lazy. It gets the list of workers which are lazy and are safe to terminate. So maybe `getLazyWorkersToTerminate`?
```suggestion
Collection<Worker> getLazyWorkersToTerminate(Predicate<ImmutableWorkerInfo> isLazyWorker, int maxToTerminate);
```
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] HttpRemoteTaskRunner: Fix markLazyWorkers for maxLazyWorkers == 0. (druid)
Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm commented on PR #14532:
URL: https://github.com/apache/druid/pull/14532#issuecomment-1624161761
The only failure is due to #14531, which is unrelated.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] HttpRemoteTaskRunner: Fix markLazyWorkers for maxLazyWorkers == 0. (druid)
Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm commented on code in PR #14532:
URL: https://github.com/apache/druid/pull/14532#discussion_r1253902802
##########
indexing-service/src/main/java/org/apache/druid/indexing/overlord/WorkerTaskRunner.java:
##########
@@ -48,8 +48,11 @@ enum ActionType
/**
* Check which workers can be marked as lazy
+ *
+ * @param isLazyWorker predicate that checks if a worker is lazy
+ * @param maxLazyWorkers maximum number of lazy workers to return
*/
- Collection<Worker> markWorkersLazy(Predicate<ImmutableWorkerInfo> isLazyWorker, int maxWorkers);
+ Collection<Worker> markWorkersLazy(Predicate<ImmutableWorkerInfo> isLazyWorker, int maxLazyWorkers);
Review Comment:
I agree the name is funny, but I didn't want to rename it since it's an interface method. I figured for interfaces that people might be using in extensions, best to retain existing signatures unless there is an important functional reason to change them.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org