+1 (binding) Best, Hang
Zike Yang <zky...@streamnative.io.invalid> 于2022年1月12日周三 09:59写道： > > +1 (non-binding) > > On Wed, Jan 12, 2022 at 9:58 AM Haiting Jiang <jianghait...@apache.org> wrote: > > > > This is the voting thread for PIP-132. It will stay open for at least 48 > > hours. > > > > https://github.com/apache/pulsar/issues/13591 > > > > Pasted below for quoting convenience. > > > > ---- > > > > ## Motivation > > > > Currently, Pulsar client (Java) only checks payload size for max message > > size validation. > > > > Client throws TimeoutException if we produce a message with too many > > properties, see [1]. > > But the root cause is that is trigged TooLongFrameException in broker > > server. > > > > In this PIP, I propose to include message header size when check > > maxMessageSize of non-batch > > messages, this brings the following benefits. > > 1. Clients can throw InvalidMessageException immediately if properties > > takes too much storage space. > > 2. This will make the behaviour consistent with topic level max message > > size check in broker. > > 3. Strictly limit the entry size less than maxMessageSize, avoid sending > > message to bookkeeper failed. > > > > ## Goal > > > > Include message header size when check maxMessageSize for non-batch message > > on the client side. > > > > ## Implementation > > > > ``` > > // Add a size check in > > org.apache.pulsar.client.impl.ProducerImpl#processOpSendMsg > > if (op.msg != null // for non-batch messages only > > && op.getMessageHeaderAndPayloadSize() > ClientCnx.getMaxMessageSize()) { > > // finish send op with InvalidMessageException > > releaseSemaphoreForSendOp(op); > > op.sendComplete(new PulsarClientException(new InvalidMessageException, > > op.sequenceId)); > > } > > > > > > // > > org.apache.pulsar.client.impl.ProducerImpl.OpSendMsg#getMessageHeaderAndPayloadSize > > > > public int getMessageHeaderAndPayloadSize() { > > ByteBuf cmdHeader = cmd.getFirst(); > > cmdHeader.markReaderIndex(); > > int totalSize = cmdHeader.readInt(); > > int cmdSize = cmdHeader.readInt(); > > int msgHeadersAndPayloadSize = totalSize - cmdSize - 4; > > cmdHeader.resetReaderIndex(); > > return msgHeadersAndPayloadSize; > > } > > ``` > > > > ## Reject Alternatives > > Add a new property like "maxPropertiesSize" or "maxHeaderSize" in > > broker.conf and pass it to > > client like maxMessageSize. But the implementation is much more complex, > > and don't have the > > benefit 2 and 3 mentioned in Motivation. > > > > ## Compatibility Issue > > As a matter of fact, this PIP narrows down the sendable range. Previously, > > when maxMessageSize > > is 1KB, it's ok to send message with 1KB properties and 1KB payload. But > > with this PIP, the > > sending will fail with InvalidMessageException. > > > > One conservative way is to add a boolean config "includeHeaderInSizeCheck" > > to enable this > > feature. But I think it's OK to enable this directly as it's more > > reasonable, and I don't see good > > migration plan if we add a config for this. > > > > [1] https://github.com/apache/pulsar/issues/13560 > > > > Thanks, > > Haiting Jiang > > > > -- > Zike Yang
