walterzhaoJR commented on PR #3417: URL: https://github.com/apache/brpc/pull/3417#issuecomment-5251286722
@chenBright Thank you again for pointing out the layering problem. I agree that we should not continue refining the current recursive-mutex implementation before agreeing on the intended semantics and dependency direction. I would first like to clarify two requirements for #2888: 1. Should `Variable::describe()` and `PassiveStatus` getters officially support yielding or blocking? 2. If so, must concurrent `hide()` and destruction remain safe while a callback is running? Calling `describe()` outside the VarMap lock is not sufficient by itself because the map currently stores raw `Variable*` pointers. We would need to pin the variable's lifetime while the callback runs. However, `hide()` would then need to wait for active readers, which can still block a worker if `hide()` is called from a bthread while a callback is yielding. If yielding callbacks must be supported safely, the cleanest direction I currently see is to first separate the scheduler/mutex/butex core from its bvar instrumentation. The scheduler core would no longer depend on bvar, while the bvar metrics integration would live in a higher-level component depending on both. After that, bvar could use scheduler-aware synchronization without introducing a dependency cycle. This would be a larger prerequisite refactoring, probably better handled in a separate PR before revisiting the VarMap change. Would you consider this the appropriate direction, or should yielding metric callbacks instead be treated as unsupported behavior? -- 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]
