chenBright commented on code in PR #3647:
URL: https://github.com/apache/brpc/pull/3647#discussion_r4358019014
##########
src/brpc/span.cpp:
##########
@@ -847,22 +874,31 @@ leveldb::Status SpanDB::RemoveSpansBefore(int64_t tm) {
// Write span into leveldb.
void Span::dump_to_db() {
+ // This must outlive all locals, including the local DB reference: shutdown
+ // must wait for both persistence and any resulting DB destruction.
+ BAIDU_SCOPED_LOCK(g_span_dump_mutex);
+ {
+ BAIDU_SCOPED_LOCK(g_span_db_mutex);
+ if (g_span_ending) {
+ return;
+ }
+ }
StartIndexingIfNeeded();
Review Comment:
The collector already uses a single dump thread and invokes
`dump_and_destroy()` sequentially. This mutex therefore does not reduce
existing persistence parallelism; it coordinates that thread with shutdown.
The lock must cover DB opening, persistence, and local DB-reference cleanup
so shutdown cannot return while these operations still access
teardown-sensitive resources. Narrowing its scope would break that guarantee.
An active-operation counter would provide no concurrency benefit in the current
design.
I have added a comment explaining the existing single-threaded dumping model.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
