You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by "quantranhong1999 (via GitHub)" <gi...@apache.org> on 2024/03/21 09:26:25 UTC
[PR] [Netty] Size estimator for ChunkedStream [james-project]
quantranhong1999 opened a new pull request, #2145:
URL: https://github.com/apache/james-project/pull/2145
Rationale: `DefaultMessageSizeEstimator` does not support estimating size for `ChunkedStream` (the wrapper type JAMES IMAP is using to wrap ByteBuf over Netty channel), therefore Netty can not know if it should delay writing this message yet (e.g. IMAP server memory is full and the message size is big then Netty would delay the write).
Implementing a `MessageSizeEstimator` support `ChunkedStream` would help.
Before: the back-pressure is triggered at minimum (1-2 times over transferring 500 **big** messages)
cf: [without chunked stream size estimator.txt](https://github.com/apache/james-project/files/14692758/without.chunked.stream.size.estimator.txt)
After: the back-pressure is triggered more often (over 20 times over transferring 500 **big** messages)
cf: [with chunked stream size estimator.txt](https://github.com/apache/james-project/files/14692764/with.chunked.stream.size.estimator.txt)
cc @vttranlina @chibenwa
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] [Netty] Size estimator for ChunkedStream [james-project]
Posted by "quantranhong1999 (via GitHub)" <gi...@apache.org>.
quantranhong1999 commented on PR #2145:
URL: https://github.com/apache/james-project/pull/2145#issuecomment-2014640384
> With this we could potentially increase watermarks without risking things to blow up...
Likely yes.
I tested with a lower `WriteBufferWaterMark` => more back-pressure triggered, while a higher `WriteBufferWaterMark` => less back-pressure triggered.
Selecting a big enough `WriteBufferWaterMark` likely would avoid being throttled too soon.
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] [Netty] Size estimator for ChunkedStream [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa closed pull request #2145: [Netty] Size estimator for ChunkedStream
URL: https://github.com/apache/james-project/pull/2145
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] [Netty] Size estimator for ChunkedStream [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #2145:
URL: https://github.com/apache/james-project/pull/2145#discussion_r1535378432
##########
server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/ChunkedStreamWithSize.java:
##########
@@ -0,0 +1,150 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.imapserver.netty;
+
+import java.io.InputStream;
+import java.io.PushbackInputStream;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.ByteBufAllocator;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.stream.ChunkedInput;
+import io.netty.util.internal.ObjectUtil;
+
+/**
+ * A clone of io.netty.handler.stream.ChunkedStream class, with getChunkSize method added
+ */
+public class ChunkedStreamWithSize implements ChunkedInput<ByteBuf> {
+
+ static final int DEFAULT_CHUNK_SIZE = 8192;
+
+ private final PushbackInputStream in;
+ private final int chunkSize;
+ private long offset;
+ private boolean closed;
+
+ /**
+ * Creates a new instance that fetches data from the specified stream.
+ */
+ public ChunkedStreamWithSize(InputStream in) {
+ this(in, DEFAULT_CHUNK_SIZE);
+ }
+
+ /**
+ * Creates a new instance that fetches data from the specified stream.
+ *
+ * @param chunkSize the number of bytes to fetch on each
+ * {@link #readChunk(ChannelHandlerContext)} call
+ */
+ public ChunkedStreamWithSize(InputStream in, int chunkSize) {
+ ObjectUtil.checkNotNull(in, "in");
+ ObjectUtil.checkPositive(chunkSize, "chunkSize");
+
+ if (in instanceof PushbackInputStream) {
+ this.in = (PushbackInputStream) in;
+ } else {
+ this.in = new PushbackInputStream(in);
+ }
+ this.chunkSize = chunkSize;
+ }
+
+ /**
+ * Returns the number of transferred bytes.
+ */
+ public long transferredBytes() {
+ return offset;
+ }
+
+ @Override
+ public boolean isEndOfInput() throws Exception {
+ if (closed) {
+ return true;
+ }
+ if (in.available() > 0) {
+ return false;
+ }
+
+ int b = in.read();
+ if (b < 0) {
+ return true;
+ } else {
+ in.unread(b);
+ return false;
+ }
+ }
+
+ @Override
+ public void close() throws Exception {
+ closed = true;
+ in.close();
+ }
+
+ @Deprecated
+ @Override
+ public ByteBuf readChunk(ChannelHandlerContext ctx) throws Exception {
+ return readChunk(ctx.alloc());
+ }
+
+ @Override
+ public ByteBuf readChunk(ByteBufAllocator allocator) throws Exception {
+ if (isEndOfInput()) {
+ return null;
+ }
+
+ final int availableBytes = in.available();
+ final int chunkSize;
+ if (availableBytes <= 0) {
+ chunkSize = this.chunkSize;
+ } else {
+ chunkSize = Math.min(this.chunkSize, in.available());
+ }
+
+ boolean release = true;
+ ByteBuf buffer = allocator.buffer(chunkSize);
+ try {
+ // transfer to buffer
+ int written = buffer.writeBytes(in, chunkSize);
+ if (written < 0) {
+ return null;
+ }
+ offset += written;
+ release = false;
+ return buffer;
+ } finally {
+ if (release) {
+ buffer.release();
+ }
+ }
+ }
+
+ @Override
+ public long length() {
+ return -1;
+ }
+
+ @Override
+ public long progress() {
+ return offset;
+ }
+
+ public int getChunkSize() {
+ return chunkSize;
+ }
Review Comment:
Well James do actually chucks some kinda transformations of "ByteArrayInputStream".
So the chuck size is NOT representative of overall consummed memory by the write task.
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] [Netty] Size estimator for ChunkedStream [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on PR #2145:
URL: https://github.com/apache/james-project/pull/2145#issuecomment-2012230199
Cool.
With this we could potentially increase watermarks without risking things to blow up...
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] [Netty] Size estimator for ChunkedStream [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on PR #2145:
URL: https://github.com/apache/james-project/pull/2145#issuecomment-2015166917
See https://github.com/apache/james-project/pull/2149
Instead of inferring the size of the stream, which needs knowledge on the fact it is memory based is infered.
Preserving the knowledge that it is memory backed all along the chain, along with a tiny fix, enables accurate back pressure.
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org