qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Gordon Sim" <g...@redhat.com>
Subject Re: Review Request: Prevent LinkRegistry from re-storing Links and Bridges during recovery.
Date Wed, 03 Oct 2012 08:48:18 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7399/#review12127
-----------------------------------------------------------

Ship it!


I agree, this is much clearer and explicit.

- Gordon Sim


On Oct. 2, 2012, 8:36 p.m., Kenneth Giusti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7399/
> -----------------------------------------------------------
> 
> (Updated Oct. 2, 2012, 8:36 p.m.)
> 
> 
> Review request for qpid and Gordon Sim.
> 
> 
> Description
> -------
> 
> See https://issues.apache.org/jira/browse/QPID-4347
> 
> "The road to hell is paved..."
> 
> Hey Gordon, the patch you attached to the JIRA worked, but it wasn't obvious to me why.
  As a matter of fact, it wasn't obvious how I broke the code in the first place.
> 
> I think I understand now - my original change caused the Link and Bridge constructors
to store their configurations whenever they were created.  This store operation is fine when
they are being created via management, but not when they are being re-created during recovery!
 This wasn't apparent to me when I refactored the code, resulting in the bug.
> 
> How do you feel about this patch as an alternative?  Rather than rely on when the LinkRegistry's
"store" member is initialized, the constructor specifically checks if it is being called during
recovery and avoids the (re) store.
> 
> 
> This addresses bug qpid-4347.
>     https://issues.apache.org/jira/browse/qpid-4347
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/cpp/src/qpid/broker/Broker.h 1393152 
>   /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1393152 
>   /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.cpp 1393152 
> 
> Diff: https://reviews.apache.org/r/7399/diff/
> 
> 
> Testing
> -------
> 
> unit tests + a new test I'm adding to the store to check for this error.
> 
> 
> Thanks,
> 
> Kenneth Giusti
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message