Skip to content

[fix](editlog) Add missing edit log roll check in bulk write path#62060

Open
dataroaring wants to merge 1 commit intoapache:masterfrom
dataroaring:fix/editlog-list-roll
Open

[fix](editlog) Add missing edit log roll check in bulk write path#62060
dataroaring wants to merge 1 commit intoapache:masterfrom
dataroaring:fix/editlog-list-roll

Conversation

@dataroaring
Copy link
Copy Markdown
Contributor

Summary

  • logEdit(short op, List<T> entries) increments txId by entries.size() but never checks whether txId has reached Config.edit_log_roll_num
  • Bulk writes through this path (e.g. batch partition operations) never trigger edit log rolling, causing the current journal database to grow without bound
  • Both logEditDirectly() and flushEditLog() correctly check; this fix adds the same check to the bulk write path

Test plan

  • Verify edit log rolls after edit_log_roll_num transactions via bulk write path
  • Verify batch partition operations trigger roll correctly

🤖 Generated with Claude Code

…ulk write path

The logEdit(short op, List<T> entries) method increments txId by
entries.size() but never checks whether txId has reached
Config.edit_log_roll_num. This means bulk writes through this path
(e.g. batch partition operations) never trigger edit log rolling,
causing the current journal database to grow without bound.

Both logEditDirectly() and flushEditLog() correctly check txId against
edit_log_roll_num after incrementing. This fix adds the same check to
the bulk write path for consistency.
Copilot AI review requested due to automatic review settings April 2, 2026 14:28
@Thearas
Copy link
Copy Markdown
Contributor

Thearas commented Apr 2, 2026

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds the missing edit-log rolling threshold check to the bulk logEdit(short op, List<T> entries) path so bulk writes also trigger rolling and prevent unbounded journal DB growth.

Changes:

  • Add Config.edit_log_roll_num threshold check after incrementing txId in the bulk write path
  • Roll edit log and reset txId when the threshold is reached

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

LOG.info("txId {} is equal to or larger than edit_log_roll_num {}, will roll edit.",
txId, Config.edit_log_roll_num);
rollEditLog();
txId = 0;
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

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

Resetting txId to 0 after rolling can drop any remainder when entries.size() pushes txId past the threshold by more than the exact roll amount (e.g., txId goes from 90 to 190 with roll=100). This can delay the next roll because the extra 90 transactions are effectively forgotten. Consider preserving the remainder (e.g., txId %= Config.edit_log_roll_num or txId -= Config.edit_log_roll_num in a loop if multiple thresholds can be crossed) so rolling behavior is consistent with the configured transaction count.

Suggested change
txId = 0;
txId %= Config.edit_log_roll_num;

Copilot uses AI. Check for mistakes.
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.

3 participants