Skip to content

Pause state machine before pg_destroy in BR - #326

Merged
yuwmao merged 1 commit into
eBay:mainfrom
yuwmao:pause_sm
Jul 14, 2025
Merged

Pause state machine before pg_destroy in BR#326
yuwmao merged 1 commit into
eBay:mainfrom
yuwmao:pause_sm

Conversation

@yuwmao

@yuwmao yuwmao commented Jul 10, 2025

Copy link
Copy Markdown
Contributor

No description provided.

@codecov-commenter

codecov-commenter commented Jul 10, 2025

Copy link
Copy Markdown

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

Codecov Report

❌ Patch coverage is 6.89655% with 27 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.95%. Comparing base (1746bcc) to head (bf45209).
⚠️ Report is 168 commits behind head on main.

Files with missing lines Patch % Lines
src/lib/homestore_backend/hs_pg_manager.cpp 7.69% 24 Missing ⚠️
...ib/homestore_backend/replication_state_machine.cpp 0.00% 3 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #326      +/-   ##
==========================================
- Coverage   63.15%   61.95%   -1.20%     
==========================================
  Files          32       35       +3     
  Lines        1900     4114    +2214     
  Branches      204      504     +300     
==========================================
+ Hits         1200     2549    +1349     
- Misses        600     1293     +693     
- Partials      100      272     +172     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@JacksonYao287

Copy link
Copy Markdown
Member

is ths ready for review?

@yuwmao
yuwmao force-pushed the pause_sm branch 2 times, most recently from 0ccf7a0 to bdcae36 Compare July 14, 2025 03:44
@yuwmao
yuwmao marked this pull request as ready for review July 14, 2025 03:44
@yuwmao
yuwmao requested review from Besroy and JacksonYao287 July 14, 2025 03:45
Besroy
Besroy previously approved these changes Jul 14, 2025

@Besroy Besroy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@JacksonYao287

Copy link
Copy Markdown
Member

please rebase this PR and update the conan version

@JacksonYao287 JacksonYao287 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

other parts looks good, one one minor comment

std::this_thread::sleep_for(std::chrono::milliseconds(delay.get()));
}
// It's an inifinite loop until the state machine resumes.
home_object_->resume_pg_state_machine(m_snp_rcv_handler->get_context_pg_id());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

https://github.com/eBay/NuRaft/blob/5a3938fe77a6031a160717391eef67e80aecba57/src/handle_snapshot_sync.cxx#L545-L546

I am not sure do we need to resume pg state machine explictly, since after snapshot is applied, nuraft will resume statemachine

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Seems no need to resume explicitly, comment it out.

@yuwmao
yuwmao merged commit 80f4dfe into eBay:main Jul 14, 2025
25 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