MM-67505 Add AnalyticsQueryTimeout setting and use when refreshing materialized views#8846
MM-67505 Add AnalyticsQueryTimeout setting and use when refreshing materialized views#8846
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughUpdated Database environment configuration docs: widened the Query timeout (Database) table layout (no config/value changes) and added a new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsTimed out fetching pipeline failures after 30000ms Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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`:
- Around line 851-853: The RST section title "Analytics query timeout" has an
underline that is too short; update the underline (the line of ~ characters
immediately following the heading) so its length equals the length of "Analytics
query timeout" (use the same ~ character repeated to match the heading length)
to avoid RST title warnings during docs build.
- Around line 855-857: The table still references the old symbols
SqlSettings.QueryTimeout and MM_SQLSETTINGS_QUERYTIMEOUT; update the config key,
environment variable, and System Config path to the new analytics-specific
settings instead (replace occurrences of "SqlSettings.QueryTimeout" and
"MM_SQLSETTINGS_QUERYTIMEOUT" in the table with the new analytics config key and
env var, and change the System Config path entry from "Environment > Database"
to the analytics path such as "Environment > Analytics" to match the new
setting).
🪄 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: a629a6f3-bc7f-4135-9b0e-33454e1bf8e6
📒 Files selected for processing (1)
source/administration-guide/configure/environment-configuration-settings.rst
source/administration-guide/configure/environment-configuration-settings.rst
Show resolved
Hide resolved
source/administration-guide/configure/environment-configuration-settings.rst
Outdated
Show resolved
Hide resolved
|
Newest code from mattermost has been published to preview environment for Git SHA 5701778 |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
source/administration-guide/configure/environment-configuration-settings.rst (1)
855-855:⚠️ Potential issue | 🟠 MajorFix stale config key/default in the analytics timeout table
Line 855 still points to
SqlSettings > QueryTimeout > 30, which contradicts the newanalytics-query-timeoutsetting definition above. This should referenceAnalyticsQueryTimeoutwith default300to avoid misconfiguration.Proposed fix
-| The number of seconds to wait for a response from the database after opening a connection | - ``config.json`` setting: ``SqlSettings`` > ``QueryTimeout`` > ``30`` | +| The number of seconds to wait for a response from the database after opening a connection | - ``config.json`` setting: ``SqlSettings`` > ``AnalyticsQueryTimeout`` > ``300`` |🤖 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 855, Update the stale config key and default in the analytics timeout table: replace the reference to "SqlSettings > QueryTimeout > 30" with the new key name "AnalyticsQueryTimeout" and the correct default value "300" so the table matches the `analytics-query-timeout` setting definition; ensure any surrounding text or table column that names the config key or default is updated to the exact identifier AnalyticsQueryTimeout and default 300 to avoid misconfiguration.
🤖 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`:
- Line 855: Update the stale config key and default in the analytics timeout
table: replace the reference to "SqlSettings > QueryTimeout > 30" with the new
key name "AnalyticsQueryTimeout" and the correct default value "300" so the
table matches the `analytics-query-timeout` setting definition; ensure any
surrounding text or table column that names the config key or default is updated
to the exact identifier AnalyticsQueryTimeout and default 300 to avoid
misconfiguration.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b755e186-9212-449e-a178-b0d9ef0e7cad
📒 Files selected for processing (1)
source/administration-guide/configure/environment-configuration-settings.rst
|
Newest code from mattermost has been published to preview environment for Git SHA 3cc3c5d |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
source/administration-guide/configure/environment-configuration-settings.rst (1)
854-860:⚠️ Potential issue | 🟠 MajorAnalytics query timeout table still references the old config key/default.
Line 855 should point to
AnalyticsQueryTimeoutwith default300, matching Lines 847-849. Right now it still referencesQueryTimeout/30.Proposed fix
-| The number of seconds to wait for a response from the database after opening a connection | - ``config.json`` setting: ``SqlSettings`` > ``QueryTimeout`` > ``30`` | +| The number of seconds to wait for a response from the database after opening a connection | - ``config.json`` setting: ``SqlSettings`` > ``AnalyticsQueryTimeout`` > ``300`` | | and sending certain analytics queries. This setting only applies to long queries which are | - System Config path: **Environment > Database** | | run in the background to populate some information in the Team and Site Statistics pages. | - Environment variable: ``MM_SQLSETTINGS_ANALYTICSQUERYTIMEOUT`` |🤖 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 854 - 860, The table entry incorrectly references SqlSettings > QueryTimeout with default 30; update it to reference SqlSettings > AnalyticsQueryTimeout with the default value 300 and ensure the Environment variable remains MM_SQLSETTINGS_ANALYTICSQUERYTIMEOUT and System Config path stays Environment > Database so the row matches the other mentions of AnalyticsQueryTimeout (use the same key name "AnalyticsQueryTimeout" and default "300" as on lines 847-849).
🤖 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`:
- Around line 836-842: The table incorrectly maps Query timeout to
AnalyticsQueryTimeout; update the doc to reference the correct config key:
change the config.json setting mapping from ``SqlSettings`` >
``AnalyticsQueryTimeout`` to ``SqlSettings`` > ``QueryTimeout`` (keep the
Environment variable ``MM_SQLSETTINGS_QUERYTIMEOUT`` and System Config path
**Environment > Database** as-is) so the table matches the actual setting used
by SqlSettings.QueryTimeout.
---
Duplicate comments:
In
`@source/administration-guide/configure/environment-configuration-settings.rst`:
- Around line 854-860: The table entry incorrectly references SqlSettings >
QueryTimeout with default 30; update it to reference SqlSettings >
AnalyticsQueryTimeout with the default value 300 and ensure the Environment
variable remains MM_SQLSETTINGS_ANALYTICSQUERYTIMEOUT and System Config path
stays Environment > Database so the row matches the other mentions of
AnalyticsQueryTimeout (use the same key name "AnalyticsQueryTimeout" and default
"300" as on lines 847-849).
🪄 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: ebe19d9a-797f-4187-8fd9-b7c8b614e138
📒 Files selected for processing (1)
source/administration-guide/configure/environment-configuration-settings.rst
source/administration-guide/configure/environment-configuration-settings.rst
Show resolved
Hide resolved
|
Newest code from mattermost has been published to preview environment for Git SHA dcf42d4 |
|
Newest code from mattermost has been published to preview environment for Git SHA 1e08e01 |
|
@coderabbitai Re-review this |
|
✅ Actions performedFull review triggered. |
|
Newest code from mattermost has been published to preview environment for Git SHA dedb986 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 855: The documentation has an inconsistent default for
AnalyticsQueryTimeout in SqlSettings (config.json): line 855 shows "30" while
surrounding lines show "300"; update the text referencing AnalyticsQueryTimeout
under SqlSettings/AnalyticsQueryTimeout to use the correct default value "300"
so all occurrences match, and verify any adjacent table cells or examples also
reflect the 300-second default.
🪄 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: 23e77829-6511-4a49-aee2-3fc7a4a02736
📒 Files selected for processing (1)
source/administration-guide/configure/environment-configuration-settings.rst
source/administration-guide/configure/environment-configuration-settings.rst
Outdated
Show resolved
Hide resolved
|
Newest code from mattermost has been published to preview environment for Git SHA bbf2f97 |
|
Newest code from mattermost has been published to preview environment for Git SHA 62aff38 |
Summary
Docs update for the new setting added in mattermost/mattermost#35906
Ticket Link
https://mattermost.atlassian.net/browse/MM-67505