You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/01/22 19:35:55 UTC

[GitHub] [pulsar-client-go] flowchartsman opened a new issue #447: pulsar-client-go reports message too large before compressing it.

flowchartsman opened a new issue #447:
URL: https://github.com/apache/pulsar-client-go/issues/447


   I'm getting "message too large" messages on some large JSON documents that, when compressed, are nowhere near the limit.
   
   in `producer_partiton.go`:
   ```go
   	// if msg is too large
   	if len(payload) > int(p.cnx.GetMaxMessageSize()) {
   		p.publishSemaphore.Release()
   		request.callback(nil, request.msg, errMessageTooLarge)
   		p.log.WithError(errMessageTooLarge).
   			WithField("size", len(payload)).
   			WithField("properties", msg.Properties).
   			Error()
   		p.metrics.PublishErrorsMsgTooLarge.Inc()
   		return
   	}
   ```
   
   Correct me if I'm wrong, but this appears to check the payload size before any compression.
   
   Whereas in `ProducerImpl.java`:
   ```java
   int compressedSize = compressedPayload.readableBytes();
               if (compressedSize > ClientCnx.getMaxMessageSize() && !this.conf.isChunkingEnabled()) {
                   compressedPayload.release();
   ```


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-go] flowchartsman edited a comment on issue #447: pulsar-client-go appears to report message too large before compressing it.

Posted by GitBox <gi...@apache.org>.
flowchartsman edited a comment on issue #447:
URL: https://github.com/apache/pulsar-client-go/issues/447#issuecomment-953218258


   @cocktail828 I see you have checked in a fix for this issue in your fork; is there any way you'd be able to issue a PR for this?


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-go] flowchartsman edited a comment on issue #447: pulsar-client-go appears to report message too large before compressing it.

Posted by GitBox <gi...@apache.org>.
flowchartsman edited a comment on issue #447:
URL: https://github.com/apache/pulsar-client-go/issues/447#issuecomment-765654177


   Okay, so wait, I’m confused. I guess it might help to discuss the intended purpose of maxmessagesize.  If it’s just a knob to adjust cluster performance and storage allocation and such, that’s one thing, but if it is intended as a guarantee that, for example, a consumer can keep a fixed buffer for handing the payload, then this would seem to be correct behavior, am I right? Are the Java and C++ clients actually *incorrect* in allowing message compression before payload size checking?


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-go] merlimat commented on issue #447: pulsar-client-go appears to report message too large before compressing it.

Posted by GitBox <gi...@apache.org>.
merlimat commented on issue #447:
URL: https://github.com/apache/pulsar-client-go/issues/447#issuecomment-765652105


   Yes, if the max limit is 5MB, it will only guaranteed to accept up to 5MB. Above that, it's just "best effort" to get that to fit in with compression.


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-go] flowchartsman commented on issue #447: pulsar-client-go appears to report message too large before compressing it.

Posted by GitBox <gi...@apache.org>.
flowchartsman commented on issue #447:
URL: https://github.com/apache/pulsar-client-go/issues/447#issuecomment-765646571


   > Keep in mind, though, that it won't change the "guaranteed" max size, since there is guarantee that the compression will actually reduce the size.
   
   I am not sure what this means. Of course there is never a guarantee with compression...


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-go] merlimat commented on issue #447: pulsar-client-go appears to report message too large before compressing it.

Posted by GitBox <gi...@apache.org>.
merlimat commented on issue #447:
URL: https://github.com/apache/pulsar-client-go/issues/447#issuecomment-765642969


   That's right, the Java and C++ clients are more permissive in that the message go through if it's below the limit after compression. We should also do the same thing in Go. 
   
   Keep in mind, though, that it won't change the "guaranteed" max size, since there is guarantee that the compression will actually reduce the size.


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-go] flowchartsman commented on issue #447: pulsar-client-go appears to report message too large before compressing it.

Posted by GitBox <gi...@apache.org>.
flowchartsman commented on issue #447:
URL: https://github.com/apache/pulsar-client-go/issues/447#issuecomment-765654177


   Okay, so wait, I’m confused. I guess it might help to discuss the intended purpose of maxmessagesize.  If it’s just a knob to adjust cluster performance and storage allocation and such, that’s one thing, but if it is intended as a to guarantee that, for example, a consumer can keep a fixed buffer for handing the payload, then this would seem to be correct behavior, am I right? Are the Java and C++ clients actually *incorrect* in allowing message compression before payload size checking?


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-go] freeznet commented on issue #447: pulsar-client-go appears to report message too large before compressing it.

Posted by GitBox <gi...@apache.org>.
freeznet commented on issue #447:
URL: https://github.com/apache/pulsar-client-go/issues/447#issuecomment-769087177


   @flowchartsman as my understanding, Java and C++ are doing the right way. `MaxMessageSize` can be considered as the buffer size between broker and client. But it also the buffer size between bookkeeper and pulsar. Pulsar and bookkeeper will not de-compress the message, and the decompression is a client side job. This is why we need to ensure that client side features are consistent across different languages. 


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-go] cocktail828 commented on issue #447: pulsar-client-go appears to report message too large before compressing it.

Posted by GitBox <gi...@apache.org>.
cocktail828 commented on issue #447:
URL: https://github.com/apache/pulsar-client-go/issues/447#issuecomment-925543545


   any updates on this issue? We meet the same problem recently.


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-go] flowchartsman commented on issue #447: pulsar-client-go appears to report message too large before compressing it.

Posted by GitBox <gi...@apache.org>.
flowchartsman commented on issue #447:
URL: https://github.com/apache/pulsar-client-go/issues/447#issuecomment-953218258


   @cocktail828 I see you have checked in a fix for this issue; is there any way you'd be able to issue a PR for this?


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-go] cocktail828 commented on issue #447: pulsar-client-go appears to report message too large before compressing it.

Posted by GitBox <gi...@apache.org>.
cocktail828 commented on issue #447:
URL: https://github.com/apache/pulsar-client-go/issues/447#issuecomment-925655761


   hi everybody:
   I just fix this issue this afternoon. Now in my fork, I move **compressionProvider** into _producer_partition.go_ from _batch_builder.go_, so I can compress payload before check the message size. 
   [cocktail828/pulsar-client-go fixes apache#447](https://github.com/cocktail828/pulsar-client-go/commit/c0eeb48e750d6f4d82fcbfdc3daa3c1f96747111).
   
   


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-go] cocktail828 removed a comment on issue #447: pulsar-client-go appears to report message too large before compressing it.

Posted by GitBox <gi...@apache.org>.
cocktail828 removed a comment on issue #447:
URL: https://github.com/apache/pulsar-client-go/issues/447#issuecomment-925655761


   hi everybody:
   I just fix this issue this afternoon. Now in my fork, I move **compressionProvider** into _producer_partition.go_ from _batch_builder.go_, so I can compress payload before check the message size. 
   [cocktail828/pulsar-client-go fixes apache#447](https://github.com/cocktail828/pulsar-client-go/commit/c0eeb48e750d6f4d82fcbfdc3daa3c1f96747111).
   
   


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org