Skip to content

[BTS-1732, BTS-2177] Fix stopping the rocksdb sync thread during shutdown #21912

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

Open
wants to merge 5 commits into
base: devel
Choose a base branch
from

Conversation

goedderz
Copy link
Member

@goedderz goedderz commented Aug 14, 2025

Scope & Purpose

A test running into a maintainer mode assertion in a commit during shutdown revealed possible races regarding the RocksDBSyncThread during shutdown.

The following changes are made in this PR:

  • After the RocksDBSyncThread has been stopped, it no longer executes listeners.
    This directly addresses BTS-1732, where the listener scheduled a task after the Scheduler was stopped.
  • The thread is still stopped in RocksDBEngine::stop(), but its destruction was moved to RocksDBEngine::unprepare().
    Data structures should always be destroyed and deleted in unprepare, not in stop. This could have lead to use-after-free errors during shutdown (BTS-2177).
  • When Replication 2 is disabled, the RocksDBEngine no longer creates a log persistor for it, nor registers it as a sync listener in the RocksDBSyncThread. This listener was the one posting on the Scheduler, triggering the assertion.
  • 💩 Bugfix

Checklist

  • 📖 CHANGELOG entry made

Related Information

@goedderz goedderz self-assigned this Aug 14, 2025
@cla-bot cla-bot bot added the cla-signed label Aug 14, 2025
@goedderz goedderz marked this pull request as ready for review August 14, 2025 14:34
@goedderz goedderz requested review from a team and mchacki August 14, 2025 14:35
@goedderz goedderz changed the title [BTS-1732] Fix stopping the rocksdb sync thread during shutdown [BTS-1732, BTS-2177] Fix stopping the rocksdb sync thread during shutdown Aug 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant