-----------------------------------------------------------
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
> 
>

Reply via email to