qianye1001 commented on issue #10434:
URL: https://github.com/apache/rocketmq/issues/10434#issuecomment-4645195327

   **Issue Evaluation**
   
   Category: `type/enhancement` | Status: **Evaluated**
   
   **Feasibility:** feasible (low complexity, ~200-300 lines including tests)
   **Scope:** `proxy` module only — no changes to broker, namesrv, client, 
remoting, or proto
   **Compatibility:** no breaking changes — purely additive, new `@Override` 
methods where `UNIMPLEMENTED` was previously returned
   
   ### Assessment
   
   The issue description is accurate. Verified against current codebase:
   
   1. `GrpcMessagingApplication.java` has 13 `@Override` methods — 
`queryOffset`, `getOffset`, `updateOffset`, `pullMessage` are **not** among 
them.
   2. `MessagingProcessor` already has fully implemented backend methods: 
`queryConsumerOffset`, `getMaxOffset`, `updateConsumerOffset`, `pullMessage` — 
all wired through `DefaultMessagingProcessor` → `ConsumerProcessor` → 
`ClusterMessageService` → broker remoting.
   3. The proto stubs from `rocketmq-proto:2.1.2` already declare these RPCs as 
no-op stubs.
   
   This is pure gRPC wiring following the well-established `RouteActivity` / 
`AckMessageActivity` pattern (12 existing Activity classes). Zero new business 
logic needed.
   
   ### Proposed Change Summary
   
   | Layer | File | Change |
   |-------|------|--------|
   | Activity (new) | `proxy/.../grpc/v2/consumer/OffsetActivity.java` | New 
class, ~80 lines |
   | Interface | `GrpcMessagingActivity.java` | +2-3 method signatures |
   | Delegation | `DefaultGrpcMessagingActivity.java` | +2-3 delegation calls |
   | gRPC entry | `GrpcMessagingApplication.java` | +2-3 `@Override` methods |
   | Tests (new) | `proxy/.../grpc/v2/consumer/OffsetActivityTest.java` | Unit 
tests with mocked `MessagingProcessor` |
   
   ### Caveat
   
   The proto message types (`QueryOffsetRequest/Response`, 
`GetOffsetRequest/Response`) should be verified against the actual 
`rocketmq-proto:2.1.2` artifact to confirm field names exist. If they don't 
exist in v2.1.2, a coordinated `rocketmq-proto` release would be needed first.
   
   **Recommendation:** proceed
   
   ---
   *Automated evaluation by github-manager-bot*


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

Reply via email to