> On Aug. 19, 2013, 7 p.m., Gordon Sim wrote: > > /trunk/qpid/cpp/src/qpid/broker/Broker.cpp, line 358 > > <https://reviews.apache.org/r/13650/diff/1/?file=342212#file342212line358> > > > > 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'.
I didn't move this since I didn't want to risk any side-effects. > On Aug. 19, 2013, 7 p.m., Gordon Sim wrote: > > /trunk/qpid/cpp/src/qpid/broker/Queue.h, line 388 > > <https://reviews.apache.org/r/13650/diff/1/?file=342213#file342213line388> > > > > 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. Changed to setOwningUser() > On Aug. 19, 2013, 7 p.m., Gordon Sim wrote: > > /trunk/qpid/cpp/src/qpid/broker/Queue.cpp, line 1194 > > <https://reviews.apache.org/r/13650/diff/1/?file=342214#file342214line1194> > > > > Agree on naming. Maybe updateQuota()? Used Chuck's suggestion if updateAclUserQueueCount() > On Aug. 19, 2013, 7 p.m., Gordon Sim wrote: > > /trunk/qpid/cpp/src/qpid/broker/Queue.cpp, line 1217 > > <https://reviews.apache.org/r/13650/diff/1/?file=342214#file342214line1217> > > > > 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) Surrounded this code with a if (buffer.available()) block > On Aug. 19, 2013, 7 p.m., Gordon Sim wrote: > > /trunk/qpid/cpp/src/qpid/broker/QueueRegistry.cpp, line 67 > > <https://reviews.apache.org/r/13650/diff/1/?file=342215#file342215line67> > > > > Do you need the test? If its an empty string will that not simply make > > no difference to setUserId()? Dropped the test - Ernie ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13650/#review25311 ----------------------------------------------------------- On Aug. 29, 2013, 2:50 p.m., Ernie Allen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/13650/ > ----------------------------------------------------------- > > (Updated Aug. 29, 2013, 2:50 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 > >
