----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13650/#review25311 -----------------------------------------------------------
/trunk/qpid/cpp/src/qpid/broker/Broker.cpp <https://reviews.apache.org/r/13650/#comment49686> ACL isn't a plugin anymore. I wonder if this could be done as the last phase of recovery. If it can that would seem a little 'tidier'. /trunk/qpid/cpp/src/qpid/broker/Queue.h <https://reviews.apache.org/r/13650/#comment49687> I feel userid is a little ambiguous on its own. I realise that there is already an owner variable though. Perhaps setOwningUser()? Not the nicest either, but at least its clear as to the purpose. Alternatively add a comment to explain what it is for. /trunk/qpid/cpp/src/qpid/broker/Queue.cpp <https://reviews.apache.org/r/13650/#comment49688> Agree on naming. Maybe updateQuota()? /trunk/qpid/cpp/src/qpid/broker/Queue.cpp <https://reviews.apache.org/r/13650/#comment49684> Would be better to check that there is available data here, so that the broker can still read in 'old' store data (i.e. data written by brokers before this change) /trunk/qpid/cpp/src/qpid/broker/QueueRegistry.cpp <https://reviews.apache.org/r/13650/#comment49685> Do you need the test? If its an empty string will that not simply make no difference to setUserId()? - Gordon Sim On Aug. 19, 2013, 5:20 p.m., Ernie Allen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/13650/ > ----------------------------------------------------------- > > (Updated Aug. 19, 2013, 5:20 p.m.) > > > Review request for qpid and Chug Rolke. > > > Bugs: https://bugzilla.redhat.com/show_bug.cgi?id=955688 > > https://issues.apache.org/jira/browse/https://bugzilla.redhat.com/show_bug.cgi?id=955688 > > > Repository: qpid > > > Description > ------- > > When creating/deleting durable queues, their "owner" isn't tracked after > restart. Therefore it is possible to exceed queue quotas after a restart. > > Problem scenario 1: > -Set ACL limit on a user to 10 queues > -Create 10 durable queues for that user > -Restart the broker > -The 10 durable queues are automatically recovered > -The user can now create an additional 10 queues > > Problem scenario 2: > - Set ACL limit on a user to 10 queues > - Create 10 durable queues for that user > - Decrease ACL limit on that user to 5 queues > - Restart the broker > - All 10 queues should be recovered, but no new ones should be allow until > user drops below 5 queues > > This is a patch supplied by Pavel Moravec. > It allows all existing durable queues to be recovered during broker restart, > but prevents any new queues from being created until the user is below the > ACL limit. > > > Diffs > ----- > > /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1515496 > /trunk/qpid/cpp/src/qpid/broker/Queue.h 1515496 > /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1515496 > /trunk/qpid/cpp/src/qpid/broker/QueueRegistry.cpp 1515496 > > Diff: https://reviews.apache.org/r/13650/diff/ > > > Testing > ------- > > Tried patch with both scenarios from description. > > > Thanks, > > Ernie Allen > >
