----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7727/#review12720 -----------------------------------------------------------
Ship it! The code looks good, but could use some explanatory comments in the header files. How the channelIds are managed and some one-liners at least for what the methods do. returnChannelId may not be too obvious without reading the implementing code. - Steve Huston On Oct. 24, 2012, 7:55 p.m., Chug Rolke wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/7727/ > ----------------------------------------------------------- > > (Updated Oct. 24, 2012, 7:55 p.m.) > > > Review request for qpid. > > > Description > ------- > > Previously Link channel numbers are allocated by a simple incrementer. When > the counter wrapped then old channel numbers were blindly reused. > > Now a pool of available channel numbers is kept in a RangeSet. Each bridge > allocates a channel number from the pool when it is created and returns the > channel number to the pool when it is destroyed. > > Debug level messages log the allocation and freeing of channel numbers. > > An exception is thrown if the pool runs empty. > > > This addresses bug QPID-4392. > https://issues.apache.org/jira/browse/QPID-4392 > > > Diffs > ----- > > trunk/qpid/cpp/src/qpid/broker/Bridge.h 1400732 > trunk/qpid/cpp/src/qpid/broker/Link.h 1400732 > trunk/qpid/cpp/src/qpid/broker/Link.cpp 1400732 > trunk/qpid/cpp/src/qpid/broker/LinkRegistry.cpp 1400732 > > Diff: https://reviews.apache.org/r/7727/diff/ > > > Testing > ------- > > A test broker is built with a pool of only 15 (vs normal 32767) channel > numbers. Simple tests verify that 1) old code wraps and causes failures, 2) > new code detects pool exhaustion, and 3) new code logs channel numbers being > properly reused. > > Normal self tests pass. > > > Thanks, > > Chug Rolke > >
