zchuango commented on PR #3290:
URL: https://github.com/apache/brpc/pull/3290#issuecomment-5340695771

   Thanks for the detailed review and testing.
   
   Since the UBRing/UBShmTransport was originally contributed in #3290, we'd 
like to address these findings as part of a broader UBShmTransport improvement 
effort, with the changes split into focused PRs to make the improvements easier 
to review and track.
   
   After an initial review, we plan to organize the follow-up work into the 
following phases. The exact PR boundaries may be adjusted as we validate the 
implementation details and receive feedback from the community.
   
   ### Phase 1: Code quality and maintainability
   
   We will first address low-risk maintainability issues, including:
   
   - removing debugging leftovers;
   - improving unclear naming and RDMA-specific terminology in UBRing code;
   - clarifying time-related configuration units and comments.
   
   These changes should not affect runtime behavior and can help make 
subsequent improvements easier to review.
   
   ### Phase 2: Timer and lifecycle stability
   
   We will prioritize the timer-related correctness and lifecycle issues.
   
   This phase will cover:
   
   - reproducing and fixing timer deletion and cleanup issues;
   - reviewing callback/deletion lifecycle races;
   - improving stale fd / fd reuse safety;
   - avoiding potentially blocking cleanup paths.
   
   We will also evaluate whether the current custom timerfd/epoll-based 
implementation can be replaced with brpc's existing timer infrastructure, such 
as `bthread::TimerThread`, if it provides a cleaner lifecycle model.
   
   ### Phase 3: Backend abstraction and data-path improvements
   
   We will further review the differences between IPC and UBS backends, 
especially around:
   
   - message layout and payload size;
   - publication semantics and memory ordering;
   - backend-specific optimization opportunities.
   
   For message format or capability negotiation, we will consider aligning with 
the common handshake discussion in #3432 instead of introducing an independent 
mechanism.
   
   ### Phase 4: Test coverage and continuous hardening
   
   Regression and end-to-end tests will be added together with each 
improvement, covering areas such as:
   
   - timer lifecycle;
   - disconnect cleanup;
   - large attachments;
   - multiple concurrent connections;
   - backend-specific data paths.
   
   We will keep the changes in focused PRs and link each PR back to this issue 
so that the progress can be tracked here.
   
   We welcome further feedback and suggestions from the community. Please feel 
free to ping me if there are any concerns or if you would like to discuss any 
of the proposed directions. We will keep this issue updated as the work 
progresses.
   @ZhengweiZhu @wwbmmm @chenBright 


-- 
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]

Reply via email to