Skip to content

Commit

Permalink
Merge branch 'main' into codeowners-for-kibana-rd
Browse files Browse the repository at this point in the history
  • Loading branch information
elasticmachine authored Jul 12, 2023
2 parents f5670b6 + 4cf74e5 commit dbd841a
Show file tree
Hide file tree
Showing 37 changed files with 538 additions and 220 deletions.
5 changes: 5 additions & 0 deletions docs/changelog/97581.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 97581
summary: Add Setting to optionally use mmap for shared cache IO
area: Snapshot/Restore
type: enhancement
issues: []
5 changes: 5 additions & 0 deletions docs/changelog/97594.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 97594
summary: Improve wildcard query and terms query rewrite
area: Search
type: enhancement
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ private static DataStreamLifecycle.Downsampling randomDownsampling() {
case 0 -> null;
case 1 -> DataStreamLifecycle.Downsampling.NULL;
default -> {
var count = randomIntBetween(0, 10);
var count = randomIntBetween(0, 9);
List<DataStreamLifecycle.Downsampling.Round> rounds = new ArrayList<>();
var previous = new DataStreamLifecycle.Downsampling.Round(
TimeValue.timeValueDays(randomIntBetween(1, 365)),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -368,10 +368,13 @@ private static boolean assertNotCalledFromClusterStateApplier() {
final String className = element.getClassName();
final String methodName = element.getMethodName();
if (className.equals(ClusterStateObserver.class.getName())) {
// people may start an observer from an applier
// it's legitimate to start a ClusterStateObserver on the applier thread, since this class handles lost updates
return true;
} else if (className.equals(ClusterApplierService.class.getName()) && methodName.equals("callClusterStateAppliers")) {
throw new AssertionError("should not be called by a cluster state applier: the applied state is not yet available");
throw new AssertionError("""
On the cluster applier thread you must use ClusterChangedEvent#state() and ClusterChangedEvent#previousState() \
instead of ClusterApplierService#state(). It is almost certainly a bug to read the latest-applied state from \
within a cluster applier since the new state has been committed at this point but is not yet applied.""");
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,10 @@ public final Query internalTermQueryCaseInsensitive(Object value, QueryRewriteCo

@Override
public final Query termsQuery(Collection<?> values, SearchExecutionContext context) {
return innerTermsQuery(values, context);
}

public final Query innerTermsQuery(Collection<?> values, QueryRewriteContext context) {
for (Object value : values) {
String pattern = valueToString(value);
if (matches(pattern, false, context)) {
Expand Down Expand Up @@ -117,6 +121,10 @@ public final Query wildcardQuery(
boolean caseInsensitive,
SearchExecutionContext context
) {
return wildcardQuery(value, caseInsensitive, context);
}

public final Query wildcardQuery(String value, boolean caseInsensitive, QueryRewriteContext context) {
if (matches(value, caseInsensitive, context)) {
return Queries.newMatchAllQuery();
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ public String getWriteableName() {
protected QueryBuilder doIndexMetadataRewrite(QueryRewriteContext context) throws IOException {
MappedFieldType fieldType = context.getFieldType(this.fieldName);
if (fieldType == null) {
return new MatchNoneQueryBuilder("The \"" + getName() + "\" query was rewritten to a \"match_none\" query.");
return new MatchNoneQueryBuilder("The \"" + getName() + "\" query is against a field that does not exist");
} else if (fieldType instanceof ConstantFieldType constantFieldType) {
// This logic is correct for all field types, but by only applying it to constant
// fields we also have the guarantee that it doesn't perform I/O, which is important
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ protected void addExtraXContent(XContentBuilder builder, Params params) throws I
protected QueryBuilder doIndexMetadataRewrite(QueryRewriteContext context) throws IOException {
MappedFieldType fieldType = context.getFieldType(this.fieldName);
if (fieldType == null) {
return new MatchNoneQueryBuilder("The \"" + getName() + "\" query was rewritten to a \"match_none\" query.");
return new MatchNoneQueryBuilder("The \"" + getName() + "\" query is against a field that does not exist");
} else if (fieldType instanceof ConstantFieldType constantFieldType) {
// This logic is correct for all field types, but by only applying it to constant
// fields we also have the guarantee that it doesn't perform I/O, which is important
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ protected boolean doEquals(TermsQueryBuilder other) {
}

@Override
protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) {
protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) throws IOException {
if (supplier != null) {
return supplier.get() == null ? this : new TermsQueryBuilder(this.fieldName, supplier.get());
} else if (this.termsLookup != null) {
Expand All @@ -387,27 +387,27 @@ protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) {
if (values == null || values.isEmpty()) {
return new MatchNoneQueryBuilder("The \"" + getName() + "\" query was rewritten to a \"match_none\" query.");
}
return super.doRewrite(queryRewriteContext);
}

SearchExecutionContext context = queryRewriteContext.convertToSearchExecutionContext();
if (context != null) {
MappedFieldType fieldType = context.getFieldType(this.fieldName);
if (fieldType == null) {
@Override
protected QueryBuilder doIndexMetadataRewrite(QueryRewriteContext context) throws IOException {
MappedFieldType fieldType = context.getFieldType(this.fieldName);
if (fieldType == null) {
return new MatchNoneQueryBuilder("The \"" + getName() + "\" query is against a field that does not exist");
} else if (fieldType instanceof ConstantFieldType constantFieldType) {
// This logic is correct for all field types, but by only applying it to constant
// fields we also have the guarantee that it doesn't perform I/O, which is important
// since rewrites might happen on a network thread.
Query query = constantFieldType.innerTermsQuery(values, context);
if (query instanceof MatchAllDocsQuery) {
return new MatchAllQueryBuilder();
} else if (query instanceof MatchNoDocsQuery) {
return new MatchNoneQueryBuilder("The \"" + getName() + "\" query was rewritten to a \"match_none\" query.");
} else if (fieldType instanceof ConstantFieldType) {
// This logic is correct for all field types, but by only applying it to constant
// fields we also have the guarantee that it doesn't perform I/O, which is important
// since rewrites might happen on a network thread.
Query query = fieldType.termsQuery(values, context);
if (query instanceof MatchAllDocsQuery) {
return new MatchAllQueryBuilder();
} else if (query instanceof MatchNoDocsQuery) {
return new MatchNoneQueryBuilder("The \"" + getName() + "\" query was rewritten to a \"match_none\" query.");
} else {
assert false : "Constant fields must produce match-all or match-none queries, got " + query;
}
} else {
assert false : "Constant fields must produce match-all or match-none queries, got " + query;
}
}

return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,28 +198,24 @@ public static WildcardQueryBuilder fromXContent(XContentParser parser) throws IO
}

@Override
protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) throws IOException {
SearchExecutionContext context = queryRewriteContext.convertToSearchExecutionContext();
if (context != null) {
MappedFieldType fieldType = context.getFieldType(this.fieldName);
if (fieldType == null) {
protected QueryBuilder doIndexMetadataRewrite(QueryRewriteContext context) throws IOException {
MappedFieldType fieldType = context.getFieldType(this.fieldName);
if (fieldType == null) {
return new MatchNoneQueryBuilder("The \"" + getName() + "\" query is against a field that does not exist");
} else if (fieldType instanceof ConstantFieldType constantFieldType) {
// This logic is correct for all field types, but by only applying it to constant
// fields we also have the guarantee that it doesn't perform I/O, which is important
// since rewrites might happen on a network thread.
Query query = constantFieldType.wildcardQuery(value, caseInsensitive, context); // the rewrite method doesn't matter
if (query instanceof MatchAllDocsQuery) {
return new MatchAllQueryBuilder();
} else if (query instanceof MatchNoDocsQuery) {
return new MatchNoneQueryBuilder("The \"" + getName() + "\" query was rewritten to a \"match_none\" query.");
} else if (fieldType instanceof ConstantFieldType) {
// This logic is correct for all field types, but by only applying it to constant
// fields we also have the guarantee that it doesn't perform I/O, which is important
// since rewrites might happen on a network thread.
Query query = fieldType.wildcardQuery(value, null, caseInsensitive, context); // the rewrite method doesn't matter
if (query instanceof MatchAllDocsQuery) {
return new MatchAllQueryBuilder();
} else if (query instanceof MatchNoDocsQuery) {
return new MatchNoneQueryBuilder("The \"" + getName() + "\" query was rewritten to a \"match_none\" query.");
} else {
assert false : "Constant fields must produce match-all or match-none queries, got " + query;
}
} else {
assert false : "Constant fields must produce match-all or match-none queries, got " + query;
}
}

return super.doRewrite(queryRewriteContext);
return this;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -351,9 +351,15 @@ public void testClusterStateApplierCantSampleClusterState() throws InterruptedEx
clusterApplierService.state();
error.set(new AssertionError("successfully sampled state"));
} catch (AssertionError e) {
if (e.getMessage().contains("should not be called by a cluster state applier") == false) {
error.set(e);
// NB not a string constant shared between implementation and test, because the content of this message is important and
// mustn't be changed inadvertently.
if (e.getMessage().equals("""
On the cluster applier thread you must use ClusterChangedEvent#state() and ClusterChangedEvent#previousState() instead \
of ClusterApplierService#state(). It is almost certainly a bug to read the latest-applied state from within a cluster \
applier since the new state has been committed at this point but is not yet applied.""")) {
return;
}
error.set(e);
}
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -293,17 +293,19 @@ public void testSerializationFailsUnlessFetched() throws IOException {

public void testRewriteIndexQueryToMatchNone() throws IOException {
TermsQueryBuilder query = new TermsQueryBuilder("_index", "does_not_exist", "also_does_not_exist");
SearchExecutionContext searchExecutionContext = createSearchExecutionContext();
QueryBuilder rewritten = query.rewrite(searchExecutionContext);
assertThat(rewritten, instanceOf(MatchNoneQueryBuilder.class));
for (QueryRewriteContext context : new QueryRewriteContext[] { createSearchExecutionContext(), createQueryRewriteContext() }) {
QueryBuilder rewritten = query.rewrite(context);
assertThat(rewritten, instanceOf(MatchNoneQueryBuilder.class));
}
}

public void testRewriteIndexQueryToNotMatchNone() throws IOException {
// At least one name is good
TermsQueryBuilder query = new TermsQueryBuilder("_index", "does_not_exist", getIndex().getName());
SearchExecutionContext searchExecutionContext = createSearchExecutionContext();
QueryBuilder rewritten = query.rewrite(searchExecutionContext);
assertThat(rewritten, instanceOf(MatchAllQueryBuilder.class));
for (QueryRewriteContext context : new QueryRewriteContext[] { createSearchExecutionContext(), createQueryRewriteContext() }) {
QueryBuilder rewritten = query.rewrite(context);
assertThat(rewritten, instanceOf(MatchAllQueryBuilder.class));
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,18 +141,20 @@ public void testParseFailsWithMultipleFields() throws IOException {

public void testRewriteIndexQueryToMatchNone() throws IOException {
WildcardQueryBuilder query = new WildcardQueryBuilder("_index", "does_not_exist");
SearchExecutionContext searchExecutionContext = createSearchExecutionContext();
QueryBuilder rewritten = query.rewrite(searchExecutionContext);
assertThat(rewritten, instanceOf(MatchNoneQueryBuilder.class));
for (QueryRewriteContext context : new QueryRewriteContext[] { createSearchExecutionContext(), createQueryRewriteContext() }) {
QueryBuilder rewritten = query.rewrite(context);
assertThat(rewritten, instanceOf(MatchNoneQueryBuilder.class));
}
}

public void testRewriteIndexQueryNotMatchNone() throws IOException {
String fullIndexName = getIndex().getName();
String firstHalfOfIndexName = fullIndexName.substring(0, fullIndexName.length() / 2);
WildcardQueryBuilder query = new WildcardQueryBuilder("_index", firstHalfOfIndexName + "*");
SearchExecutionContext searchExecutionContext = createSearchExecutionContext();
QueryBuilder rewritten = query.rewrite(searchExecutionContext);
assertThat(rewritten, instanceOf(MatchAllQueryBuilder.class));
for (QueryRewriteContext context : new QueryRewriteContext[] { createSearchExecutionContext(), createQueryRewriteContext() }) {
QueryBuilder rewritten = query.rewrite(context);
assertThat(rewritten, instanceOf(MatchAllQueryBuilder.class));
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ public List<Setting<?>> getSettings() {
SharedBlobCacheService.SHARED_CACHE_RECOVERY_RANGE_SIZE_SETTING,
SharedBlobCacheService.SHARED_CACHE_MAX_FREQ_SETTING,
SharedBlobCacheService.SHARED_CACHE_DECAY_INTERVAL_SETTING,
SharedBlobCacheService.SHARED_CACHE_MIN_TIME_DELTA_SETTING
SharedBlobCacheService.SHARED_CACHE_MIN_TIME_DELTA_SETTING,
SharedBlobCacheService.SHARED_CACHE_MMAP
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,12 @@ public void validate(ByteSizeValue value, Map<Setting<?>, Object> settings, bool
Setting.Property.NodeScope
);

public static final Setting<Boolean> SHARED_CACHE_MMAP = Setting.boolSetting(
SHARED_CACHE_SETTINGS_PREFIX + "mmap",
false,
Setting.Property.NodeScope
);

private static final Logger logger = LogManager.getLogger(SharedBlobCacheService.class);

private final ConcurrentHashMap<RegionKey<KeyType>, Entry<CacheFileRegion>> keyMapping;
Expand Down Expand Up @@ -300,7 +306,14 @@ public SharedBlobCacheService(NodeEnvironment environment, Settings settings, Th
this.minTimeDelta = SHARED_CACHE_MIN_TIME_DELTA_SETTING.get(settings).millis();
freqs = new Entry[maxFreq];
try {
sharedBytes = new SharedBytes(numRegions, regionSize, environment, writeBytes::add, readBytes::add);
sharedBytes = new SharedBytes(
numRegions,
regionSize,
environment,
writeBytes::add,
readBytes::add,
SHARED_CACHE_MMAP.get(settings)
);
} catch (IOException e) {
throw new UncheckedIOException(e);
}
Expand Down
Loading

0 comments on commit dbd841a

Please sign in to comment.