Skip to content

Conversation

Ryan4253
Copy link
Contributor

@Ryan4253 Ryan4253 commented Jun 6, 2025

Closes #3017

Description of changes:

Removes extraneous calls to rocksdb::CancelAllBackgroundWork. See the issue for further explanation.

Test Plan:

Unit Test

[----------] Global test environment tear-down
[==========] 430 tests from 69 test suites ran. (20101 ms total)
[  PASSED  ] 429 tests.
[  SKIPPED ] 1 test, listed below:
[  SKIPPED ] UseBitmap/RedisBitmapTest.BitfieldStringGetSetTest/0

Logs

Before

2025/06/05-14:48:25.726530 66707 [db/db_impl/db_impl.cc:463] Shutdown: canceling all background work
2025/06/05-14:48:25.819216 66707 [db/db_impl/db_impl.cc:463] Shutdown: canceling all background work
2025/06/05-14:48:25.819338 66707 [db/db_impl/db_impl.cc:463] Shutdown: canceling all background work

After

2025/06/05-14:14:15.002071 42139 [db/db_impl/db_impl.cc:463] Shutdown: canceling all background work

@Ryan4253 Ryan4253 marked this pull request as ready for review June 6, 2025 13:20
@git-hulk
Copy link
Member

git-hulk commented Jun 7, 2025

@Ryan4253 It's expected to call this explicitly to prevent the data race between background threads and the db destructor, see https://github.com/apache/kvrocks/actions/runs/15493199236/job/43625063455?pr=3019#step:17:533.

@Ryan4253 Ryan4253 force-pushed the unstable branch 2 times, most recently from dc8c599 to 71a23f5 Compare June 9, 2025 13:18
@Ryan4253 Ryan4253 marked this pull request as draft June 9, 2025 14:53
@Ryan4253
Copy link
Contributor Author

Ryan4253 commented Jun 9, 2025

@git-hulk thanks for pointing it out! This should be a more correct version that would only cleanup once.

@Ryan4253 Ryan4253 marked this pull request as ready for review June 9, 2025 19:28
Copy link

@git-hulk git-hulk merged commit 8a50e51 into apache:unstable Jun 10, 2025
69 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.

Kvrocks logs extraneous shutdowns in RocksDB
2 participants