Skip to content

Comments

Update subm for fix assert#188

Merged
yi-xmu merged 2 commits intoeloq-10.6.10from
update_subm_fix_assert
Dec 18, 2025
Merged

Update subm for fix assert#188
yi-xmu merged 2 commits intoeloq-10.6.10from
update_subm_fix_assert

Conversation

@yi-xmu
Copy link
Collaborator

@yi-xmu yi-xmu commented Dec 18, 2025

Summary by CodeRabbit

  • Tests
    • Removed internal MySQL test scenarios related to range-split deadlock handling and concurrent read-blocking, including associated test configuration changes.
  • Chores
    • Updated a subproject/infrastructure reference to a new commit version.

✏️ Tip: You can customize this high-level summary in your review settings.

@yi-xmu yi-xmu self-assigned this Dec 18, 2025
@coderabbitai
Copy link

coderabbitai bot commented Dec 18, 2025

Walkthrough

The PR advances the data_substrate submodule to a new commit and removes several MySQL test files (and one test result) under storage/eloq/mysql-test/mono_basic/ related to range-split, read-blocking, and deadlock-abort scenarios.

Changes

Cohort / File(s) Summary
Submodule Update
data_substrate
Subproject commit updated from 7c0a42621ceaae1283ae0a8a531796b49c547a3a to f74a44e73215a180839a0d0b01114a3e991d5c2c.
Range Read Block on Write Lock Tests
storage/eloq/mysql-test/mono_basic/t/range_read_block_on_write_lock.opt, storage/eloq/mysql-test/mono_basic/t/range_read_block_on_write_lock.test
Removed test option file (deleted checkpointer_interval=86400) and removed the multi-case end-to-end test script that validated range-split behavior under write locks and read blocking.
Range Split Deadlock Abort Tests
storage/eloq/mysql-test/mono_basic/r/range_split_deadlock_abort.result, storage/eloq/mysql-test/mono_basic/t/range_split_deadlock_abort.opt, storage/eloq/mysql-test/mono_basic/t/range_split_deadlock_abort.test
Removed test result, option, and test script that exercised a range-split deadlock-abort scenario (fault-injection at phase 8, lock downgrade and retry verification).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Focus review on confirming the intent to remove these tests and that CI/test plans are adjusted.
  • Verify the submodule pointer targets the intended upstream commit.

Possibly related PRs

Suggested reviewers

  • githubzilla
  • liunyl

Poem

🐰
I hopped through tests at break of day,
Old scripts and checks I swept away.
A submodule turned, a tidy spree—
Quiet ranges now bounce free. ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Update subm for fix assert' is vague and does not clearly convey the main changes—removing test files for range split scenarios and updating a subproject commit. Consider a more descriptive title that clarifies the primary change, such as 'Remove range split test scenarios and update subproject commit' or 'Clean up range split test files'
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch update_subm_fix_assert

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 53686c9 and 5a8c570.

📒 Files selected for processing (1)
  • data_substrate (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • data_substrate

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3e820a6 and 53686c9.

📒 Files selected for processing (6)
  • data_substrate (1 hunks)
  • storage/eloq/mysql-test/mono_basic/r/range_split_deadlock_abort.result (0 hunks)
  • storage/eloq/mysql-test/mono_basic/t/range_read_block_on_write_lock.opt (0 hunks)
  • storage/eloq/mysql-test/mono_basic/t/range_read_block_on_write_lock.test (0 hunks)
  • storage/eloq/mysql-test/mono_basic/t/range_split_deadlock_abort.opt (0 hunks)
  • storage/eloq/mysql-test/mono_basic/t/range_split_deadlock_abort.test (0 hunks)
💤 Files with no reviewable changes (5)
  • storage/eloq/mysql-test/mono_basic/t/range_split_deadlock_abort.opt
  • storage/eloq/mysql-test/mono_basic/t/range_read_block_on_write_lock.opt
  • storage/eloq/mysql-test/mono_basic/r/range_split_deadlock_abort.result
  • storage/eloq/mysql-test/mono_basic/t/range_read_block_on_write_lock.test
  • storage/eloq/mysql-test/mono_basic/t/range_split_deadlock_abort.test

data_substrate Outdated
@@ -1 +1 @@
Subproject commit 7c0a42621ceaae1283ae0a8a531796b49c547a3a
Subproject commit 34d63022d6f0a1777c4226f67db268ebf0f3aa98
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash

# 1. Check submodule configuration
echo "=== Submodule Configuration ==="
git config --file .gitmodules --get-all submodule.data_substrate.url 2>/dev/null || echo "No data_substrate submodule found in .gitmodules"

# 2. Check current state of data_substrate in HEAD
echo -e "\n=== Current data_substrate pointer in HEAD ==="
git ls-tree HEAD data_substrate 2>/dev/null || echo "data_substrate not found in HEAD"

# 3. Get details about the commit range for this change
echo -e "\n=== Recent commits affecting data_substrate ==="
git log --oneline -20 -- data_substrate 2>/dev/null || echo "Could not retrieve commit history"

# 4. Check for deleted test files
echo -e "\n=== Check for deleted MySQL test files ==="
git log --diff-filter=D --summary -20 -- 'storage/eloq/mysql-test/mono_basic/*' 2>/dev/null | head -30 || echo "No recent deletions found"

# 5. Check current PR or branch info
echo -e "\n=== Current branch and status ==="
git branch -v 2>/dev/null | head -5
git status --short 2>/dev/null | head -10

# 6. Check if we have access to view the .gitmodules file
echo -e "\n=== .gitmodules content ==="
cat .gitmodules 2>/dev/null || echo ".gitmodules not found"

Repository: eloqdata/eloqsql

Length of output: 1505


🏁 Script executed:

#!/bin/bash

# 1. Show the commit that updated the submodule
echo "=== Commit details for submodule update ==="
git show 53686c96d4 --stat

# 2. Show the actual diff of the submodule pointer change
echo -e "\n=== Diff showing old vs new submodule commit ==="
git show 53686c96d4 -- data_substrate

# 3. Get the parent commit to see what the previous hash was
echo -e "\n=== Parent commit of submodule update ==="
git show 53686c96d4^ -- data_substrate 2>/dev/null || echo "Could not get parent"

# 4. Check git log for what changed in the parent
echo -e "\n=== Files changed in parent commit ==="
git show 53686c96d4^ --stat 2>/dev/null | head -20 || echo "Could not retrieve"

# 5. Try to get more context on the tx_service repository
echo -e "\n=== Submodule details ==="
git config --file .gitmodules --get submodule.data_substrate/tx_service.url

# 6. Look for any related commits or PRs
echo -e "\n=== Last few commits on current branch ==="
git log --oneline -10

Repository: eloqdata/eloqsql

Length of output: 50374


🏁 Script executed:

#!/bin/bash

# Get concise statistics about the submodule commit
echo "=== Commit statistics ==="
git show 53686c96d4 --stat | tail -20

# Get just the short stat summary
echo -e "\n=== Short stat summary ==="
git show 53686c96d4 --shortstat

# Check what the previous commit was
echo -e "\n=== Previous data_substrate commit ==="
git show 53686c96d4~1:data_substrate 2>/dev/null || echo "Could not access previous commit"

# Check if there's any test-related changes mentioned in the diff
echo -e "\n=== Check for test file changes in diff ==="
git show 53686c96d4 -- '*.test' '*.disabled' 'mysql-test/*' 2>/dev/null | head -50 || echo "No test file changes found"

Repository: eloqdata/eloqsql

Length of output: 3441


Clarify the scope and purpose of this massive submodule update before merging.

This commit adds 26,580 files with 9.5 million lines to the data_substrate submodule, yet the PR title ("Update subm for fix assert") and commit message ("update submodule") provide no meaningful explanation. This scale of change to a production-target branch (eloq-10.6.10) requires:

  1. What assertion/issue is being fixed? The vague title does not justify importing an entire MariaDB codebase into the submodule.
  2. Provide commit details: Link to the data_substrate repository's commit 34d63022d6f0a1777c4226f67db268ebf0f3aa98 or include a changelog explaining the changes.
  3. Confirm compatibility: Verify this update does not break existing tests or introduce incompatibilities with the eloq-10.6.10 branch.
  4. Update PR description: Add a detailed summary explaining why this massive update is necessary and what problem it solves.
🤖 Prompt for AI Agents
In data_substrate around lines 1 to 1, the PR adds a huge submodule update
without context; update the PR and commit metadata to clearly justify the
change: state exactly which assertion/bug is being fixed and how, reference the
submodule commit 34d63022d6f0a1777c4226f67db268ebf0f3aa98 with a link to the
upstream repo or paste its changelog, run and attach CI/test results showing
compatibility with eloq-10.6.10 (unit/integration/regression), describe any
API/behavioral changes and migration steps, and expand the PR description to
summarize why importing the large codebase is necessary and confirm no breaking
changes.

@yi-xmu yi-xmu requested a review from liunyl December 18, 2025 09:18
@yi-xmu yi-xmu force-pushed the update_subm_fix_assert branch from 53686c9 to 5a8c570 Compare December 18, 2025 09:26
@yi-xmu yi-xmu merged commit a56b0cd into eloq-10.6.10 Dec 18, 2025
4 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Dec 22, 2025
@yi-xmu yi-xmu deleted the update_subm_fix_assert branch January 5, 2026 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants