lh2debug-2 commented on PR #3421: URL: https://github.com/apache/brpc/pull/3421#issuecomment-5162708593
> There is a scenario like this: > > 1. Server receive a request and create a local parent span > 2. During processing this request, a client start a new async call, create a child span > 3. Server send the response before the client return, the local parent span submit, and also submit the child span > 4. The client's response return, because the client span's parent is now expired, so the client will submit the span, too. > 5. Consequently, the client span is submitted twice. > > Is that right? Thanks for pointing this out. For the exact case where the parent span is submitted while the async child span is still active, the child is not serialized under the parent, because `SpanDB::Index()` only collects inactive child spans. Later, when the child finishes and its local parent has expired, it is submitted as a standalone client span. So that exact case does not duplicate the child. The real duplicate risk is a nearby race: the child finishes first, the parent dump serializes it into `parent.client_spans`, then the child later submits itself after the parent expires. I fixed this by making rpcz collection idempotent. Each `Span` now has a submitted flag. Both paths must claim it before writing: - standalone submit: `Span::Submit()` - nested child serialization: `SpanDB::Index()` | Case | Result | |---|---| | Root server span | submitted once as root | | Root client span | submitted once as root | | Child span, parent alive | not standalone submitted | | Child ended, parent dumps first | nested under parent; later standalone submit skipped | | Parent dumps while child active | not nested; later submitted standalone | Performance impact should be negligible. The flag is `butil::atomic<bool>` with relaxed ordering, `sizeof(Span)` does not increase, and the CAS is only on rpcz submit/dump paths. In the duplicate case it also reduces LevelDB writes by avoiding the extra standalone child record. -- 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]
