Skip to content

Commit

Permalink
Made hybrid search active by default by flipping feature flag default (
Browse files Browse the repository at this point in the history
…#292)

Signed-off-by: Martin Gaievski <gaievski@amazon.com>
(cherry picked from commit 174f2c9)
  • Loading branch information
martin-gaievski authored Sep 8, 2023
1 parent 5bf36ed commit 8484be9
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 26 deletions.
4 changes: 0 additions & 4 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -253,10 +253,6 @@ testClusters.integTest {
// Increase heap size from default of 512mb to 1gb. When heap size is 512mb, our integ tests sporadically fail due
// to ml-commons memory circuit breaker exception
jvmArgs("-Xms1g", "-Xmx1g")

// enable features for testing
// hybrid search
systemProperty('plugins.neural_search.hybrid_search_enabled', 'true')
}

// Remote Integration Tests
Expand Down
24 changes: 13 additions & 11 deletions src/main/java/org/opensearch/neuralsearch/plugin/NeuralSearch.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

package org.opensearch.neuralsearch.plugin;

import static org.opensearch.neuralsearch.settings.NeuralSearchSettings.NEURAL_SEARCH_HYBRID_SEARCH_ENABLED;
import static org.opensearch.neuralsearch.settings.NeuralSearchSettings.NEURAL_SEARCH_HYBRID_SEARCH_DISABLED;

import java.util.Arrays;
import java.util.Collection;
Expand Down Expand Up @@ -99,16 +99,18 @@ public Map<String, Processor.Factory> getProcessors(Processor.Parameters paramet

@Override
public Optional<QueryPhaseSearcher> getQueryPhaseSearcher() {
if (FeatureFlags.isEnabled(NEURAL_SEARCH_HYBRID_SEARCH_ENABLED.getKey())) {
log.info("Registering hybrid query phase searcher with feature flag [{}]", NEURAL_SEARCH_HYBRID_SEARCH_ENABLED.getKey());
return Optional.of(new HybridQueryPhaseSearcher());
// we're using "is_disabled" flag as there are no proper implementation of FeatureFlags.isDisabled(). Both
// cases when flag is not set or it is "false" are interpretted in the same way. In such case core is reading
// the actual value from settings.
if (FeatureFlags.isEnabled(NEURAL_SEARCH_HYBRID_SEARCH_DISABLED.getKey())) {
log.info(
"Not registering hybrid query phase searcher because feature flag [{}] is disabled",
NEURAL_SEARCH_HYBRID_SEARCH_DISABLED.getKey()
);
return Optional.empty();
}
log.info(
"Not registering hybrid query phase searcher because feature flag [{}] is disabled",
NEURAL_SEARCH_HYBRID_SEARCH_ENABLED.getKey()
);
// we want feature be disabled by default due to risk of colliding and breaking concurrent search in core
return Optional.empty();
log.info("Registering hybrid query phase searcher with feature flag [{}]", NEURAL_SEARCH_HYBRID_SEARCH_DISABLED.getKey());
return Optional.of(new HybridQueryPhaseSearcher());
}

@Override
Expand All @@ -123,6 +125,6 @@ public Map<String, org.opensearch.search.pipeline.Processor.Factory<SearchPhaseR

@Override
public List<Setting<?>> getSettings() {
return List.of(NEURAL_SEARCH_HYBRID_SEARCH_ENABLED);
return List.of(NEURAL_SEARCH_HYBRID_SEARCH_DISABLED);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ public final class NeuralSearchSettings {
* Currently query phase searcher added with hybrid search will conflict with concurrent search in core.
* Once that problem is resolved this feature flag can be removed.
*/
public static final Setting<Boolean> NEURAL_SEARCH_HYBRID_SEARCH_ENABLED = Setting.boolSetting(
"plugins.neural_search.hybrid_search_enabled",
public static final Setting<Boolean> NEURAL_SEARCH_HYBRID_SEARCH_DISABLED = Setting.boolSetting(
"plugins.neural_search.hybrid_search_disabled",
false,
Setting.Property.NodeScope
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,18 +38,18 @@ public void testQuerySpecs() {

public void testQueryPhaseSearcher() {
NeuralSearch plugin = new NeuralSearch();
Optional<QueryPhaseSearcher> queryPhaseSearcher = plugin.getQueryPhaseSearcher();

assertNotNull(queryPhaseSearcher);
assertTrue(queryPhaseSearcher.isEmpty());

initFeatureFlags();

Optional<QueryPhaseSearcher> queryPhaseSearcherWithFeatureFlagDisabled = plugin.getQueryPhaseSearcher();

assertNotNull(queryPhaseSearcherWithFeatureFlagDisabled);
assertFalse(queryPhaseSearcherWithFeatureFlagDisabled.isEmpty());
assertTrue(queryPhaseSearcherWithFeatureFlagDisabled.get() instanceof HybridQueryPhaseSearcher);

initFeatureFlags();

Optional<QueryPhaseSearcher> queryPhaseSearcher = plugin.getQueryPhaseSearcher();

assertNotNull(queryPhaseSearcher);
assertTrue(queryPhaseSearcher.isEmpty());
}

public void testProcessors() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import static java.util.Collections.emptyMap;
import static java.util.Collections.singletonMap;
import static java.util.stream.Collectors.toList;
import static org.opensearch.neuralsearch.settings.NeuralSearchSettings.NEURAL_SEARCH_HYBRID_SEARCH_ENABLED;
import static org.opensearch.neuralsearch.settings.NeuralSearchSettings.NEURAL_SEARCH_HYBRID_SEARCH_DISABLED;

import java.io.IOException;
import java.util.Arrays;
Expand Down Expand Up @@ -227,6 +227,6 @@ public float getMaxScore(int upTo) {

@SuppressForbidden(reason = "manipulates system properties for testing")
protected static void initFeatureFlags() {
System.setProperty(NEURAL_SEARCH_HYBRID_SEARCH_ENABLED.getKey(), "true");
System.setProperty(NEURAL_SEARCH_HYBRID_SEARCH_DISABLED.getKey(), "true");
}
}

0 comments on commit 8484be9

Please sign in to comment.