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

refactor(storagenode): committer to remove requireCommitWaitTasks parameter #969

Merged

Conversation

ijsong
Copy link
Member

@ijsong ijsong commented Jan 21, 2025

What this PR does

This PR removes the requireCommitWaitTasks parameter from the commitInternal
method. The parameter was used to support sync replication, which is the
recovery between two replicas in a log stream. However, sync replication doesn't
use commitInternal anymore.

@ijsong ijsong requested a review from hungryjang as a code owner January 21, 2025 12:30
Copy link

codecov bot commented Jan 21, 2025

Codecov Report

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

Project coverage is 80.04%. Comparing base (42003ae) to head (c62ffc5).
Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
internal/storagenode/logstream/committer.go 80.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #969      +/-   ##
==========================================
- Coverage   80.22%   80.04%   -0.18%     
==========================================
  Files         178      178              
  Lines       21505    21484      -21     
==========================================
- Hits        17252    17197      -55     
- Misses       3474     3500      +26     
- Partials      779      787       +8     

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

@ijsong ijsong force-pushed the remove_deferred_write_batch_code branch from bbcb5b1 to 3e1d1de Compare January 23, 2025 12:09
@ijsong ijsong force-pushed the refactor_sn_committer_to_remove_requirecommitwaittasks_parameter branch from 75d32d4 to b74daea Compare January 23, 2025 12:09
@ijsong ijsong force-pushed the remove_deferred_write_batch_code branch from 3e1d1de to 176898b Compare February 3, 2025 10:58
@ijsong ijsong force-pushed the refactor_sn_committer_to_remove_requirecommitwaittasks_parameter branch from b74daea to efdf19f Compare February 3, 2025 10:58
@ijsong ijsong force-pushed the remove_deferred_write_batch_code branch from 176898b to a5563c6 Compare February 3, 2025 11:25
@ijsong ijsong force-pushed the refactor_sn_committer_to_remove_requirecommitwaittasks_parameter branch from efdf19f to d89b402 Compare February 3, 2025 11:25
@ijsong ijsong force-pushed the remove_deferred_write_batch_code branch from a5563c6 to 2c91418 Compare February 7, 2025 05:34
@ijsong ijsong force-pushed the refactor_sn_committer_to_remove_requirecommitwaittasks_parameter branch from d89b402 to 39badb5 Compare February 7, 2025 05:35
@ijsong ijsong force-pushed the remove_deferred_write_batch_code branch from 2c91418 to d038320 Compare February 18, 2025 23:54
@ijsong ijsong force-pushed the refactor_sn_committer_to_remove_requirecommitwaittasks_parameter branch from 39badb5 to 6a78fac Compare February 18, 2025 23:54
@ijsong ijsong force-pushed the remove_deferred_write_batch_code branch from d038320 to f0e2f6d Compare February 19, 2025 01:15
@ijsong ijsong force-pushed the refactor_sn_committer_to_remove_requirecommitwaittasks_parameter branch from 6a78fac to 836ae8b Compare February 19, 2025 01:15
@ijsong ijsong force-pushed the remove_deferred_write_batch_code branch from f0e2f6d to 0112600 Compare February 21, 2025 09:00
@ijsong ijsong force-pushed the refactor_sn_committer_to_remove_requirecommitwaittasks_parameter branch from 836ae8b to 1467dc3 Compare February 21, 2025 09:00
@ijsong ijsong force-pushed the remove_deferred_write_batch_code branch from 0112600 to 8b14788 Compare February 21, 2025 10:16
@ijsong ijsong force-pushed the refactor_sn_committer_to_remove_requirecommitwaittasks_parameter branch from 1467dc3 to 11e60ba Compare February 21, 2025 10:16
@ijsong ijsong force-pushed the remove_deferred_write_batch_code branch from 8b14788 to 6b9b962 Compare February 21, 2025 12:59
@ijsong ijsong force-pushed the refactor_sn_committer_to_remove_requirecommitwaittasks_parameter branch from 11e60ba to 769148b Compare February 21, 2025 12:59
Removed the llsnList from the replicateTask to improve performance and simplify
the code. The beginLLSN field is now used instead. This change eliminates the
need to fill llsnList, resulting in a slight performance improvement.
Removed dead code that was commented out. This improves code readability and
maintainability by eliminating dead code.
…ameter

This PR removes the requireCommitWaitTasks parameter from the commitInternal
method. The parameter was used to support sync replication, which is the
recovery between two replicas in a log stream. However, sync replication doesn't
use commitInternal anymore.
@ijsong ijsong force-pushed the remove_deferred_write_batch_code branch from 6b9b962 to b193b68 Compare February 21, 2025 13:20
@ijsong ijsong force-pushed the refactor_sn_committer_to_remove_requirecommitwaittasks_parameter branch from 769148b to c62ffc5 Compare February 21, 2025 13:20
Base automatically changed from remove_deferred_write_batch_code to main February 21, 2025 13:36
@ijsong ijsong merged commit b9d2565 into main Feb 21, 2025
9 checks passed
@ijsong ijsong deleted the refactor_sn_committer_to_remove_requirecommitwaittasks_parameter branch February 21, 2025 13:37
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