Move routing calculation#79394
Conversation
nik9000
added
>non-issue
:Distributed/Distributed
elasticmachine
added
Team:Distributed
|
Pinging @elastic/es-distributed (Team:Distributed) |
|
Pinging @elastic/es-analytics-geo (Team:Analytics) |
This slightly moves the routing calculation out of a difficult to reason about `switch` statement and into real OO method implementation. Its a tiny tiny change but it makes me feel much better about it.
| // check if routing is required, if so, throw error if routing wasn't specified | ||
| if (docWriteRequest.routing() == null && metadata.routingRequired(concreteIndex.getName())) { | ||
| throw new RoutingMissingException(concreteIndex.getName(), docWriteRequest.id()); | ||
| } |
There was a problem hiding this comment.
I played around a little bit and most of this switch statement can be removed I think. But I want to do it bit by bit because its in an important bit of code.
There was a problem hiding this comment.
I opened #79472 which slices out a lot of the rest of this switch statement. I believe all that is left is the validation around data streams.
|
run elasticsearch-ci/part-2 |
henningandersen
left a comment
henningandersen
left a comment
There was a problem hiding this comment.
LGTM, this makes it much nicer.
| } | ||
|
|
||
| @Override | ||
| public int route(IndexRouting indexRouting) { |
There was a problem hiding this comment.
Maybe add comment here that resolveRouting and process must be called prior to this.
Also I think we can cement parts of that by asserting that id != null?
There was a problem hiding this comment.
👍
I'd sort of been hoping to fold those two methods together. But that is a thing for another time.
…uting_from_source_clean_1
…uting_from_source_clean_1
nik9000
added
auto-merge-without-approval
Now that we have elastic#79394 and elastic#79472 the switch statement in `TransportBulkAction` is mostly just assigning routing and doing some stuff with INDEX and UPDATE requests. This pulls the routing stuff into a single call and calls out the index-only stuff. Now, at least, it's clearer that only INDEX and UPDATE are special.
Now that we have #79394 and #79472 the switch statement in `TransportBulkAction` is mostly just assigning routing and doing some stuff with INDEX and UPDATE requests. This pulls the routing stuff into a single call and calls out the index-only stuff. Now, at least, it's clearer that only INDEX and UPDATE are special.
4 participants
This slightly moves the routing calculation out of a difficult to reason
about
switchstatement and into real OO method implementation. Its atiny tiny change but it makes me feel much better about it.