Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

skip append dummy logs to log dev #624

Merged
merged 1 commit into from
Jan 16, 2025

Conversation

Besroy
Copy link
Contributor

@Besroy Besroy commented Jan 8, 2025

During the baseline resynchronization process, when a follower successfully receives a snapshot, it calls the compact function to truncate all logs that are no larger than snapshot.last_log_idx. In the current implementation, compact initially appends dummy logs up to snapshot.last_log_idx before truncating them. This operation is resource-intensive, and the size of the data waiting to be flushed can exceed the journal vdev's chunk size, posing a risk of system crashes.

This change eliminates the need to append dummy logs to the log device by directly updating certain indices. It also removes the dependency of log dev truncation on log store's LSN.

src/lib/logstore/log_store.cpp Outdated Show resolved Hide resolved
@Besroy Besroy force-pushed the skip_dummpy_logs_in_compact branch 2 times, most recently from 86fefef to b78115c Compare January 8, 2025 13:53
@codecov-commenter
Copy link

codecov-commenter commented Jan 8, 2025

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 94.11765% with 1 line in your changes missing coverage. Please review.

Project coverage is 66.14%. Comparing base (1a0cef8) to head (c6275a4).
Report is 121 commits behind head on master.

Files with missing lines Patch % Lines
src/lib/logstore/log_dev.cpp 83.33% 0 Missing and 1 partial ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #624      +/-   ##
==========================================
+ Coverage   56.51%   66.14%   +9.63%     
==========================================
  Files         108      109       +1     
  Lines       10300    10988     +688     
  Branches     1402     1507     +105     
==========================================
+ Hits         5821     7268    +1447     
+ Misses       3894     3005     -889     
- Partials      585      715     +130     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Besroy Besroy force-pushed the skip_dummpy_logs_in_compact branch from b78115c to 8f015e3 Compare January 10, 2025 12:44
src/lib/logstore/log_dev.cpp Outdated Show resolved Hide resolved
src/lib/logstore/log_store.cpp Outdated Show resolved Hide resolved
src/lib/logstore/log_store.cpp Show resolved Hide resolved
@Besroy Besroy force-pushed the skip_dummpy_logs_in_compact branch 2 times, most recently from 19878fd to b8f76c6 Compare January 14, 2025 10:22
@JacksonYao287 JacksonYao287 requested a review from sanebay January 15, 2025 00:34
@JacksonYao287
Copy link
Contributor

@sanebay pls also take a look

@Besroy Besroy force-pushed the skip_dummpy_logs_in_compact branch from b8f76c6 to d4368df Compare January 15, 2025 00:54
src/lib/logstore/log_store.cpp Show resolved Hide resolved
src/tests/test_log_dev.cpp Outdated Show resolved Hide resolved
src/tests/test_log_dev.cpp Show resolved Hide resolved
@Besroy Besroy force-pushed the skip_dummpy_logs_in_compact branch from d4368df to dc053d7 Compare January 15, 2025 07:35
1. directly update indices when there are holes in baseline resync
2. update definition of logdev_key::out_of_bound_ld_key and use it to imply log dev can truncate freely.
3. remove is_active check in HomeLogStore::flush to unblock flush when there are holes in m_records
@Besroy Besroy force-pushed the skip_dummpy_logs_in_compact branch from dc053d7 to c6275a4 Compare January 15, 2025 07:42
Copy link
Contributor

@JacksonYao287 JacksonYao287 left a comment

Choose a reason for hiding this comment

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

LGTM, one small question

src/lib/logstore/log_dev.cpp Show resolved Hide resolved
@xiaoxichen xiaoxichen merged commit 16b0e36 into eBay:master Jan 16, 2025
21 checks passed
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.

4 participants