Invoke GetRollupCapsAction in a management thread#98314
Invoke GetRollupCapsAction in a management thread#98314csoulios merged 9 commits intoelastic:mainfrom
GetRollupCapsAction in a management thread#98314Conversation
GetRollupCapsAction in a management threadGetRollupCapsAction in a management thread
|
Pinging @elastic/es-analytics-geo (Team:Analytics) |
DaveCTurner
left a comment
There was a problem hiding this comment.
LGTM thanks @csoulios. Nit: would you revert the whitespace changes?
| ) { | ||
| super(GetRollupCapsAction.NAME, transportService, actionFilters, GetRollupCapsAction.Request::new, ThreadPool.Names.MANAGEMENT); | ||
| this.clusterService = clusterService; | ||
| this.threadPool = threadPool; |
There was a problem hiding this comment.
nit: I don't think we need to capture this, it's not used AFAICT. Also it's available already as transportService.getThreadPool(), no need to add a constructor arg.
| ThreadPool.Names.MANAGEMENT | ||
| ); | ||
| this.clusterService = clusterService; | ||
| this.threadPool = threadPool; |
There was a problem hiding this comment.
nit: likewise here, I don't think we need to capture this, it's not used AFAICT. Also it's available already as transportService.getThreadPool(), no need to add a constructor arg.
| ThreadPool threadPool, | ||
| ActionFilters actionFilters | ||
| ) { | ||
| super(GetRollupCapsAction.NAME, transportService, actionFilters, GetRollupCapsAction.Request::new, ThreadPool.Names.MANAGEMENT); |
There was a problem hiding this comment.
@DaveCTurner should I use ThreadPool.Names.SAME here, like we do in
There was a problem hiding this comment.
In practice it doesn't matter in this case because this action is only ever executed from REST actions, which ignore this parameter, but yes it'd be good to follow the same pattern anyway. Please also include the comment about #97916.
DaveCTurner
left a comment
There was a problem hiding this comment.
LGTM still (even better than before)
As discussed in #92179,
RestGetRollupCapsActionandRestGetRollupIndexCapsActioninvoke expensiveGetRollupCapsActionon transport threads.This PR modifies the rest actions so that they invoke
GetRollupCapsActionin a separate threadFixes #92179
Relates to #97916