test #31
test #31
Conversation
WalkthroughAdds two new system variables (a DDL-embedded ANALYZE toggle and an optimizer selectivity factor), introduces documentation for ANALYZE embedded in DDL, updates TOC entries, clarifies restore-mode behavior in BR docs, expands DM MySQL compatibility rows, and refactors OWNERS/OWNERS_ALIASES to filter-based ownership groups. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TiDB_DDL as TiDB DDL Job
participant StatsWorker as Stats Collector
participant Storage as Storage/PD
Note over User,TiDB_DDL: Create or reorganize index (ADD INDEX / MODIFY/CHANGE COLUMN)
User->>TiDB_DDL: submit DDL job
alt tidb_stats_update_during_ddl = ON
TiDB_DDL->>StatsWorker: trigger ANALYZE for index (before visibility)
StatsWorker->>Storage: read table data, build stats
StatsWorker-->>TiDB_DDL: report stats (may be async if timeout)
TiDB_DDL->>TiDB_DDL: make index visible (after stats or timeout)
else tidb_stats_update_during_ddl = OFF
TiDB_DDL->>TiDB_DDL: create index without embedded ANALYZE
TiDB_DDL->>User: DDL completes (stats not updated)
end
Note over TiDB_DDL,User: Use SHOW STATS_HISTOGRAMS / ADMIN SHOW DDL JOBS to verify
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (5)
glossary.md (4)
47-49: Consider adding a documentation link for CF.The definition is accurate but could be enhanced with a link to TiKV's storage documentation for more details about Column Families.
59-84: Consider adding documentation links for CTE, DDL, and DML entries.While the definitions are accurate, consider adding links to:
- CTE: Link to
WITHclause documentation already referenced- DDL: Link to DDL statements documentation
- DML: Link to DML statements documentation
This would maintain consistency with other entries that include documentation links.
Line range hint
165-167: Consider adding TiDB-specific documentation link for MVCC.While the Wikipedia link provides general context, consider adding a link to TiDB's transaction documentation for MVCC implementation details.
Line range hint
246-267: Consider adding documentation links for Region-related entries.While the definitions are clear, consider adding links to:
- Region/peer/Raft group: Link to TiKV architecture documentation
- Region split: Link to Region management documentation
This would help users find more detailed information about these fundamental concepts.
alert-rules.md (1)
569-570: Consider enhancing the alert description.While the description clearly explains the alert's purpose, consider adding information about potential system impact when Raftstore threads are under heavy pressure, such as increased latency or reduced throughput.
This rule monitors CPU usage by Raftstore. If the value is high, it indicates pressure on Raftstore threads is heavy. +This can lead to increased latency in Raft operations and reduced overall system throughput.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
alert-rules.md(1 hunks)glossary.md(3 hunks)
🔇 Additional comments (5)
glossary.md (4)
33-35: LGTM!
The BR entry is clear, concise, and includes appropriate documentation reference.
103-120: LGTM!
The entries are well-defined with appropriate external and internal documentation links where relevant.
123-128: LGTM!
The HTAP entry provides a clear definition, TiDB-specific context, and appropriate external reference.
Line range hint 238-244: LGTM!
Both entries provide clear definitions with appropriate configuration references and status indicators.
alert-rules.md (1)
565-571: LGTM! Well-structured alert rule for Raftstore CPU monitoring.
The alert rule is properly configured with an appropriate threshold (80% of raftstore.store-pool-size) and includes clear metric selection.
glossary.md
Outdated
|
|
||
| Disaster Recovery (DR) includes solutions that can be used to recover data from a disaster in the future. These solutions typically involve backups and standby clusters. | ||
|
|
||
| For more information, see [Overview of TiDB Disaster Recovery Solutions](dr-solution-introduction). |
There was a problem hiding this comment.
Fix broken documentation link in DR entry.
The link to dr-solution-introduction is missing the .md extension.
-For more information, see [Overview of TiDB Disaster Recovery Solutions](dr-solution-introduction).
+For more information, see [Overview of TiDB Disaster Recovery Solutions](/dr-solution-introduction.md).Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
glossary.md(3 hunks)
🧰 Additional context used
🪛 LanguageTool
glossary.md
[grammar] ~131-~131: The plural noun “files” cannot be used with the article “a”. Did you mean “a file” or “files”?
Context: ...mputing-technologies). Test Thies are a files. ## I ### Index Merge Index Merge is...
(A_NNS)
🔇 Additional comments (1)
glossary.md (1)
Line range hint 33-96: LGTM on new glossary entries!
The new entries (BR, CF, CTE, DDL, DM, DML, DMR, DR, DXF) are well-structured and provide clear, accurate definitions with appropriate documentation links.
Note: The broken documentation link in the DR entry still needs to be fixed as mentioned in the previous review.
🧰 Tools
🪛 LanguageTool
[grammar] ~131-~131: The plural noun “files” cannot be used with the article “a”. Did you mean “a file” or “files”?
Context: ...mputing-technologies). Test Thies are a files. ## I ### Index Merge Index Merge is...
(A_NNS)
glossary.md
Outdated
| Test | ||
|
|
||
| Thies are a files. |
There was a problem hiding this comment.
Remove test content
These lines appear to be test content that was accidentally committed. They break both the document structure and alphabetical ordering of the glossary entries.
-Test
-
-Thies are a files.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Test | |
| Thies are a files. |
🧰 Tools
🪛 LanguageTool
[grammar] ~131-~131: The plural noun “files” cannot be used with the article “a”. Did you mean “a file” or “files”?
Context: ...mputing-technologies). Test Thies are a files. ## I ### Index Merge Index Merge is...
(A_NNS)
glossary.md
Outdated
|
|
||
| ### HTTP | ||
|
|
||
| Hypertext Transfer Protocol (HTTP) is a application-layer protocol using for transmitting hypermedia dacuments, such as HTML. It is the foundation of data communication on the web, enabling browsers and servers to request and transfer data in a standardized way. HTTP operates as a request-response protocol: a client sends a request to a server, which then returns a response. Common HTTP methods include GET, POST, PUT, and DELETE, which are used to retrieve, create, update, or delete resources, respectively. |
There was a problem hiding this comment.
@qiancai, I noticed a few typos and grammatical errors in the above line:
- "a application-layer protocol" should be "an application-layer protocol".
- "using for transmitting" should be "used for transmitting".
- "dacuments" should be "documents".
Please consider correcting these issues to improve clarity.
There was a problem hiding this comment.
@coderabbitai review the above line and provide committable suggestion
There was a problem hiding this comment.
@qiancai, here is the corrected paragraph:
### HTTP
Hypertext Transfer Protocol (HTTP) is an application-layer protocol used for transmitting hypermedia documents, such as HTML. It is the foundation of data communication on the web, enabling browsers and servers to request and transfer data in a standardized way. HTTP operates as a request-response protocol: a client sends a request to a server, which then returns a response. Common HTTP methods include GET, POST, PUT, and DELETE, which are used to retrieve, create, update, or delete resources, respectively.
|
@coderabbitai configuration |
CodeRabbit configurationlanguage: en-US
tone_instructions: ''
early_access: false
enable_free_tier: true
reviews:
profile: chill
request_changes_workflow: false
high_level_summary: true
high_level_summary_placeholder: '@coderabbitai summary'
auto_title_placeholder: '@coderabbitai'
review_status: true
commit_status: true
poem: true
collapse_walkthrough: false
sequence_diagrams: true
changed_files_summary: true
labeling_instructions: []
path_filters: []
path_instructions: []
abort_on_close: true
auto_review:
enabled: true
auto_incremental_review: true
ignore_title_keywords: []
labels: []
drafts: false
base_branches: []
tools:
shellcheck:
enabled: true
ruff:
enabled: true
markdownlint:
enabled: true
github-checks:
enabled: true
timeout_ms: 90000
languagetool:
enabled: true
enabled_only: false
level: default
biome:
enabled: true
hadolint:
enabled: true
swiftlint:
enabled: true
phpstan:
enabled: true
level: default
golangci-lint:
enabled: true
yamllint:
enabled: true
gitleaks:
enabled: true
checkov:
enabled: true
detekt:
enabled: true
eslint:
enabled: true
rubocop:
enabled: true
buf:
enabled: true
regal:
enabled: true
actionlint:
enabled: true
pmd:
enabled: true
cppcheck:
enabled: true
semgrep:
enabled: true
circleci:
enabled: true
chat:
auto_reply: true
knowledge_base:
opt_out: false
learnings:
scope: auto
issues:
scope: auto
jira:
project_keys: []
linear:
team_keys: []
pull_requests:
scope: auto
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (6)
.coderabbit.yaml (5)
11-11: Consider versioning documentation linksThe link to
/auto-increment.mdshould include version information to ensure it works correctly across different TiDB versions. Consider using a versioned path or relative link.
92-101: Add explanatory comments for SHOW REGIONS outputThe
SHOW TABLE t REGIONScommand output contains important information about data distribution, but it may be difficult for users to understand. Consider adding explanatory comments about:
- What each column represents
- How to interpret the region distribution
- The relationship between region distribution and the AUTO_RANDOM shard bits
131-135: Enhance shard bits selection documentation with practical examplesWhile the explanation of shard bits selection is technically accurate, it would be more helpful to include:
- A concrete example showing the impact of different shard bit values on write distribution
- Performance implications of various shard bit configurations
- Example scenarios for different cluster sizes
172-172: Clarify NONCLUSTERED primary key restrictionThe restriction regarding NONCLUSTERED primary keys would benefit from:
- An explanation of why AUTO_RANDOM is incompatible with NONCLUSTERED indexes
- Alternative approaches for such scenarios
- A reference to relevant design documentation explaining this limitation
1-178: Consider restructuring documentation for better readabilityWhile the content is comprehensive, consider reorganizing it to:
- Start with a clear "Quick Start" section for common use cases
- Move detailed technical implementation after basic usage
- Add a troubleshooting section with common issues and solutions
- Include a comparison table between AUTO_RANDOM and AUTO_INCREMENT
This would align better with the documentation style mentioned in the PR objectives.
🧰 Tools
🪛 yamllint
[warning] 4-4: too few spaces after comma
(commas)
[error] 23-23: syntax error: found character '`' that cannot start any token
(syntax)
glossary.md (1)
47-49: Consider enhancing the CF definitionWhile accurate, the entry could be more helpful by briefly explaining the purpose of Column Families in TiKV's storage architecture.
Consider expanding the definition:
-In RocksDB and TiKV, a Column Family (CF) represents a logical grouping of key-value pairs within a database. +In RocksDB and TiKV, a Column Family (CF) represents a logical grouping of key-value pairs within a database. TiKV uses different CFs to store various types of data (such as default data, write operations, and lock information) for better performance and management.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
.coderabbit.yaml(1 hunks)glossary.md(3 hunks)
🧰 Additional context used
🪛 yamllint
.coderabbit.yaml
[warning] 4-4: too few spaces after comma
(commas)
[error] 23-23: syntax error: found character '`' that cannot start any token
(syntax)
🪛 LanguageTool
glossary.md
[misspelling] ~131-~131: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... Hypertext Transfer Protocol (HTTP) is a application-layer protocol using for tr...
(EN_A_VS_AN)
🔇 Additional comments (7)
.coderabbit.yaml (1)
1-6:
File extension should match content type
This file contains Markdown content but has a .yaml extension. Consider renaming the file to use the .md extension to accurately reflect its content type and ensure proper handling by documentation tools.
🧰 Tools
🪛 yamllint
[warning] 4-4: too few spaces after comma
(commas)
glossary.md (6)
33-35: LGTM: BR entry is well-defined
The entry is clear, concise, and includes proper documentation link.
59-61: LGTM: CTE entry is well-defined
The entry provides a clear definition and proper documentation link.
65-77: LGTM: DDL, DM, and DML entries are well-defined
All entries provide clear definitions and proper formatting. The DM entry appropriately includes a documentation link.
85-89: LGTM with existing link issue
The definition is clear and accurate. Note that there's an existing comment about the broken documentation link that needs to be addressed.
109-127: LGTM: Remaining entries are well-defined
The entries for GA, GC, GTID, and HTAP are clear, accurate, and properly formatted with appropriate documentation links where needed.
129-131:
Fix grammatical errors and typos in HTTP entry
There are several issues in the text that need to be corrected:
-Hypertext Transfer Protocol (HTTP) is a application-layer protocol using for transmitting hypermedia dacuments, such as HTML.
+Hypertext Transfer Protocol (HTTP) is an application-layer protocol used for transmitting hypermedia documents, such as HTML.Likely invalid or redundant comment.
🧰 Tools
🪛 LanguageTool
[misspelling] ~131-~131: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... Hypertext Transfer Protocol (HTTP) is a application-layer protocol using for tr...
(EN_A_VS_AN)
|
/@gemini-code-assist help me review this PR |
|
It looks like you're asking me to review this pull request using the |
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several new terms to the glossary, updates alert rules, and adds the AUTO_RANDOM attribute for BIGINT columns. Overall, the changes seem well-organized and contribute to the documentation's comprehensiveness. However, some minor improvements can be made to enhance clarity and consistency.
Merge Readiness
The pull request is almost ready for merging. Addressing the low and medium severity issues would further improve the quality of the documentation. I am unable to approve this pull request, and recommend that others review and approve this code before merging.
Synced from: qiancai/docs-cn#15 Target PR: #31 AI Provider: gemini Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
|
Auto-sync completed successfully Source PR: qiancai/docs-cn#15 |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
system-variables.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.md
⚙️ CodeRabbit configuration file
As a technical writer in the database area, you need to first learn the changes in this PR, and then improve the content to make it clear, concise, and easy to understand in English according to your database knowledge.
Files:
system-variables.md
🪛 markdownlint-cli2 (0.18.1)
system-variables.md
1795-1795: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above
(MD022, blanks-around-headings)
system-variables.md
Outdated
| ### `tidb_opt_selectivity_factor` <span class="version-mark">Introduced in v9.0.0</span> | ||
|
|
||
| - Scope: SESSION | GLOBAL | ||
| - Is persisted to the cluster: Yes | ||
| - Is controlled by Hint [SET_VAR](/optimizer-hints.md#set_varvar_namevar_value): Yes | ||
| - Type: Floating-point number | ||
| - Value range: `[0, 1]` | ||
| - Default value: `0.8` | ||
| - This variable is used to specify the default selectivity of the TiDB optimizer. In some cases, when the optimizer cannot derive the predicate selectivity based on statistics, the optimizer uses this default selectivity as an alternative value. **It is not recommended** to modify this value. | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Fix misplaced heading, style, and MD022; move to the tidb_opt_ section and standardize labels.*
- The new variable block is inserted inside the
tidb_dml_batch_sizesection, breaking the structure and triggering MD022 (missing blank line). - Labels and formatting deviate from house style; heading should not be code-formatted; use “New in …” and standard field names.
- Alphabetical order:
tidb_opt_selectivity_factorbelongs with othertidb_opt_*variables (neartidb_opt_seek_factor).
Apply:
- Remove the misplaced block here.
- Add the corrected block after
tidb_opt_seek_factor(see second diff).
-### `tidb_opt_selectivity_factor` <span class="version-mark">Introduced in v9.0.0</span>
-
-- Scope: SESSION | GLOBAL
-- Is persisted to the cluster: Yes
-- Is controlled by Hint [SET_VAR](/optimizer-hints.md#set_varvar_namevar_value): Yes
-- Type: Floating-point number
-- Value range: `[0, 1]`
-- Default value: `0.8`
-- This variable is used to specify the default selectivity of the TiDB optimizer. In some cases, when the optimizer cannot derive the predicate selectivity based on statistics, the optimizer uses this default selectivity as an alternative value. **It is not recommended** to modify this value.Add (outside this hunk; see next diff) the corrected, relocated block.
Additional change (relocation + style fix):
```diff
@@
### tidb_opt_seek_factor
...
+### tidb_opt_selectivity_factor <span class="version-mark">New in v9.0.0</span>
+
+- Scope: SESSION | GLOBAL
+- Persists to cluster: Yes
+- Applies to hint [SET_VAR](/optimizer-hints.md#set_varvar_namevar_value): Yes
+- Type: Float
+- Range: `[0, 1]`
+- Default value: `0.8`
+- Sets the fallback selectivity the optimizer uses when predicate selectivity cannot be derived from statistics. Changing this can affect cardinality estimates and plan selection. It is not recommended to modify this value; prefer collecting/updating statistics instead.
+
### tidb_opt_skew_distinct_agg
Please confirm:
- The canonical variable name and visibility (SESSION | GLOBAL) match TiDB implementation.
- The “New in v9.0.0” marker is correct for this doc version.
- Whether a cross‑reference to
tidb_default_string_match_selectivityis desired.
I need to gather information from the codebase to verify this review comment. Let me inspect the file structure and formatting.
🏁 Script executed:
#!/bin/bash
# Locate system-variables.md and check the content around lines 1795-1804
fd -t f "system-variables.md" | head -1 | xargs -I {} sh -c 'echo "=== File: {} ===" && wc -l {} && echo "" && echo "=== Lines 1790-1810 ===" && sed -n "1790,1810p" {}'Length of output: 1908
🏁 Script executed:
#!/bin/bash
# Find tidb_dml_batch_size and tidb_opt_seek_factor sections to understand structure
fd -t f "system-variables.md" | head -1 | xargs -I {} sh -c 'echo "=== tidb_dml_batch_size section ===" && sed -n "/^### tidb_dml_batch_size/,/^### /p" {} | head -20 && echo "" && echo "=== tidb_opt_seek_factor section ===" && sed -n "/^### tidb_opt_seek_factor/,/^### /p" {} | head -25'Length of output: 674
🏁 Script executed:
#!/bin/bash
# Check formatting patterns for other tidb_opt_* variables
fd -t f "system-variables.md" | head -1 | xargs -I {} sh -c 'grep -n "^### tidb_opt_" {} | head -15'Length of output: 795
🏁 Script executed:
#!/bin/bash
# Check the version marker style used in the file
fd -t f "system-variables.md" | head -1 | xargs -I {} sh -c 'echo "=== Version marker patterns ===" && grep -n "version-mark" {} | head -10'Length of output: 963
🏁 Script executed:
#!/bin/bash
# Verify standard field labels by checking a nearby variable entry
fd -t f "system-variables.md" | head -1 | xargs -I {} sh -c 'echo "=== tidb_opt_seek_factor full entry ===" && sed -n "/^### tidb_opt_seek_factor/,/^### /p" {} | head -15'Length of output: 527
Fix structural break, formatting, and alphabetical order of tidb_opt_selectivity_factor.
The variable block is incorrectly placed inside the tidb_dml_batch_size section (lines 1803–1811), breaking the document structure and leaving tidb_dml_batch_size without a body. Additionally, the formatting deviates from house style used throughout the file:
- Version marker should be
New in v9.0.0(notIntroduced), consistent with all other entries. - Field labels should use
Persists to cluster:andApplies to hint:(notIs persisted to the cluster:/Is controlled by Hint:), matchingtidb_opt_seek_factorand surrounding entries. - Type should be
Float(notFloating-point number); Range should beRange:(notValue range:). - Variable must be relocated to its alphabetically correct position after
tidb_opt_seek_factor.
Actions:
- Remove the misplaced block at lines 1803–1811.
- Insert the corrected block after
tidb_opt_seek_factorwith standardized formatting.
-### `tidb_opt_selectivity_factor` <span class="version-mark">Introduced in v9.0.0</span>
-
-- Scope: SESSION | GLOBAL
-- Is persisted to the cluster: Yes
-- Is controlled by Hint [SET_VAR](/optimizer-hints.md#set_varvar_namevar_value): Yes
-- Type: Floating-point number
-- Value range: `[0, 1]`
-- Default value: `0.8`
-- This variable is used to specify the default selectivity of the TiDB optimizer. In some cases, when the optimizer cannot derive the predicate selectivity based on statistics, the optimizer uses this default selectivity as an alternative value. **It is not recommended** to modify this value.Insert after tidb_opt_seek_factor:
+### tidb_opt_selectivity_factor <span class="version-mark">New in v9.0.0</span>
+
+- Scope: SESSION | GLOBAL
+- Persists to cluster: Yes
+- Applies to hint [SET_VAR](/optimizer-hints.md#set_varvar_namevar_value): Yes
+- Type: Float
+- Range: `[0, 1]`
+- Default value: `0.8`
+- Sets the fallback selectivity the optimizer uses when predicate selectivity cannot be derived from statistics. Changing this can affect cardinality estimates and plan selection. It is not recommended to modify this value; prefer collecting or updating statistics instead.Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
1795-1795: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents
In system-variables.md around lines 1795–1804 (and specifically remove the
misplaced block at lines 1803–1811), the tidb_opt_selectivity_factor entry is
incorrectly nested under tidb_dml_batch_size and uses nonstandard
labels/formatting; delete that misplaced block and insert a new tidy entry
immediately after tidb_opt_seek_factor (to maintain alphabetical order) using
house style: "New in v9.0.0", "Scope: SESSION | GLOBAL", "Persists to cluster:
Yes", "Applies to hint: Yes", "Type: Float", "Range: [0, 1]", "Default value:
0.8", and a one-sentence description that it specifies the default optimizer
selectivity used when statistics are unavailable and is not recommended to
change.
Synced from: qiancai/docs-cn#15 Target PR: #31 AI Provider: gemini Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
|
Auto-sync completed successfully Source PR: qiancai/docs-cn#15 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
br/br-pitr-guide.md (1)
120-121: Consider simplifying the wording for better readability.The explanation is clear and accurate. However, the phrase "the table mode of the target table" could be simplified to "the target table mode" for conciseness. Additionally, breaking the long paragraph into two sentences might improve readability:
"During data restore, the target table is automatically set to
restoremode, which prevents all read and write operations. After restore completes, the mode automatically reverts tonormalto resume normal operations. This mechanism ensures task stability and data consistency."br/br-snapshot-guide.md (1)
106-107: Consider simplifying the wording for better readability.The explanation matches the one in br-pitr-guide.md, which is good for consistency. The same optional improvement applies: "the table mode of the target table" could be simplified to "the target table mode" for conciseness.
ddl_embedded_analyze.md (2)
15-44: Consider breaking up the long explanation paragraph.The usage scenario is well-explained with a concrete example. However, the paragraph at line 42 is quite long (6 sentences) and covers multiple concepts: the problem, heuristic behavior, potential issues, TiDB improvements, and the solution. Consider breaking it into 2-3 shorter paragraphs for better readability:
- First paragraph: Explain the problem and heuristic behavior
- Second paragraph: Mention TiDB v8.5.0 improvements
- Third paragraph: Introduce the embedded ANALYZE solution and the system variable
46-111: Well-documented ADD INDEX section with comprehensive examples.The section effectively demonstrates the feature with clear examples and verification steps. The timeout mechanism explanation is important and well-placed. The progression from DDL execution to verification (EXPLAIN → SHOW STATS_HISTOGRAMS → ADMIN SHOW DDL JOBS) provides a complete picture of the feature's behavior.
One minor suggestion: Consider adding a brief explanation of what "Reorg phase" means for readers unfamiliar with DDL internals, or link to relevant documentation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
OWNERS(1 hunks)OWNERS_ALIASES(2 hunks)TOC-tidb-cloud.md(1 hunks)TOC.md(1 hunks)br/br-pitr-guide.md(1 hunks)br/br-snapshot-guide.md(1 hunks)ddl_embedded_analyze.md(1 hunks)dm/dm-compatibility-catalog.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.md
⚙️ CodeRabbit configuration file
As a technical writer in the database area, you need to first learn the changes in this PR, and then improve the content to make it clear, concise, and easy to understand in English according to your database knowledge.
Files:
dm/dm-compatibility-catalog.mdddl_embedded_analyze.mdbr/br-pitr-guide.mdTOC-tidb-cloud.mdTOC.mdbr/br-snapshot-guide.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: tidb-cloud-check
- GitHub Check: tidb-check
🔇 Additional comments (7)
dm/dm-compatibility-catalog.md (1)
23-25: LGTM! Clear compatibility documentation.The new MySQL version compatibility entries are well-structured and informative. The GitHub issue reference for MySQL 8.4 incompatibility is particularly helpful for users encountering compatibility issues.
OWNERS (1)
1-8: LGTM! Clean migration to filter-based ownership model.The restructuring from static lists to a filter-based schema is well-implemented. The references to
sig-community-approversandsig-community-reviewersalign with the groups defined in OWNERS_ALIASES, and the updated documentation URL is current.OWNERS_ALIASES (1)
1-51: LGTM! Well-organized ownership groups.The new
sig-community-reviewersandsig-community-approversgroups are properly structured and support the filter-based ownership model in the OWNERS file. The header comments clearly explain the syncing mechanism.ddl_embedded_analyze.md (2)
1-14: Clear introduction with proper context.The front matter and introduction are well-structured. The version mark is correctly placed, and the explanation of the two DDL statement types is clear. The benefit statement effectively communicates why this feature is important.
112-176: Consistent and comprehensive documentation for index reorganization DDLs.The section maintains excellent parallel structure with the ADD INDEX section, making it easy for readers to understand the consistent behavior across different DDL types. The MODIFY COLUMN example effectively demonstrates the feature, and the verification steps confirm proper statistics collection.
TOC.md (1)
1091-1091: LGTM! Well-placed TOC entry.The new entry is properly placed in the Reference section near related DDL content, with consistent formatting and correct link path.
TOC-tidb-cloud.md (1)
842-842: LGTM! Appropriate placement in TiDB Cloud TOC.The entry is well-positioned adjacent to DDL-related documentation with consistent formatting and correct link path.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
system-variables.md (1)
1795-1804: Fix misplacedtidb_opt_selectivity_factor, restore structure, and standardize labels.
- The block is inserted inside the
tidb_enable_fast_create_tablesection, breaking document structure.- Standardize labels: “Persists to cluster”, “Applies to hint”, “Type: Float”, “Range”.
- Heading should not be code-formatted and use “New in …”.
- Relocate alphabetically after
tidb_opt_seek_factor.Apply (remove here):
-### `tidb_opt_selectivity_factor` <span class="version-mark">Introduced in v9.0.0</span> - -- Scope: SESSION | GLOBAL -- Persist to cluster: Yes -- Controlled by Hint [SET_VAR](/optimizer-hints.md#set_varvar_namevar_value): Yes -- Type: Floating-point number -- Range: `[0, 1]` -- Default value: `0.8` -- This variable is used to specify the default selectivity of the TiDB optimizer. In some cases, when the optimizer cannot derive the predicate selectivity based on statistics, the optimizer uses this default selectivity as an alternative value. **It is not recommended** to modify this value.Insert after the
### tidb_opt_seek_factorentry:+### tidb_opt_selectivity_factor <span class="version-mark">New in v9.0.0</span> + +- Scope: SESSION | GLOBAL +- Persists to cluster: Yes +- Applies to hint [SET_VAR](/optimizer-hints.md#set_varvar_namevar_value): Yes +- Type: Float +- Range: `[0, 1]` +- Default value: `0.8` +- Sets the fallback selectivity the optimizer uses when predicate selectivity cannot be derived from statistics. Changing this can affect cardinality estimates and plan selection. It is not recommended to modify this value; prefer collecting or updating statistics instead.
🧹 Nitpick comments (1)
system-variables.md (1)
1659-1666: Add missing Type and keep alphabetical order for system variables.
- Add “Type: Boolean” to this entry to match house style.
- Consider relocating this variable under the
tidb_stats_*section to maintain alphabetical order acrosstidb_*variables.Apply:
### tidb_stats_update_during_ddl <span class="version-mark">New in v8.5.4 and v9.0.0</span> - Scope: GLOBAL - Persists to cluster: Yes - Applies to hint [SET_VAR](/optimizer-hints.md#set_varvar_namevar_value): No +- Type: Boolean - Default value: `OFF` - This variable controls whether to enable DDL-embedded `ANALYZE`. When enabled, DDL statements that create new indexes ([`ADD INDEX`](/sql-statements/sql-statement-add-index.md)) or reorganize existing indexes ([`MODIFY COLUMN`](/sql-statements/sql-statement-modify-column.md) and [`CHANGE COLUMN`](/sql-statements/sql-statement-change-column.md)) automatically collect statistics before the index becomes visible. For more information, see [`ANALYZE` Embedded in DDL Statements](/ddl_embedded_analyze.md).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
system-variables.md(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.md
⚙️ CodeRabbit configuration file
As a technical writer in the database area, you need to first learn the changes in this PR, and then improve the content to make it clear, concise, and easy to understand in English according to your database knowledge.
Files:
system-variables.md
…ingcap#19275)
First-time contributors' checklist
What is changed, added or deleted? (Required)
Which TiDB version(s) do your changes apply to? (Required)
Tips for choosing the affected version(s):
By default, CHOOSE MASTER ONLY so your changes will be applied to the next TiDB major or minor releases. If your PR involves a product feature behavior change or a compatibility change, CHOOSE THE AFFECTED RELEASE BRANCH(ES) AND MASTER.
For details, see tips for choosing the affected versions.
What is the related PR or file link(s)?
Do your changes match any of the following descriptions?
Summary by CodeRabbit
New Features
Documentation