activemq-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From clebertsuconic <...@git.apache.org>
Subject [GitHub] activemq-artemis pull request #1119: ARTEMIS-1025 OutOfDirectMemoryError rai...
Date Fri, 31 Mar 2017 20:56:46 GMT
Github user clebertsuconic commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1119#discussion_r109248555
  
    --- Diff: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyConnection.java
---
    @@ -92,69 +86,143 @@ public NettyConnection(final Map<String, Object> configuration,
     
           this.listener = listener;
     
    +      this.directDeliver = directDeliver;
    +
           this.batchingEnabled = batchingEnabled;
     
    -      this.directDeliver = directDeliver;
    +      this.writeBufferHighWaterMark = this.channel.config().getWriteBufferHighWaterMark();
    +
    +      this.batchLimit = batchingEnabled ? Math.min(this.writeBufferHighWaterMark, DEFAULT_BATCH_BYTES)
: 0;
    +   }
    +
    +   private static void waitFor(ChannelPromise promise, long millis) {
    +      try {
    +         final boolean completed = promise.await(millis);
    +         if (!completed) {
    +            ActiveMQClientLogger.LOGGER.timeoutFlushingPacket();
    +         }
    +      } catch (InterruptedException e) {
    +         throw new ActiveMQInterruptedException(e);
    +      }
    +   }
    +
    +   /**
    +    * Returns an estimation of the current size of the write buffer in the channel.
    +    * To obtain a more precise value is necessary to use the unsafe API of the channel
to
    +    * call the {@link io.netty.channel.ChannelOutboundBuffer#totalPendingWriteBytes()}.
    +    * Anyway, both these values are subject to concurrent modifications.
    +    */
    +   private static int batchBufferSize(Channel channel, int writeBufferHighWaterMark)
{
    +      //Channel::bytesBeforeUnwritable is performing a volatile load
    +      //this is the reason why writeBufferHighWaterMark is passed as an argument
    +      final int bytesBeforeUnwritable = (int) channel.bytesBeforeUnwritable();
    +      assert bytesBeforeUnwritable >= 0;
    +      final int writtenBytes = writeBufferHighWaterMark - bytesBeforeUnwritable;
    +      assert writtenBytes >= 0;
    +      return writtenBytes;
    +   }
    +
    +   /**
    +    * When batching is not enabled, it tries to back-pressure the caller thread.
    +    * The back-pressure provided is not before the writeAndFlush request, buf after it:
too many threads that are not
    +    * using {@link Channel#isWritable} to know when push unbatched data will risk to
cause OOM due to the enqueue of each own {@link Channel#writeAndFlush} requests.
    +    * Trying to provide back-pressure before the {@link Channel#writeAndFlush} request
could work, but in certain scenarios it will block {@link Channel#isWritable} to be true.
    +    */
    +   private static ChannelFuture backPressuredWriteAndFlush(final ByteBuf bytes,
    +                                                           final int readableBytes,
    +                                                           final Channel channel,
    +                                                           final ChannelPromise promise)
{
    +      final ChannelFuture future;
    +      if (!channel.isWritable()) {
    +         final ChannelPromise channelPromise = promise.isVoid() ? channel.newPromise()
: promise;
    +         future = channel.writeAndFlush(bytes, channelPromise);
    +         //is the channel is not writable wait the current request to be flushed, providing
backpressuring on the caller thread
    +         if (!channel.isWritable() && !future.awaitUninterruptibly(DEFAULT_BACK_PRESSURE_WAIT_MILLIS))
{
    +            if (ActiveMQClientLogger.LOGGER.isTraceEnabled()) {
    --- End diff --
    
    I previously complained here that the log.was trace, and you were checking of isTrace...
    
    This is probably a warn...
    
    
    if it's not being able to flush in time, the writable thread is probably busy with something
else.
    
    
    perhaps we should even do something more drastic eventually.. such as close the connection?
    
    
    for now a warn will do... (I may merge this and do it myself)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message