Skip to content

Commit f11d853

Browse files
finnegancarrollvinaykpud
authored andcommitted
Ignore attribute awareness when custom preference is set (opensearch-project#18848)
Signed-off-by: Finn Carroll <[email protected]>
1 parent 6c6e262 commit f11d853

File tree

3 files changed

+74
-3
lines changed

3 files changed

+74
-3
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
9595
- Field-level ignore_malformed should override index-level setting ([#18706](https://github.com/opensearch-project/OpenSearch/pull/18706))
9696
- Fixed Staggered merge - load average replace with AverageTrackers, some Default thresholds modified ([#18666](https://github.com/opensearch-project/OpenSearch/pull/18666))
9797
- Use `new SecureRandom()` to avoid blocking ([18729](https://github.com/opensearch-project/OpenSearch/issues/18729))
98+
- Ignore awareness attributes when a custom preference string is included with a search request ([#18848](https://github.com/opensearch-project/OpenSearch/pull/18848))
9899
- Use ScoreDoc instead of FieldDoc when creating TopScoreDocCollectorManager to avoid unnecessary conversion ([#18802](https://github.com/opensearch-project/OpenSearch/pull/18802))
99100
- Fix leafSorter optimization for ReadOnlyEngine and NRTReplicationEngine ([#18639](https://github.com/opensearch-project/OpenSearch/pull/18639))
100101

server/src/internalClusterTest/java/org/opensearch/cluster/routing/WeightedRoutingIT.java

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import org.opensearch.action.admin.cluster.shards.routing.weighted.get.ClusterGetWeightedRoutingResponse;
1515
import org.opensearch.action.admin.cluster.shards.routing.weighted.put.ClusterPutWeightedRoutingResponse;
1616
import org.opensearch.action.admin.cluster.state.ClusterStateRequest;
17+
import org.opensearch.action.search.SearchResponse;
1718
import org.opensearch.cluster.health.ClusterHealthStatus;
1819
import org.opensearch.common.settings.Settings;
1920
import org.opensearch.core.rest.RestStatus;
@@ -28,13 +29,16 @@
2829
import java.io.IOException;
2930
import java.util.Arrays;
3031
import java.util.Collection;
32+
import java.util.HashMap;
3133
import java.util.HashSet;
3234
import java.util.List;
3335
import java.util.Map;
3436
import java.util.Set;
3537
import java.util.stream.Collectors;
3638
import java.util.stream.Stream;
3739

40+
import static org.opensearch.index.query.QueryBuilders.matchAllQuery;
41+
import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;
3842
import static org.hamcrest.Matchers.equalTo;
3943

4044
@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 0, minNumDataNodes = 3)
@@ -857,4 +861,72 @@ public void testReadWriteWeightedRoutingMetadataOnNodeRestart() throws Exception
857861
);
858862

859863
}
864+
865+
/**
866+
* https://github.com/opensearch-project/OpenSearch/issues/18817
867+
* For regression in custom string query preference with awareness attributes enabled.
868+
* We expect preference will consistently route to the same shard replica. However, when awareness attributes
869+
* are configured this does not hold.
870+
*/
871+
public void testCustomPreferenceShardIdCombination() {
872+
// Configure cluster with awareness attributes
873+
Settings commonSettings = Settings.builder()
874+
.put("cluster.routing.allocation.awareness.attributes", "rack")
875+
.put("cluster.routing.allocation.awareness.force.rack.values", "rack1,rack2")
876+
.put("cluster.routing.use_adaptive_replica_selection", false)
877+
.put("cluster.search.ignore_awareness_attributes", false)
878+
.build();
879+
880+
// Start cluster
881+
internalCluster().startClusterManagerOnlyNode(commonSettings);
882+
internalCluster().startDataOnlyNodes(2, Settings.builder().put(commonSettings).put("node.attr.rack", "rack1").build());
883+
internalCluster().startDataOnlyNodes(2, Settings.builder().put(commonSettings).put("node.attr.rack", "rack2").build());
884+
885+
ensureStableCluster(5);
886+
ensureGreen();
887+
888+
// Create index with specific shard configuration
889+
assertAcked(
890+
prepareCreate("test_index").setSettings(
891+
Settings.builder().put("index.number_of_shards", 6).put("index.number_of_replicas", 1).build()
892+
)
893+
);
894+
895+
ensureGreen("test_index");
896+
897+
// Index test documents
898+
for (int i = 0; i < 30; i++) {
899+
client().prepareIndex("test_index").setId(String.valueOf(i)).setSource("field", "value" + i).get();
900+
}
901+
refreshAndWaitForReplication("test_index");
902+
903+
/*
904+
Execute the same match all query with custom string preference.
905+
For each search and each shard in the response we record the node on which the shard was located.
906+
Given the custom string preference, we expect each shard or each search should report the exact same node id.
907+
Otherwise, the custom string pref is not producing consistent shard routing.
908+
*/
909+
Map<String, Set<String>> shardToNodes = new HashMap<>();
910+
for (int i = 0; i < 20; i++) {
911+
SearchResponse response = client().prepareSearch("test_index")
912+
.setQuery(matchAllQuery())
913+
.setPreference("test_preference_123")
914+
.setSize(30)
915+
.get();
916+
for (int j = 0; j < response.getHits().getHits().length; j++) {
917+
String shardId = response.getHits().getAt(j).getShard().getShardId().toString();
918+
String nodeId = response.getHits().getAt(j).getShard().getNodeId();
919+
shardToNodes.computeIfAbsent(shardId, k -> new HashSet<>()).add(nodeId);
920+
}
921+
}
922+
923+
/*
924+
If more than one node was responsible for serving a request for a given shard,
925+
then there was a regression in the custom preference string.
926+
*/
927+
logger.info("--> shard to node mappings: {}", shardToNodes);
928+
for (Map.Entry<String, Set<String>> entry : shardToNodes.entrySet()) {
929+
assertThat("Shard " + entry.getKey() + " should consistently route to the same node", entry.getValue().size(), equalTo(1));
930+
}
931+
}
860932
}

server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -446,10 +446,8 @@ private ShardIterator preferenceActiveShardIterator(
446446
isFailOpenEnabled,
447447
routingHash
448448
);
449-
} else if (ignoreAwarenessAttributes()) {
450-
return indexShard.activeInitializingShardsIt(routingHash);
451449
} else {
452-
return indexShard.preferAttributesActiveInitializingShardsIt(awarenessAttributes, nodes, routingHash);
450+
return indexShard.activeInitializingShardsIt(routingHash);
453451
}
454452
}
455453

0 commit comments

Comments
 (0)