support for Elastic(Open)search CJK analysis plugins#8830
Conversation
Documents the new ElasticsearchSettings.EnableCJKAnalyzers configuration setting that enables language-specific analyzer plugins for improved search results for Korean (analysis-nori), Japanese (analysis-kuromoji), and Chinese (analysis-smartcn) content. Disabled by default. - Add config setting entry to environment-configuration-settings.rst - Add optional CJK plugin installation notes to elasticsearch-setup.rst - Add optional CJK plugin installation notes to opensearch-setup.rst Closes #8829 Co-authored-by: Combs7th <Combs7th@users.noreply.github.com>
Combs7th
added
1: Dev Review
|
@carlisgg - Do the above doc updates look accurate to you? |
|
Newest code from mattermost has been published to preview environment for Git SHA 567649a |
|
|
||
| sudo /usr/share/elasticsearch/bin/elasticsearch-plugin install analysis-icu | ||
|
|
||
| **(Optional) CJK language analyzer plugins**: If you need improved search for Korean, Japanese, or Chinese content, install the required language-specific analyzer plugins and enable the :ref:`EnableCJKAnalyzers <administration-guide/configure/environment-configuration-settings:enable cjk analyzers>` configuration setting. The supported plugins are ``analysis-nori`` (Korean), ``analysis-kuromoji`` (Japanese), and ``analysis-smartcn`` (Chinese). Install them using the same ``elasticsearch-plugin install`` command, for example: |
There was a problem hiding this comment.
There is a page about CJK support when using DB related search (https://docs.mattermost.com/administration-guide/configure/enabling-chinese-japanese-korean-search.html) that mentions ES and OS options. Maybe we could mention and link to the specific location where CJK support and flag is explained?
|
@claude - Please implement the changes requested by carlisgg above. |
|
Claude finished @Combs7th's task in 1m 11s —— View job Implemented carlisgg's Review Requests
Changes Made
|
- Add note in EnableCJKAnalyzers setting that existing servers must purge and rebuild indexes when enabling this setting - Add link to Enabling CJK Search docs page in elasticsearch-setup.rst and opensearch-setup.rst Co-authored-by: Combs7th <Combs7th@users.noreply.github.com>
|
Newest code from mattermost has been published to preview environment for Git SHA 004336a |
|
@carlisgg - How's it lookin' now? |
|
@carlisgg - Could I ask for your help with one more review on this one? If it looks good, I'll pass to Eric for merge. |
|
@Combs7th there are unintended changes in here (desktop stuff). |
|
Good catch @esethna, thanks! Ready for review again now. |
|
Newest code from mattermost has been published to preview environment for Git SHA a348a09 |
|
Newest code from mattermost has been published to preview environment for Git SHA 84b7ced |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
|
||
| .. note:: | ||
|
|
||
| Available from Mattermost v11.6. All required analyzer plugins must be installed on the Elasticsearch or AWS OpenSearch server before enabling this setting. See the :doc:`Elasticsearch setup </administration-guide/scale/elasticsearch-setup>` and :doc:`AWS OpenSearch setup </administration-guide/scale/opensearch-setup>` documentation for plugin installation instructions. |
There was a problem hiding this comment.
this is great, just one last detail. The setting can be enabled even if plugins are not installed but with no effect apart from a warning being written to the logs about the fact. Also it will work with whatever plugins are installed and make sense for current deployment not requiring all to be installed, even though description is ok, maybe could be confusing.
So maybe we could rephrase
All required analyzer plugins must be installed on the Elasticsearch or AWS OpenSearch server before enabling this setting
to something like
The required analyzer plugins should be installed on the Elasticsearch or AWS OpenSearch server before enabling this setting for full analysis support. If no plugin is detected, a warning will be logged
or similar
There was a problem hiding this comment.
Thanks @carlisgg! Great feedback. That wording has been updated on the draft.
|
@Combs7th see Amy's comments. There are still unintended desktop changelog and release docs changes in here |
|
|
||
| sudo /usr/share/elasticsearch/bin/elasticsearch-plugin install analysis-icu | ||
|
|
||
| **(Optional) CJK language analyzer plugins**: If you need improved search for Korean, Japanese, or Chinese content, install the required language-specific analyzer plugins and enable the :ref:`EnableCJKAnalyzers <administration-guide/configure/environment-configuration-settings:enable cjk analyzers>` configuration setting. The supported plugins are ``analysis-nori`` (Korean), ``analysis-kuromoji`` (Japanese), and ``analysis-smartcn`` (Chinese). Install them using the same ``elasticsearch-plugin install`` command, for example. See also :doc:`Enabling Chinese, Japanese, and Korean Search </administration-guide/configure/enabling-chinese-japanese-korean-search>` for additional CJK search configuration options. |
There was a problem hiding this comment.
@Combs7th let's review from the Editor Evie persona. The sentence structure and grammar is a bit funky here. https://github.com/mattermost/docs/blob/master/.cursor/rules/docs-editor-persona-evie.mdc
|
|
||
| sudo /usr/share/opensearch/bin/opensearch-plugin install analysis-icu | ||
|
|
||
| **(Optional) CJK language analyzer plugins**: If you need improved search for Korean, Japanese, or Chinese content, install the required language-specific analyzer plugins and enable the :ref:`EnableCJKAnalyzers <administration-guide/configure/environment-configuration-settings:enable cjk analyzers>` configuration setting. The supported plugins are ``analysis-nori`` (Korean), ``analysis-kuromoji`` (Japanese), and ``analysis-smartcn`` (Chinese). Install them using the same ``opensearch-plugin install`` command, for example. See also :doc:`Enabling Chinese, Japanese, and Korean Search </administration-guide/configure/enabling-chinese-japanese-korean-search>` for additional CJK search configuration options. |
There was a problem hiding this comment.
@Combs7th let's review from the Editor Evie persona. The sentence structure and grammar is a bit funky here. https://github.com/mattermost/docs/blob/master/.cursor/rules/docs-editor-persona-evie.mdc
|
@claude - Please analyze the text blocks Eric responded to above, and fix the sentence structure and grammar using the Editor Evie reviewer persona. |
|
Claude finished @Combs7th's task in 1m 24s —— View job Fixing Grammar/Sentence Structure per Editor Evie Persona
Changes MadeBoth paragraphs had the same issues:
Rewrote both paragraphs to:
|
Co-authored-by: Combs7th <Combs7th@users.noreply.github.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 55 minutes and 20 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughDocumentation updates across three pages to introduce and explain the new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (1)
source/administration-guide/configure/environment-configuration-settings.rst (1)
1917-1919:⚠️ Potential issue | 🟡 MinorClarify plugin prerequisite wording to avoid conflicting guidance
Line 1917 says plugins “must be installed” before enabling, but Line 1933 explains the setting can still be enabled and logs a warning when no plugin is detected. Please make this wording consistent with the fallback behaviour.
Suggested minimal diff
-| Chinese content. The required analyzer plugins must be | -| installed on the Elasticsearch or AWS OpenSearch server | -| before enabling this setting. | +| Chinese content. Install the relevant analyzer plugins | +| on the Elasticsearch or AWS OpenSearch server for full | +| CJK analysis support. |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@source/administration-guide/configure/environment-configuration-settings.rst` around lines 1917 - 1919, Update the phrasing around the MM_ELASTICSEARCHSETTINGS_ENABLECJKANALYZERS setting so it matches the actual behavior: replace the absolute "must be installed" claim with wording that these Chinese analyzer plugins should be installed on the Elasticsearch/OpenSearch server to ensure full functionality, and add a short note that the application will still start with the setting enabled but will log a warning and fall back if the plugins are not detected (referencing MM_ELASTICSEARCHSETTINGS_ENABLECJKANALYZERS to locate the relevant text).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@source/administration-guide/configure/environment-configuration-settings.rst`:
- Line 1935: The sentence referencing only Elasticsearch should include a
parallel internal link to AWS OpenSearch; update the text that currently
contains ":doc:`Elasticsearch setup
</administration-guide/scale/elasticsearch-setup>`" to also reference the
OpenSearch guide using the same internal link syntax (for example add ":doc:`AWS
OpenSearch setup </administration-guide/scale/opensearch-setup>`"), so admins
using either backend are directed to the correct purge-and-rebuild instructions.
- Around line 1931-1936: Replace the current ".. note::" admonition with an "..
important::" admonition for the block that contains the sentence "If you enable
this setting on a server that was previously running Elasticsearch or AWS
OpenSearch, you must purge and rebuild the search indexes for existing content
to be properly searchable with the new analyzers." (locate the ".. note::"
directive and that exact sentence), leaving the content intact but using "..
important::" to mark this as a mandatory reindex prerequisite per the
documentation guidelines.
In `@source/administration-guide/scale/elasticsearch-setup.rst`:
- Around line 71-78: Add explicit operational steps after the plugin
installation block: instruct operators to restart the Elasticsearch service so
newly installed plugins (analysis-nori, analysis-kuromoji, analysis-smartcn) are
loaded, and add a clear warning that when turning on the EnableCJKAnalyzers
setting existing indexes must be purged and rebuilt (reindex) to avoid search
inconsistencies; reference the EnableCJKAnalyzers setting and the three plugin
names so readers can locate the guidance and perform restart + reindex on
production servers.
- Around line 71-72: Split the long sentence into two clear steps: first, state
that for better Korean, Japanese, or Chinese search you should install one or
more language analyzer plugins and list the plugin names (analysis-nori,
analysis-kuromoji, analysis-smartcn) and specify the exact command to run
(elasticsearch-plugin install <plugin-name>); second, instruct readers to enable
the EnableCJKAnalyzers configuration setting and reference the Enabling Chinese,
Japanese, and Korean Search doc for additional configuration options. Ensure the
two sentences are short and procedural (install plugins via elasticsearch-plugin
install <plugin-name>; then enable EnableCJKAnalyzers) and remove the phrase
"using the same ... command" that implies a prior command.
In `@source/administration-guide/scale/opensearch-setup.rst`:
- Around line 101-108: After the plugin install commands for
analysis-nori/analysis-kuromoji/analysis-smartcn, add a note instructing
operators to restart the OpenSearch service so the plugins are loaded (e.g.,
restart opensearch process/service), and add a clear warning that enabling the
EnableCJKAnalyzers setting requires purging and rebuilding existing search
indexes on affected nodes to avoid inconsistent/incorrect search results;
reference the EnableCJKAnalyzers setting name and the installed plugin names
(analysis-nori, analysis-kuromoji, analysis-smartcn) so readers know this
applies when they enable CJK analyzers.
- Around line 101-102: The sentence is a run-on mixing explanation and steps;
split it into a short purpose sentence (e.g., "To improve search for Korean,
Japanese, or Chinese content, install language-specific CJK analyzers.")
followed by a procedural sentence instructing to install one or more plugins
with the opensearch-plugin install command (list the plugin names analysis-nori,
analysis-kuromoji, analysis-smartcn), then a final instruction to enable the
EnableCJKAnalyzers configuration setting and refer to the Enabling Chinese,
Japanese, and Korean Search document for additional options.
In `@source/product-overview/desktop-app-changelog.md`:
- Line 209: The Download Binaries link currently points to
".../releases/v5.13.2" (the line reading "**Download Binaries:** [Mattermost
Desktop on GitHub](https://github.com/mattermost/desktop/releases/v5.13.2)")
which is inconsistent with the documented latest v5.13 patch; update that URL to
the correct patch (e.g., change v5.13.2 to v5.13.5 or to whichever latest v5.13
patch you intend) and verify the same link/version is used consistently across
the desktop docs pages.
In `@source/product-overview/mattermost-desktop-releases.md`:
- Line 23: The v5.13 release row was unintentionally downgraded to v5.13.4;
update the v5.13 row to reference the correct v5.13.5 artifacts by changing the
download link and tag text from v5.13.4 to v5.13.5 and the SBOM download
URL/filename from sbom-desktop-v5.13.4.json to sbom-desktop-v5.13.5.json (i.e.,
restore the v5.13 row to point to v5.13.5).
---
Duplicate comments:
In
`@source/administration-guide/configure/environment-configuration-settings.rst`:
- Around line 1917-1919: Update the phrasing around the
MM_ELASTICSEARCHSETTINGS_ENABLECJKANALYZERS setting so it matches the actual
behavior: replace the absolute "must be installed" claim with wording that these
Chinese analyzer plugins should be installed on the Elasticsearch/OpenSearch
server to ensure full functionality, and add a short note that the application
will still start with the setting enabled but will log a warning and fall back
if the plugins are not detected (referencing
MM_ELASTICSEARCHSETTINGS_ENABLECJKANALYZERS to locate the relevant text).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f71a3525-00d6-42fc-bcfc-10392e60ac1a
📒 Files selected for processing (5)
source/administration-guide/configure/environment-configuration-settings.rstsource/administration-guide/scale/elasticsearch-setup.rstsource/administration-guide/scale/opensearch-setup.rstsource/product-overview/desktop-app-changelog.mdsource/product-overview/mattermost-desktop-releases.md
|
Newest code from mattermost has been published to preview environment for Git SHA 2869b73 |
|
Newest code from mattermost has been published to preview environment for Git SHA 7a66085 |
|
Newest code from mattermost has been published to preview environment for Git SHA 0a178a8 |
|
@claude - Implement all 4 of the recommended changes from Veteran Vince and Editor Evie above. |
|
Claude finished @Combs7th's task in 1m 50s —— View job Implemented Veteran Vince & Editor Evie Changes
Summary of ChangesBoth
|
- Split run-on CJK plugin intro sentence; remove misleading "same command" reference - Move enable-setting and see-also link to after the install code block - Add service restart step (elasticsearch/opensearch) after plugin install - Add .. important:: admonition: purge/rebuild indexes when enabling on existing server Co-authored-by: Combs7th <Combs7th@users.noreply.github.com>
|
Newest code from mattermost has been published to preview environment for Git SHA 253e2c9 |
There was a problem hiding this comment.
♻️ Duplicate comments (3)
source/administration-guide/configure/environment-configuration-settings.rst (3)
1963-1968:⚠️ Potential issue | 🟡 MinorUse
importantfor the mandatory reindex prerequisiteLine 1967 describes a required operational step (“must purge and rebuild”), so this should be
.. important::, not.. note::.Suggested minimal diff
-.. note:: +.. important::Based on learnings, "Use
importantadmonition for prerequisites, constraints, or high-impact information that materially affects correctness, supportability, compliance, or success."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@source/administration-guide/configure/environment-configuration-settings.rst` around lines 1963 - 1968, Replace the ".. note::" admonition that contains the required reindex sentence with an ".. important::" admonition so the sentence beginning "If you enable this setting on a server that was previously running Elasticsearch or AWS OpenSearch, you must purge and rebuild the search indexes for existing content to be properly searchable with the new analyzers." is highlighted as a mandatory prerequisite; update the block that also references Elasticsearch/OpenSearch setup docs (the same paragraph) to use ".. important::" instead of ".. note::".
1949-1951:⚠️ Potential issue | 🟡 MinorAlign plugin prerequisite wording with actual behaviour
Line 1949 says plugins “must” be installed before enabling, but Line 1965 already documents that enabling without detected plugins logs a warning. Please make this wording consistent to avoid misleading admins.
Suggested minimal diff
-| Chinese content. The required analyzer plugins must be | - Environment variable: ``MM_ELASTICSEARCHSETTINGS_ENABLECJKANALYZERS`` | -| installed on the Elasticsearch or AWS OpenSearch server | | -| before enabling this setting. | | +| Chinese content. Install one or more supported analyzer | - Environment variable: ``MM_ELASTICSEARCHSETTINGS_ENABLECJKANALYZERS`` | +| plugins on the Elasticsearch or AWS OpenSearch server | | +| before enabling this setting for full analysis support. | |As per coding guidelines, "Flag commands, settings, prerequisites, or sequencing that are misleading, incomplete, or unlikely to work as written."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@source/administration-guide/configure/environment-configuration-settings.rst` around lines 1949 - 1951, The documentation currently says plugins "must" be installed before enabling MM_ELASTICSEARCHSETTINGS_ENABLECJKANALYZERS but elsewhere (around the MM_ELASTICSEARCHSETTINGS_ENABLECJKANALYZERS handling) you document that enabling without detected plugins only logs a warning; change the wording at the earlier occurrence to match actual behaviour—replace "must be installed before enabling" with language such as "should be installed (a warning is logged if not detected)" or "recommended; enabling without them will log a warning" so the docs are consistent with the warning behavior.
1967-1967:⚠️ Potential issue | 🟡 MinorAdd OpenSearch setup link to rebuild guidance
Line 1967 applies to Elasticsearch and AWS OpenSearch, but the rebuild instruction links only to Elasticsearch setup.
Suggested minimal diff
- If you enable this setting on a server that was previously running Elasticsearch or AWS OpenSearch, you must purge and rebuild the search indexes for existing content to be properly searchable with the new analyzers. See the :doc:`Elasticsearch setup </administration-guide/scale/elasticsearch-setup>` documentation for instructions on purging and rebuilding indexes. + If you enable this setting on a server that was previously running Elasticsearch or AWS OpenSearch, you must purge and rebuild the search indexes for existing content to be properly searchable with the new analyzers. See the :doc:`Elasticsearch setup </administration-guide/scale/elasticsearch-setup>` and :doc:`AWS OpenSearch setup </administration-guide/scale/opensearch-setup>` documentation for instructions on purging and rebuilding indexes.As per coding guidelines, "Describe Mattermost feature behavior, UI labels, and capability boundaries accurately."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@source/administration-guide/configure/environment-configuration-settings.rst` at line 1967, The sentence starting "If you enable this setting on a server that was previously running Elasticsearch or AWS OpenSearch..." links only to the Elasticsearch setup; update the link to reference OpenSearch setup as well (either by adding an additional :doc: link to the OpenSearch doc or replacing the single link with a combined sentence that points to both :doc:`Elasticsearch setup </administration-guide/scale/elasticsearch-setup>` and the corresponding OpenSearch setup doc), ensuring the rebuild guidance accurately covers both Elasticsearch and AWS OpenSearch.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@source/administration-guide/configure/environment-configuration-settings.rst`:
- Around line 1963-1968: Replace the ".. note::" admonition that contains the
required reindex sentence with an ".. important::" admonition so the sentence
beginning "If you enable this setting on a server that was previously running
Elasticsearch or AWS OpenSearch, you must purge and rebuild the search indexes
for existing content to be properly searchable with the new analyzers." is
highlighted as a mandatory prerequisite; update the block that also references
Elasticsearch/OpenSearch setup docs (the same paragraph) to use ".. important::"
instead of ".. note::".
- Around line 1949-1951: The documentation currently says plugins "must" be
installed before enabling MM_ELASTICSEARCHSETTINGS_ENABLECJKANALYZERS but
elsewhere (around the MM_ELASTICSEARCHSETTINGS_ENABLECJKANALYZERS handling) you
document that enabling without detected plugins only logs a warning; change the
wording at the earlier occurrence to match actual behaviour—replace "must be
installed before enabling" with language such as "should be installed (a warning
is logged if not detected)" or "recommended; enabling without them will log a
warning" so the docs are consistent with the warning behavior.
- Line 1967: The sentence starting "If you enable this setting on a server that
was previously running Elasticsearch or AWS OpenSearch..." links only to the
Elasticsearch setup; update the link to reference OpenSearch setup as well
(either by adding an additional :doc: link to the OpenSearch doc or replacing
the single link with a combined sentence that points to both :doc:`Elasticsearch
setup </administration-guide/scale/elasticsearch-setup>` and the corresponding
OpenSearch setup doc), ensuring the rebuild guidance accurately covers both
Elasticsearch and AWS OpenSearch.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4f043fac-e6ca-4d36-b847-b23ad4036069
📒 Files selected for processing (3)
source/administration-guide/configure/environment-configuration-settings.rstsource/administration-guide/scale/elasticsearch-setup.rstsource/administration-guide/scale/opensearch-setup.rst
✅ Files skipped from review due to trivial changes (2)
- source/administration-guide/scale/elasticsearch-setup.rst
- source/administration-guide/scale/opensearch-setup.rst
|
Newest code from mattermost has been published to preview environment for Git SHA bfc6bc7 |
@Combs7th No worries, thank you! |
|
Newest code from mattermost has been published to preview environment for Git SHA 560ec44 |
4 participants
Documents the new
ElasticsearchSettings.EnableCJKAnalyzersconfiguration setting that enables language-specific analyzer plugins for improved search results for Korean (analysis-nori), Japanese (analysis-kuromoji), and Chinese (analysis-smartcn) content. Disabled by default.Closes #8829
Generated with Claude Code