Add default sort for message.template_id field in logsdb indices #136571
Conversation
jordan-powers
added
>non-issue
Team:StorageEngine
:StorageEngine/Logs
|
Pinging @elastic/es-storage-engine (Team:StorageEngine) |
martijnvg
left a comment
martijnvg
left a comment
There was a problem hiding this comment.
Thanks Jordan, I left a few questions.
...gin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/LogsdbIndexModeSettingsProvider.java
Show resolved
Hide resolved
...gin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/LogsdbIndexModeSettingsProvider.java
Show resolved
Hide resolved
.../javaRestTest/java/org/elasticsearch/xpack/logsdb/patterntext/PatternTextBasicLicenseIT.java
Show resolved
Hide resolved
| } | ||
| } | ||
|
|
||
| @SuppressWarnings("unchecked") |
There was a problem hiding this comment.
nit: add the supress warning to the places where it is needed? For example:
@SuppressWarnings("unchecked")
Map<String, Object> mapping = getMapping(client(), backingIndex0);
@SuppressWarnings("unchecked")
Map<String, Object> patternFieldMapping = (Map<String, Object>) ((Map<String, Object>) mapping.get("properties")).get(
"message"
);There was a problem hiding this comment.
Oops, yep I just added the whole-function SuppressWarnings just while I was developing the test and I meant to move it to the specific line, but it slipped my mind
| Setting.Property.Dynamic, | ||
| Setting.Property.NodeScope | ||
| ); | ||
| static final Setting<Boolean> LOGSDB_DEFAULT_SORT_ON_MESSAGE_TEMPLATE = Setting.boolSetting( |
There was a problem hiding this comment.
I chatted with @romseygeek about this and we agree with introducing this setting that makes it easier to experiment with pattern_text field type (compared to manually updating index sorting). Given that this setting is disabled by default and only makes sense if pattern_text field type is configured for the message field.
Before merging this PR we should wait to see the result of #134221. So let's merge this PR and enabled it in the benchmark run that uses pattern_text next week?
|
One small post approval comment: Maybe also update the docs for pattern_text to explain this new index setting? |
🔍 Preview links for changed docs |
ℹ️ Important: Docs version tagging👋 Thanks for updating the docs! Just a friendly reminder that our docs are now cumulative. This means all 9.x versions are documented on the same page and published off of the main branch, instead of creating separate pages for each minor version. We use applies_to tags to mark version-specific features and changes. Expand for a quick overviewWhen to use applies_to tags:✅ At the page level to indicate which products/deployments the content applies to (mandatory) What NOT to do:❌ Don't remove or replace information that applies to an older version 🤔 Need help?
|
…stic#136571) This PR adds the index setting `index.logsdb.default_sort_on_message_template`. When set, LogsDB indices will add `message.template_id` to the default sort fields (assuming the `message` field is present and of type `pattern_text`).
Use the mapping parameter added in elastic/elasticsearch#136571 to sort on message.template_id instead of manually specifying with index.sort.fields. Relates to #825.
…stic#136571) This PR adds the index setting `index.logsdb.default_sort_on_message_template`. When set, LogsDB indices will add `message.template_id` to the default sort fields (assuming the `message` field is present and of type `pattern_text`).
3 participants
Fixes #135031