[HBASE-27924] Remove duplicate code for NettyHBaseSaslRpcServerHandler and make the sentByte metrics more accurate

• Type: Bug
• Status: Resolved
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 2.6.0, 3.0.0-alpha-4, 4.0.0-alpha-1
• Fix Version/s: 2.6.0, 3.0.0-beta-1
• Component/s: netty, rpc, security
• Assignee: chenglei
• Reporter: chenglei
• Created: 12/Jun/23 08:17
• Updated: 17/Jun/23 19:21
• Resolved: 17/Jun/23 01:50

Description

NettyHBaseSaslRpcServerHandler.doResponse and ServerRpcConnection.doRawSaslReply are very similar, I think we could replace NettyHBaseSaslRpcServerHandler.doResponse with ServerRpcConnection.doRawSaslReply：

private void doResponse(ChannelHandlerContext ctx, SaslStatus status, Writable rv,
    String errorClass, String error) throws IOException {
    // In my testing, have noticed that sasl messages are usually
    // in the ballpark of 100-200. That's why the initial capacity is 256.
    ByteBuf resp = ctx.alloc().buffer(256);
    try (ByteBufOutputStream out = new ByteBufOutputStream(resp)) {
      out.writeInt(status.state); // write status
      if (status == SaslStatus.SUCCESS) {
        rv.write(out);
      } else {
        WritableUtils.writeString(out, errorClass);
        WritableUtils.writeString(out, error);
      }
    }
    NettyFutureUtils.safeWriteAndFlush(ctx, resp);
  }

protected final void doRawSaslReply(SaslStatus status, Writable rv, String errorClass,
    String error) throws IOException {
    BufferChain bc;
    // In my testing, have noticed that sasl messages are usually
    // in the ballpark of 100-200. That's why the initial capacity is 256.
    try (ByteBufferOutputStream saslResponse = new ByteBufferOutputStream(256);
      DataOutputStream out = new DataOutputStream(saslResponse)) {
      out.writeInt(status.state); // write status
      if (status == SaslStatus.SUCCESS) {
        rv.write(out);
      } else {
        WritableUtils.writeString(out, errorClass);
        WritableUtils.writeString(out, error);
      }
      bc = new BufferChain(saslResponse.getByteBuffer());
    }
    doRespond(() -> bc);
  }

At the same time, NettyHBaseSaslRpcServerHandler.doResponse sends ByteBuf directly , not the unified RpcResponse , so it would not handled by the logic in NettyRpcServerResponseEncoder.write, which would update the MetricsHBaseServer.sentBytes. Using ServerRpcConnection.doRawSaslReply uniformly would make the MetricsHBaseServer.sentBytes more accurate.

Issue Links

• links to: GitHub Pull Request #5285