From b4ed1bf9903bc673f23e22a96d4fd5aaf7b0764c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?danielhuang=28=E9=BB=84=E5=8D=8E=29?= Date: Sun, 17 May 2020 22:18:49 +0800 Subject: [PATCH 1/3] optimize complicated if-else in routing table --- .../cluster/routing/RoutingTable.java | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/RoutingTable.java b/server/src/main/java/org/elasticsearch/cluster/routing/RoutingTable.java index cb3a82f80efed..62d0c6203062e 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/RoutingTable.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/RoutingTable.java @@ -29,6 +29,7 @@ import org.elasticsearch.cluster.metadata.Metadata; import org.elasticsearch.cluster.routing.RecoverySource.SnapshotRecoverySource; import org.elasticsearch.common.Nullable; +import org.elasticsearch.common.SuppressForbidden; import org.elasticsearch.common.collect.ImmutableOpenMap; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; @@ -456,6 +457,7 @@ private static void addShard(final Map indexR * @param indices the indices to update the number of replicas for * @return the builder */ + @SuppressForbidden(reason = "Argument to Math.abs() is definitely not Long.MIN_VALUE") public Builder updateNumberOfReplicas(final int numberOfReplicas, final String[] indices) { if (indicesRouting == null) { throw new IllegalStateException("once build is called the builder cannot be reused"); @@ -472,19 +474,12 @@ public Builder updateNumberOfReplicas(final int numberOfReplicas, final String[] for (IndexShardRoutingTable indexShardRoutingTable : indexRoutingTable) { builder.addIndexShard(indexShardRoutingTable); } - if (currentNumberOfReplicas < numberOfReplicas) { - // now, add "empty" ones - for (int i = 0; i < (numberOfReplicas - currentNumberOfReplicas); i++) { + int delta = Math.abs(numberOfReplicas - currentNumberOfReplicas); + for (int i = 0; i < delta; i++) { + if (currentNumberOfReplicas < numberOfReplicas) { builder.addReplica(); - } - } else if (currentNumberOfReplicas > numberOfReplicas) { - int delta = currentNumberOfReplicas - numberOfReplicas; - if (delta <= 0) { - // ignore, can't remove below the current one... } else { - for (int i = 0; i < delta; i++) { - builder.removeReplica(); - } + builder.removeReplica(); } } indicesRouting.put(index, builder.build()); From efd2f21a7574487be3a0d703ded3b8d2661f6032 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?danielhuang=28=E9=BB=84=E5=8D=8E=29?= Date: Mon, 18 May 2020 10:04:26 +0800 Subject: [PATCH 2/3] precompute delta and distinguish add and remove --- .../org/elasticsearch/cluster/routing/RoutingTable.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/RoutingTable.java b/server/src/main/java/org/elasticsearch/cluster/routing/RoutingTable.java index 62d0c6203062e..8afc2856311c0 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/RoutingTable.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/RoutingTable.java @@ -475,10 +475,13 @@ public Builder updateNumberOfReplicas(final int numberOfReplicas, final String[] builder.addIndexShard(indexShardRoutingTable); } int delta = Math.abs(numberOfReplicas - currentNumberOfReplicas); - for (int i = 0; i < delta; i++) { - if (currentNumberOfReplicas < numberOfReplicas) { + if (currentNumberOfReplicas < numberOfReplicas) { + // now, add "empty" ones + for (int i = 0; i < delta; i++) { builder.addReplica(); - } else { + } + } else if (currentNumberOfReplicas > numberOfReplicas) { + for (int i = 0; i < delta; i++) { builder.removeReplica(); } } From 2bb15b2930b74a139b0267b87dc2d3b53e83ec60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?danielhuang=28=E9=BB=84=E5=8D=8E=29?= Date: Mon, 18 May 2020 17:21:51 +0800 Subject: [PATCH 3/3] remove abs --- .../org/elasticsearch/cluster/routing/RoutingTable.java | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/RoutingTable.java b/server/src/main/java/org/elasticsearch/cluster/routing/RoutingTable.java index 8afc2856311c0..35b19a9247e02 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/RoutingTable.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/RoutingTable.java @@ -29,7 +29,6 @@ import org.elasticsearch.cluster.metadata.Metadata; import org.elasticsearch.cluster.routing.RecoverySource.SnapshotRecoverySource; import org.elasticsearch.common.Nullable; -import org.elasticsearch.common.SuppressForbidden; import org.elasticsearch.common.collect.ImmutableOpenMap; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; @@ -457,7 +456,6 @@ private static void addShard(final Map indexR * @param indices the indices to update the number of replicas for * @return the builder */ - @SuppressForbidden(reason = "Argument to Math.abs() is definitely not Long.MIN_VALUE") public Builder updateNumberOfReplicas(final int numberOfReplicas, final String[] indices) { if (indicesRouting == null) { throw new IllegalStateException("once build is called the builder cannot be reused"); @@ -474,14 +472,13 @@ public Builder updateNumberOfReplicas(final int numberOfReplicas, final String[] for (IndexShardRoutingTable indexShardRoutingTable : indexRoutingTable) { builder.addIndexShard(indexShardRoutingTable); } - int delta = Math.abs(numberOfReplicas - currentNumberOfReplicas); if (currentNumberOfReplicas < numberOfReplicas) { // now, add "empty" ones - for (int i = 0; i < delta; i++) { + for (int i = 0; i < (numberOfReplicas - currentNumberOfReplicas); i++) { builder.addReplica(); } } else if (currentNumberOfReplicas > numberOfReplicas) { - for (int i = 0; i < delta; i++) { + for (int i = 0; i < (currentNumberOfReplicas - numberOfReplicas); i++) { builder.removeReplica(); } }