8000 Fix a rare shutdown race in RocksDBShaCalculatorThread by jsteemann · Pull Request #14709 · arangodb/arangodb · GitHub
[go: up one dir, main page]
Skip to content

Fix a rare shutdown race in RocksDBShaCalculatorThread #14709

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Sep 3, 2021
Merged

Fix a rare shutdown race in RocksDBShaCalculatorThread #14709

merged 7 commits into from
Sep 3, 2021

Conversation

jsteemann
Copy link
Contributor

Scope & Purpose

Backport of #14708

Fix a rare shutdown race in RocksDBShaCalculatorThread.
The RocksDBShaCalculatorThread object is owned by the RocksDBShaCalculator, which itself is owned by the RocksDBEngine. The RocksDBEngine has a shared_pointer on the RocksDBShaCalculator, and the shared_ptr is only resetted in the dtor of RocksDBEngine.
This may be too late, because it means the RocksDBShaCalculatorThread can still run while the dtor of RocksDBEngine is running. The thread may also access other features from the ApplicationServer which have been shut down at that point.
The fix is to wait out the end of RocksDBShaCalculator's run method in RocksDBEngine::stop(), so that when we are past it we can be sure that the RocksDBShaCalculatorThread is not running and accessing any other feature.

This issue was uncovered by nightly chaos tests in https://jenkins.arangodb.biz/job/arangodb-ANY-windows-matrix/2440/EDITION=enterprise,STORAGE_ENGINE=rocksdb,TEST_SUITE=cluster,limit=test&&windows/artifact/testfailures.log

  • 💩 Bugfix (requires CHANGELOG entry)
  • 🍕 New feature (requires CHANGELOG entry, feature documentation and release notes)
  • 🔥 Performance improvement
  • 🔨 Refactoring/simplification
  • 📖 CHANGELOG entry made

Backports:

  • No backports required

Testing & Verification

  • This change is a trivial rework / code cleanup without any test coverage.
  • The behavior in this PR was manually tested

@jsteemann jsteemann added this to the 3.8 milestone Aug 26, 2021
@jsteemann jsteemann requested review from cpjulia and mpoeter August 26, 2021 20:38
cpjulia
cpjulia approved these changes Aug 29, 2021
View reviewed changes
Copy link
Contributor
@cpjulia cpjulia left a comment

Choose a reason for hiding this comment

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

LGTM

@KVS85 KVS85 added the 3 RocksDB Storage engine related label Sep 3, 2021
@KVS85 KVS85 merged commit f093b26 into 3.8 Sep 3, 2021
@KVS85 KVS85 deleted the bug-fix-3.8/fix-sha-thread-shutdown branch September 3, 2021 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cpjulia cpjulia cpjulia approved these changes

@mpoeter mpoeter Awaiting requested review from mpoeter

Assignees

@jsteemann jsteemann

Labels

3 RocksDB Storage engine related

Projects

None yet

Milestone

3.8

Development

Successfully merging this pull request may close these issues.

4 participants

@jsteemann @cpjulia @KVS85 @goedderz
0