From 99e4d2a8eed124bd2505bcfc395d5e7ba826dd1e Mon Sep 17 00:00:00 2001 From: Thanh Ha Date: Thu, 3 Mar 2016 20:55:06 -0500 Subject: [PATCH] Revert "Bug 5118 - Unsent messages reported as failed after disconnect" This reverts commit 42d77fc7c0b4fc944b270a29f15ae600fc494cb2. Change-Id: Iac6ecd2af999ccae49afc8f9d88a68431f0045b5 Signed-off-by: Thanh Ha --- .../protocol/impl/core/PipelineHandlers.java | 4 +- .../AbstractOutboundQueueManager.java | 8 ---- .../AbstractStackedOutboundQueue.java | 38 +++++-------------- .../connection/ConnectionAdapterImpl.java | 6 +-- 4 files changed, 12 insertions(+), 44 deletions(-) diff --git a/openflow-protocol-impl/src/main/java/org/opendaylight/openflowjava/protocol/impl/core/PipelineHandlers.java b/openflow-protocol-impl/src/main/java/org/opendaylight/openflowjava/protocol/impl/core/PipelineHandlers.java index 88144d47..51ac5482 100644 --- a/openflow-protocol-impl/src/main/java/org/opendaylight/openflowjava/protocol/impl/core/PipelineHandlers.java +++ b/openflow-protocol-impl/src/main/java/org/opendaylight/openflowjava/protocol/impl/core/PipelineHandlers.java @@ -44,9 +44,9 @@ public enum PipelineHandlers { */ DELEGATING_INBOUND_HANDLER, /** - * Performs configurable efficient flushing + * Performs efficient flushing */ - CHANNEL_OUTBOUND_QUEUE_MANAGER, + CHANNEL_OUTBOUNF_QUEUE, /** * Decodes incoming messages into message frames * and filters them based on version supported diff --git a/openflow-protocol-impl/src/main/java/org/opendaylight/openflowjava/protocol/impl/core/connection/AbstractOutboundQueueManager.java b/openflow-protocol-impl/src/main/java/org/opendaylight/openflowjava/protocol/impl/core/connection/AbstractOutboundQueueManager.java index fdcc1f3e..8febb158 100644 --- a/openflow-protocol-impl/src/main/java/org/opendaylight/openflowjava/protocol/impl/core/connection/AbstractOutboundQueueManager.java +++ b/openflow-protocol-impl/src/main/java/org/opendaylight/openflowjava/protocol/impl/core/connection/AbstractOutboundQueueManager.java @@ -163,22 +163,14 @@ abstract class AbstractOutboundQueueManager OF Plugin -> queue.close() - // -> queueHandler.onConnectionQueueChanged(null). The last call causes that no more entries are enqueued - // in the queue. super.channelInactive(ctx); LOG.debug("Channel {} initiating shutdown...", ctx.channel()); - // Then we start queue shutdown, start counting written messages (so that we don't keep sending messages - // indefinitely) and failing not completed entries. shuttingDown = true; final long entries = currentQueue.startShutdown(ctx.channel()); LOG.debug("Cleared {} queue entries from channel {}", entries, ctx.channel()); - // Finally, we schedule flush task that will take care of unflushed entries. We also cover the case, - // when there is more than shutdownOffset messages enqueued in unflushed segments - // (AbstractStackedOutboundQueue#finishShutdown()). scheduleFlush(); } diff --git a/openflow-protocol-impl/src/main/java/org/opendaylight/openflowjava/protocol/impl/core/connection/AbstractStackedOutboundQueue.java b/openflow-protocol-impl/src/main/java/org/opendaylight/openflowjava/protocol/impl/core/connection/AbstractStackedOutboundQueue.java index 75963335..a32c1ad1 100644 --- a/openflow-protocol-impl/src/main/java/org/opendaylight/openflowjava/protocol/impl/core/connection/AbstractStackedOutboundQueue.java +++ b/openflow-protocol-impl/src/main/java/org/opendaylight/openflowjava/protocol/impl/core/connection/AbstractStackedOutboundQueue.java @@ -260,31 +260,16 @@ abstract class AbstractStackedOutboundQueue implements OutboundQueue { final long xid = LAST_XID_OFFSET_UPDATER.addAndGet(this, StackedSegment.SEGMENT_SIZE); shutdownOffset = (int) (xid - firstSegment.getBaseXid() - StackedSegment.SEGMENT_SIZE); - // Fails all uncompleted entries, because they will never be completed due to disconnected channel. - return lockedFailSegments(uncompletedSegments.iterator()); + return lockedShutdownFlush(); } } - /** - * Checks if the shutdown is in final phase -> all allowed entries (number of entries < shutdownOffset) are flushed - * and fails all not completed entries (if in final phase) - * @return true if in final phase, false if a flush is needed - */ boolean finishShutdown() { - boolean needsFlush; synchronized (unflushedSegments) { - // Fails all entries, that were flushed in shutdownOffset (became uncompleted) - // - they will never be completed due to disconnected channel. - lockedFailSegments(uncompletedSegments.iterator()); - // If no further flush is needed, than we fail all unflushed segments, so that each enqueued entry - // is reported as unsuccessful due to channel disconnection. No further entries should be enqueued - // by this time. - needsFlush = needsFlush(); - if (!needsFlush) { - lockedFailSegments(unflushedSegments.iterator()); - } + lockedShutdownFlush(); } - return !needsFlush; + + return !needsFlush(); } protected OutboundQueueEntry getEntry(final Long xid) { @@ -317,27 +302,22 @@ abstract class AbstractStackedOutboundQueue implements OutboundQueue { return fastSegment.getEntry(fastOffset); } - /** - * Fails not completed entries in segments and frees completed segments - * @param iterator list of segments to be failed - * @return number of failed entries - */ @GuardedBy("unflushedSegments") - private long lockedFailSegments(Iterator iterator) { + private long lockedShutdownFlush() { long entries = 0; // Fail all queues - while (iterator.hasNext()) { - final StackedSegment segment = iterator.next(); + final Iterator it = uncompletedSegments.iterator(); + while (it.hasNext()) { + final StackedSegment segment = it.next(); entries += segment.failAll(OutboundQueueException.DEVICE_DISCONNECTED); if (segment.isComplete()) { LOG.trace("Cleared segment {}", segment); - iterator.remove(); + it.remove(); } } return entries; } - } diff --git a/openflow-protocol-impl/src/main/java/org/opendaylight/openflowjava/protocol/impl/core/connection/ConnectionAdapterImpl.java b/openflow-protocol-impl/src/main/java/org/opendaylight/openflowjava/protocol/impl/core/connection/ConnectionAdapterImpl.java index 7c10be16..37635c03 100644 --- a/openflow-protocol-impl/src/main/java/org/opendaylight/openflowjava/protocol/impl/core/connection/ConnectionAdapterImpl.java +++ b/openflow-protocol-impl/src/main/java/org/opendaylight/openflowjava/protocol/impl/core/connection/ConnectionAdapterImpl.java @@ -203,11 +203,7 @@ public class ConnectionAdapterImpl extends AbstractConnectionAdapterStatistics i outputManager = ret; /* we don't need it anymore */ channel.pipeline().remove(output); - // OutboundQueueManager is put before DelegatingInboundHandler because otherwise channelInactive event would - // be first processed in OutboundQueueManager and then in ConnectionAdapter (and Openflowplugin). This might - // cause problems because we are shutting down the queue before Openflowplugin knows about it. - channel.pipeline().addBefore(PipelineHandlers.DELEGATING_INBOUND_HANDLER.name(), - PipelineHandlers.CHANNEL_OUTBOUND_QUEUE_MANAGER.name(), outputManager); + channel.pipeline().addLast(outputManager); return new OutboundQueueHandlerRegistrationImpl(handler) { @Override -- 2.36.6