Closed Florian-Schoenherr closed 4 years ago
This PR wasn't about the Exception you're mentioning, but the fact that when you configure a maxMessageSize
of e.g. 4096 you still get files served which are larger than that, without any notice, via
BasicConsumerByteBuffer.allocate(0, size);
And your push is missing on master, maybe only commited?
@Florian-Schoenherr You are trying to catch an exception but there is none to catch, other than OOME or some other unrecoverable exception. I don't know why you think there should be some sort of notification ("without any notice") when this happens . This does exactly what it was designed to do; that is, if the pooled buffers are too small then allocate a non-pooled buffer that is large enough. It's GC'd after its use.
Thank you for clarifying. I didn't get that it should use a non-pooled buffer. Also, I did misunderstand some of our conversation on Slack, but now I get it. Should I say something about that in the docs? Because I misunderstood to a large extend because it seems so definitive, it's called maxMessageSize. That's also why I wanted to log. Now I know this should not be an Exception (and yes, there is one to catch, which I'm throwing myself, because I thought that size>maxSize is illegal state). If you don't want any logging, we can close. Else I would make one more commit, where you just have:
if (size < maxMessageSize) {
return responseBufferPool.acquire("ServerActor#BasicCompletedBasedResponseCompletes#bufferFor");
}
logger().debug("Response exceeds maxMessageSize " + maxMessageSize + ", allocating size " + size + " instead.")
return BasicConsumerByteBuffer.allocate(0, size + 1024);
@Florian-Schoenherr I think time zones played in. We don't want to log this permanently because this is a very hot section of code. Even with debug()
level it can be costly because the message must still be sent and it generates garbage to be collected.
I have added a richer explanation in the docs. Thanks for the suggestion. Please see if this clarifies things.
This logs as an error if maxMessageSize is exceeded.
aquire
is different, can we make this DRY easily somehow?response
is available. Therequest
is obviously used in some methods, but I'm not quite sure how I get it over to this method. I would like to log which resource is too big (response only holds Body, sadly no filename).