----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17563/#review33312 -----------------------------------------------------------
Ship it! /trunk/qpid/cpp/src/qpid/broker/SessionAdapter.cpp <https://reviews.apache.org/r/17563/#comment62757> Is this always an 'error'? What about with federation when a failover exchange doesn't exist? Not a big deal, but if we can avoid logging those sorts of expected things at error its better. However if this is the cleanest way to log some important error that can't be logged elsewhere, then feel free to ignore this. - Gordon Sim On Jan. 30, 2014, 11:14 p.m., Alan Conway wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/17563/ > ----------------------------------------------------------- > > (Updated Jan. 30, 2014, 11:14 p.m.) > > > Review request for qpid and Gordon Sim. > > > Bugs: QPID-5528 > https://issues.apache.org/jira/browse/QPID-5528 > > > Repository: qpid > > > Description > ------- > > QPID-5528: HA Clean up error messages around rolled-back transactions. > > A simple transaction test on a 3 node cluster generates a lot of errors and > rollback notices in the > broker logs even though the test code never rolls back a transaction. E.g. > > qpid-cluster-benchmark -b 20.0.20.200 -n1 -m 1000 -q3 -s2 -r2 > --send-arg=--tx --send-arg=10 --receive-arg=--tx --receive-arg=10 > > The errors are caused by queues being deleted while backup brokers are using > them. This happens a lot in the transaction test because a transactional > session > must create a new transaction when the previous one closes. When the session > closes the open transaction is rolled back automatically. Thus there is almost > always an empty transaction that is created then immediately rolled back at > the > end of the session. Backup brokers may still be in the process of subscribing > to > the transaction's replication queue at this point, causing (harmlesss) errors. > > We'd like to clean this up completely but unfortunately there are some error > messages that are not easy to remove. This commit does the following: > > - Remove misleading "backup disconnected" message for cancelled transactions. > - Include TxReplicator destroy in QueueReplicator mutex. Idempotence check > before any destroy. > - Remove spurious warning about ignored unreplicated dequeues. > - Remove log messages about rolling back/destroying empty transactions. > - Log incoming execution exceptions as errors to explain subsequent session > detached errors. > > There should be no more unexpected log messages about rollbacks since we no > longer log rollback of an empty transaction. > > The following errors remain, they are harmless and can be ignored. They are > not > necessarily related to transactions and can occur in other situations where > queues are deleted while still replicating. > > Queue deleted, replication in progress (happens in the test, not related to > TX) > > On primary, the following errors: > - [Protocol] error Incoming Execution exception: not-found: Exchange not > found: qpid.ha-q:... > - [Protocol] error Channel exception: not-attached: Channel xxx is not > attached > > On backup: > - [Protocol] error Execution exception: not-found: Exchange not found: > qpid.ha-q:... > > Queue deleted while backup is in the process of subscribing (in the test > happens > because of empty tx) > > On primary: > [Protocol] error Execution exception: not-found: Queue not found: > qpid.ha-tx:... > > On backup: > [Broker] error Incoming execution exception: not-found: Queue not found: > qpid.ha-tx:... > > Note the "Incoming exception exception" error was actually added by this > commit, > since otherwise there's no way to tell the cause of the "channel not attached" > errors. > > This is not a satisfactory situation. The "harmless" error messages might > sometimes indicate real error situations and the clutter makes debugging > harder. I will leave this JIRA open for now and try to come up with a better > solution. > > > Diffs > ----- > > /trunk/qpid/cpp/src/qpid/Exception.cpp 1562539 > /trunk/qpid/cpp/src/qpid/broker/SessionAdapter.cpp 1562539 > /trunk/qpid/cpp/src/qpid/ha/PrimaryTxObserver.h 1562539 > /trunk/qpid/cpp/src/qpid/ha/PrimaryTxObserver.cpp 1562539 > /trunk/qpid/cpp/src/qpid/ha/QueueReplicator.h 1562539 > /trunk/qpid/cpp/src/qpid/ha/QueueReplicator.cpp 1562539 > /trunk/qpid/cpp/src/qpid/ha/TxReplicator.h 1562539 > /trunk/qpid/cpp/src/qpid/ha/TxReplicator.cpp 1562539 > > Diff: https://reviews.apache.org/r/17563/diff/ > > > Testing > ------- > > > Thanks, > > Alan Conway > >
