You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brpc.apache.org by "zchuango (via GitHub)" <gi...@apache.org> on 2026/09/30 07:51:08 UTC
[PR] Add IPC_V2 data path for UBRing IPC (brpc)
zchuango opened a new pull request, #3573:
URL: https://github.com/apache/brpc/pull/3573
### What problem does this PR solve?
Issue Number: #3463
Problem Summary:
This is the second PR in Phase 3 of Issue #3463.
The first Phase 3 PR (#3507) introduced backward-safe UBRing data-format negotiation, but both IPC and UBS still use the existing `LEGACY_64` data path. It divides data into 60-byte payload chunks and stages each chunk through `local_msg_space` before publishing a 64-byte slot with `Copy64Byte`.
This design keeps the two shared-memory backends on a common data path, but it prevents local IPC from using larger slots and copying `iovec` data directly into shared memory.
This PR adds the negotiated `IPC_V2` format for the IPC backend. `IPC_V2` uses 4096-byte slots and a dedicated SPSC ring with direct `iovec`-to-shared-memory copying. It is enabled only when both peers select `IPC_V2`; otherwise, the connection safely falls back to TCP.
The UBS backend continues to select `LEGACY_64`, and its existing data path remains unchanged.
### What is changed and the side effects?
Changed:
- Add `UBR_DATA_FORMAT_IPC_V2`.
- Select the preferred data format according to the local shared-memory backend:
- IPC selects `IPC_V2`.
- UBS continues to select `LEGACY_64`.
- Require both peers to select the same supported format before enabling UBRing.
- Fall back to TCP when the selected formats do not match.
- Add a fixed `IPC_V2` slot layout:
- 4096-byte slot;
- 16-byte header;
- 4080-byte payload;
- 64-byte alignment;
- explicit `EMPTY` and `READY` ownership states.
- Add an SPSC `IPC_V2` ring using release/acquire publication and recycling semantics.
- Copy `iovec` data directly into `IPC_V2` shared-memory slots instead of first copying it into the legacy 64-byte intermediate message buffer.
- Add `IPC_V2` support for:
- `writev` and `readv`;
- multi-slot messages;
- partial reads;
- ring wrap-around;
- backpressure;
- readable/writable checks;
- peer notification.
- Keep format-specific capacity, cursor, and initialization state in the existing UBRing transaction.
- Initialize each peer's local receive queue before enabling the `IPC_V2` data path.
- Use the shared-memory length carried by the existing 64-byte Base Hello when mapping the remote queue.
- Add focused tests for the `IPC_V2` layout, queue mapping, initialization, format selection, read/write dispatch, boundary conditions, partial reads, wrap-around, backpressure, invalid input, error mapping, and concurrent SPSC operation.
- Keep the existing `LEGACY_64` slot layout, 60-byte payload, `Copy64Byte`, and UBS data path unchanged.
The existing handshake wire format is not changed by this PR:
- The Base Hello remains 64 bytes.
- The V3 format extension remains a fixed 4-byte message.
- No additional handshake bytes are introduced.
Tests:
```text
Full CMake build passed.
```
```text
30 tests from 10 test suites passed in brpc_ubring_unittest.
```
Smoke-test results:
- An `IPC_V2` client and server successfully negotiated UBRing.
- 353678 RPC requests completed with 0 failures.
- Both peers confirmed successful UBRing establishment.
Side effects:
- Performance effects:
`IPC_V2` reduces the number of slots required for larger messages and removes the legacy `local_msg_space -> Copy64Byte` staging copy from the IPC path.
The 4096-byte slot size was selected from the evaluated `IPC_V2` microbenchmark candidates as a balance between throughput and the number of available slots.
Small messages still occupy one 4096-byte `IPC_V2` slot, so the number of simultaneously queued small messages is lower than with the legacy 64-byte layout.
UBS has no data-path performance change because it continues to use `LEGACY_64`.
- Breaking backward compatibility:
No wire-format compatibility break is introduced.
`IPC_V2` is enabled only when both peers select it. An `IPC_V2` peer connected to a peer that only supports `LEGACY_64` falls back to TCP.
IPC and UBS peers also fall back to TCP when their selected data formats differ.
The existing Base Hello and V3 format-extension layouts remain unchanged, so no additional bytes can remain unread in the TCP stream during fallback.
Timer and teardown lifecycle changes are intentionally out of scope and remain covered by the separate Phase 2 work in #3509.
---
### Check List:
- Please make sure your changes are compilable.
- When providing us with a new feature, it is best to add related tests.
- Please follow [Contributor Covenant Code of Conduct](https://github.com/apache/brpc/blob/master/CODE_OF_CONDUCT.md).
--
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: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Add IPC_V2 data path for UBRing IPC (brpc)
Posted by "chenBright (via GitHub)" <gi...@apache.org>.
chenBright merged PR #3573:
URL: https://github.com/apache/brpc/pull/3573
--
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: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Add IPC_V2 data path for UBRing IPC (brpc)
Posted by "wwbmmm (via GitHub)" <gi...@apache.org>.
wwbmmm commented on PR #3573:
URL: https://github.com/apache/brpc/pull/3573#issuecomment-6060084869
LGTM
--
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: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Add IPC_V2 data path for UBRing IPC (brpc)
Posted by "zchuango (via GitHub)" <gi...@apache.org>.
zchuango commented on PR #3573:
URL: https://github.com/apache/brpc/pull/3573#issuecomment-6142494923
@chenBright Do you have any further comments on this PR? If not, I'll go ahead and merge it.
--
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: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Add IPC_V2 data path for UBRing IPC (brpc)
Posted by "zchuango (via GitHub)" <gi...@apache.org>.
zchuango commented on code in PR #3573:
URL: https://github.com/apache/brpc/pull/3573#discussion_r4143778571
##########
src/brpc/ubshm/ub_ring.cpp:
##########
@@ -896,6 +1209,22 @@ RETURN_CODE UBRing::UbrMapRemoteShmAddTimer(SHM *local_trx_shm, const char *loca
LOG(ERROR) << "Connect Trx map shared memory failed, remote shm=" << remote_trx_shm.name;
return rc;
}
+ if (format == UBR_DATA_FORMAT_LEGACY_64) {
+ InitializeClientRemoteLegacyFormat();
+ rc = InitializeClientLocalLegacyFormat();
+ if (LIKELY(rc == UBRING_OK)) {
+ InitializeClientLegacyFormat();
+ }
+ } else if (format == UBR_DATA_FORMAT_IPC_V2) {
+ rc = UbrPrepareIpcV2Format() == 0 ? UBRING_OK : UBRING_ERR;
+ } else {
+ rc = UBRING_ERR;
+ }
+ if (UNLIKELY(rc != UBRING_OK)) {
+ LOG(ERROR) << "Initialize client data format failed, local_name="
+ << local_name << ", format=" << format;
+ return rc;
Review Comment:
Agreed that this failure path needs explicit cleanup. The specific “fewer than two slots” example may not be reachable through the normal IPC allocation path because shared memory is required to be at least 4 MiB and allocation-unit aligned, but the error path should still release all provisional resources. I’ll address it after updating this PR with #3509 and add failure-path coverage.
##########
src/brpc/ubshm/ub_endpoint.cpp:
##########
@@ -636,10 +667,13 @@ void* UBShmEndpoint::ProcessHandshakeAtServer(void* arg) {
remote_extension.Deserialize(data);
HelloFormatExtension local_extension = {
HelloFormatExtension::WIRE_SIZE, UBR_DATA_FORMAT_NONE};
- if (remote_extension.extension_len == HelloFormatExtension::WIRE_SIZE &&
- remote_extension.format_id == UBR_DATA_FORMAT_LEGACY_64) {
- local_extension.format_id = UBR_DATA_FORMAT_LEGACY_64;
- selected_format = UBR_DATA_FORMAT_LEGACY_64;
+ if (remote_extension.extension_len == HelloFormatExtension::WIRE_SIZE) {
+ selected_format =
+ SelectDataFormat(local_format, remote_extension.format_id);
+ }
+ if (selected_format != UBR_DATA_FORMAT_NONE) {
+ local_extension.format_id =
+ static_cast<uint16_t>(selected_format);
} else {
ub_transport->_ub_state = UBShmTransport::UB_OFF;
Review Comment:
Agreed. The format-mismatch fallback needs explicit rollback of the provisional server resources. This overlaps with #3509, which changes timer cancellation, cleanup ownership, and transaction slot reuse. I plan to update this PR after #3509 lands, then implement the rollback using the new lifecycle APIs and add coverage for this fallback path.
--
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: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Add IPC_V2 data path for UBRing IPC (brpc)
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3573:
URL: https://github.com/apache/brpc/pull/3573#discussion_r4142435720
##########
src/brpc/ubshm/ub_endpoint.cpp:
##########
@@ -636,10 +667,13 @@ void* UBShmEndpoint::ProcessHandshakeAtServer(void* arg) {
remote_extension.Deserialize(data);
HelloFormatExtension local_extension = {
HelloFormatExtension::WIRE_SIZE, UBR_DATA_FORMAT_NONE};
- if (remote_extension.extension_len == HelloFormatExtension::WIRE_SIZE &&
- remote_extension.format_id == UBR_DATA_FORMAT_LEGACY_64) {
- local_extension.format_id = UBR_DATA_FORMAT_LEGACY_64;
- selected_format = UBR_DATA_FORMAT_LEGACY_64;
+ if (remote_extension.extension_len == HelloFormatExtension::WIRE_SIZE) {
+ selected_format =
+ SelectDataFormat(local_format, remote_extension.format_id);
+ }
+ if (selected_format != UBR_DATA_FORMAT_NONE) {
+ local_extension.format_id =
+ static_cast<uint16_t>(selected_format);
} else {
ub_transport->_ub_state = UBShmTransport::UB_OFF;
Review Comment:
When a V3 peer proposes `LEGACY_64` to an IPC server, `AllocateServerResources()` has already mapped both queues, registered the poller, started timers, and marked the `IPC_V2` transaction connected. Setting `UB_OFF` here leaves those resources active for a TCP-only connection. The heartbeat can eventually release the pooled transaction while this endpoint's ring and poller still hold its pointer; a reused slot could then be accessed as another connection. After completing the fallback handshake, abort the provisional transaction: unregister the poller, cancel its timers, release both mappings and the manager slot, and detach the ring without waiting for peer UBRing participation.
##########
src/brpc/ubshm/ub_ring.cpp:
##########
@@ -896,6 +1209,22 @@ RETURN_CODE UBRing::UbrMapRemoteShmAddTimer(SHM *local_trx_shm, const char *loca
LOG(ERROR) << "Connect Trx map shared memory failed, remote shm=" << remote_trx_shm.name;
return rc;
}
+ if (format == UBR_DATA_FORMAT_LEGACY_64) {
+ InitializeClientRemoteLegacyFormat();
+ rc = InitializeClientLocalLegacyFormat();
+ if (LIKELY(rc == UBRING_OK)) {
+ InitializeClientLegacyFormat();
+ }
+ } else if (format == UBR_DATA_FORMAT_IPC_V2) {
+ rc = UbrPrepareIpcV2Format() == 0 ? UBRING_OK : UBRING_ERR;
+ } else {
+ rc = UBRING_ERR;
+ }
+ if (UNLIKELY(rc != UBRING_OK)) {
+ LOG(ERROR) << "Initialize client data format failed, local_name="
+ << local_name << ", format=" << format;
+ return rc;
Review Comment:
If the mapped peer queue fails `UbrPrepareIpcV2Format()` (for example, it has fewer than two aligned slots), this return leaves the client's local and remote shared-memory mappings and manager transaction allocated. No timer has started, so a TCP fallback on a long-lived socket keeps those resources indefinitely rather than reaching heartbeat cleanup. On this initialization failure, abort setup and release both mappings and the transaction (and remove the provisional poller registration) before sending ACK=0; do not use the normal close path, which waits for a connected peer.
--
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: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org