Skip to content

Commit e686609

Browse files
authored
Move routing calculation (#79394)
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.
1 parent f758648 commit e686609

File tree

5 files changed

+26
-9
lines changed

5 files changed

+26
-9
lines changed

server/src/main/java/org/elasticsearch/action/DocWriteRequest.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import org.elasticsearch.action.index.IndexRequest;
1313
import org.elasticsearch.action.support.IndicesOptions;
1414
import org.elasticsearch.action.update.UpdateRequest;
15+
import org.elasticsearch.cluster.routing.IndexRouting;
1516
import org.elasticsearch.common.io.stream.StreamInput;
1617
import org.elasticsearch.common.io.stream.StreamOutput;
1718
import org.elasticsearch.common.lucene.uid.Versions;
@@ -140,6 +141,11 @@ public interface DocWriteRequest<T> extends IndicesRequest, Accountable {
140141
*/
141142
boolean isRequireAlias();
142143

144+
/**
145+
* Pick the appropriate shard id to receive this request.
146+
*/
147+
int route(IndexRouting indexRouting);
148+
143149
/**
144150
* Requested operation type to perform on the document
145151
*/

server/src/main/java/org/elasticsearch/action/bulk/TransportBulkAction.java

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -525,7 +525,6 @@ protected void doRun() {
525525

526526
IndexRouting indexRouting = concreteIndices.routing(concreteIndex);
527527

528-
int shardId;
529528
switch (docWriteRequest.opType()) {
530529
case CREATE:
531530
case INDEX:
@@ -534,24 +533,17 @@ protected void doRun() {
534533
IndexRequest indexRequest = (IndexRequest) docWriteRequest;
535534
indexRequest.resolveRouting(metadata);
536535
indexRequest.process();
537-
shardId = indexRouting.indexShard(
538-
docWriteRequest.id(),
539-
docWriteRequest.routing(),
540-
indexRequest.getContentType(),
541-
indexRequest.source()
542-
);
543536
break;
544537
case UPDATE:
545538
docWriteRequest.routing(metadata.resolveWriteIndexRouting(docWriteRequest.routing(), docWriteRequest.index()));
546-
shardId = indexRouting.updateShard(docWriteRequest.id(), docWriteRequest.routing());
547539
break;
548540
case DELETE:
549541
docWriteRequest.routing(metadata.resolveWriteIndexRouting(docWriteRequest.routing(), docWriteRequest.index()));
550-
shardId = indexRouting.deleteShard(docWriteRequest.id(), docWriteRequest.routing());
551542
break;
552543
default:
553544
throw new AssertionError("request type not supported: [" + docWriteRequest.opType() + "]");
554545
}
546+
int shardId = docWriteRequest.route(indexRouting);
555547
List<BulkItemRequest> shardRequests = requestsByShard.computeIfAbsent(
556548
new ShardId(concreteIndex, shardId),
557549
shard -> new ArrayList<>()

server/src/main/java/org/elasticsearch/action/delete/DeleteRequest.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import org.elasticsearch.action.CompositeIndicesRequest;
1515
import org.elasticsearch.action.DocWriteRequest;
1616
import org.elasticsearch.action.support.replication.ReplicatedWriteRequest;
17+
import org.elasticsearch.cluster.routing.IndexRouting;
1718
import org.elasticsearch.common.Strings;
1819
import org.elasticsearch.common.io.stream.StreamInput;
1920
import org.elasticsearch.common.io.stream.StreamOutput;
@@ -232,6 +233,11 @@ public boolean isRequireAlias() {
232233
return false;
233234
}
234235

236+
@Override
237+
public int route(IndexRouting indexRouting) {
238+
return indexRouting.deleteShard(id, routing);
239+
}
240+
235241
@Override
236242
public void writeTo(StreamOutput out) throws IOException {
237243
super.writeTo(out);

server/src/main/java/org/elasticsearch/action/index/IndexRequest.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import org.elasticsearch.action.support.replication.ReplicationRequest;
2020
import org.elasticsearch.client.Requests;
2121
import org.elasticsearch.cluster.metadata.Metadata;
22+
import org.elasticsearch.cluster.routing.IndexRouting;
2223
import org.elasticsearch.common.UUIDs;
2324
import org.elasticsearch.common.bytes.BytesArray;
2425
import org.elasticsearch.common.bytes.BytesReference;
@@ -722,6 +723,12 @@ public boolean isRequireAlias() {
722723
return requireAlias;
723724
}
724725

726+
@Override
727+
public int route(IndexRouting indexRouting) {
728+
assert id != null : "route must be called after process";
729+
return indexRouting.indexShard(id, routing, contentType, source);
730+
}
731+
725732
public IndexRequest setRequireAlias(boolean requireAlias) {
726733
this.requireAlias = requireAlias;
727734
return this;

server/src/main/java/org/elasticsearch/action/update/UpdateRequest.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import org.elasticsearch.action.support.WriteRequest;
1818
import org.elasticsearch.action.support.replication.ReplicationRequest;
1919
import org.elasticsearch.action.support.single.instance.InstanceShardOperationRequest;
20+
import org.elasticsearch.cluster.routing.IndexRouting;
2021
import org.elasticsearch.common.Strings;
2122
import org.elasticsearch.common.bytes.BytesReference;
2223
import org.elasticsearch.common.io.stream.StreamInput;
@@ -830,6 +831,11 @@ public boolean isRequireAlias() {
830831
return requireAlias;
831832
}
832833

834+
@Override
835+
public int route(IndexRouting indexRouting) {
836+
return indexRouting.updateShard(id, routing);
837+
}
838+
833839
public UpdateRequest setRequireAlias(boolean requireAlias) {
834840
this.requireAlias = requireAlias;
835841
return this;

0 commit comments

Comments
 (0)