Skip to content

Skip ccpage that has no dirty keys since last datasync during datasyn…#374

Merged
yi-xmu merged 1 commit intomainfrom
skip_ccpages_without_dirty_keys
Jan 21, 2026
Merged

Skip ccpage that has no dirty keys since last datasync during datasyn…#374
yi-xmu merged 1 commit intomainfrom
skip_ccpages_without_dirty_keys

Conversation

@yi-xmu
Copy link
Collaborator

@yi-xmu yi-xmu commented Jan 20, 2026

During the DataSyncScan for a range partition, skip ccpages that have no dirty keys since the last DataSync.

remove useless variable

Here are some reminders before you submit the pull request

  • Add tests for the change
  • Document changes
  • Reference the link of issue using fixes eloqdb/tx_service#issue_id
  • Reference the link of RFC if exists
  • Pass ./mtr --suite=mono_main,mono_multi,mono_basic

Summary by CodeRabbit

  • Improvements

    • Data synchronization now uses timestamp-based tracking for finer sync/delta detection.
    • Scans skip partitions/pages with no new updates, improving efficiency and batch progress.
    • Enhanced boundary handling across pages and slices to better determine pause/resume points and reduce unnecessary work.
  • Bug Fixes

    • Added tighter safety checks to prevent out-of-bounds slice access.

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

@yi-xmu yi-xmu self-assigned this Jan 20, 2026
@coderabbitai
Copy link

coderabbitai bot commented Jan 20, 2026

Walkthrough

Replaced a boolean persisted-data flag with a uint64_t last-data-sync timestamp in RangePartitionDataSyncScanCc; added accessor and member, removed the boolean member, tightened a slice-index assertion, updated TemplateCcMap to skip pages with no updates since the timestamp, and updated call sites to pass the timestamp.

Changes

Cohort / File(s) Summary
Data Sync Scan Class Definition
tx_service/include/cc/cc_request.h
Replaced bool include_persisted_data constructor param with uint64_t last_data_sync_ts = 0; added uint64_t last_data_sync_ts_ and LastDataSyncTs(); removed include_persisted_data_; added assertion curr_slice_index_ < slices_to_scan_.size() in CurrentSliceKey().
Template Map Scanning Logic
tx_service/include/cc/template_cc_map.h
Added next_page_it lambda and slice_end_next_page_it usage; changed loop boundary checks to consider next-page boundaries; skip cc pages whose last_dirty_commit_ts_LastDataSyncTs() and advance slices/pages accordingly; adjusted next-pause-key and export decision logic and recomputed boundaries after advancing.
Call-site Updates
tx_service/src/cc/local_cc_shards.cpp, tx_service/src/sk_generator.cpp
Updated RangePartitionDataSyncScanCc constructor calls to pass last_sync_ts or 0 (uint64_t) instead of false.

Sequence Diagram(s)

(omitted)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • liunyl
  • MrGuin

Poem

🐰 I hop through pages where timestamps gleam,

I skip the stale tiles and follow the stream.
No boolean burden, just moments to trust,
Slices align — boundaries robust and just.
A tiny rabbit cheers this tidy adjust!

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description lacks required information: no issue reference (missing 'fixes eloqdb/tx_service#issue_id'), no RFC reference noted, and checklist items are not completed or verified. Add issue reference using 'fixes eloqdb/tx_service#issue_id' format, document the change purpose, and verify that all checklist items have been addressed or mark them accordingly.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: skipping ccpages without dirty keys during datasync for range partitions, which aligns with the changeset modifications.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch skip_ccpages_without_dirty_keys

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.

@yi-xmu yi-xmu force-pushed the skip_ccpages_without_dirty_keys branch from f326d2d to 2063d1a Compare January 20, 2026 07:13
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: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
tx_service/include/cc/template_cc_map.h (1)

5671-5727: Don’t skip clean pages when exporting persisted keys for split slices.

In checkpoint mode, a pinned slice (export_persisted_key_only == true) needs all persisted keys to derive subslice boundaries. The new skip path can drop clean pages and produce incomplete exports. Guard the skip with !export_persisted_key_only (or !slice_pinned).

🐛 Suggested fix
-            if (!req.export_base_table_item_ &&
-                ccp->last_dirty_commit_ts_ <= req.LastDataSyncTs())
+            if (!req.export_base_table_item_ && !export_persisted_key_only &&
+                ccp->last_dirty_commit_ts_ <= req.LastDataSyncTs())

@yi-xmu yi-xmu force-pushed the skip_ccpages_without_dirty_keys branch from 2063d1a to a432025 Compare January 20, 2026 08:05
@yi-xmu
Copy link
Collaborator Author

yi-xmu commented Jan 20, 2026

Update submodule pr:
eloqdata/eloqsql#218
eloqdata/eloqdoc#435

@yi-xmu yi-xmu requested a review from liunyl January 20, 2026 10:20
@yi-xmu yi-xmu merged commit fe1c2f3 into main Jan 21, 2026
4 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Feb 11, 2026
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants