> On 2012-03-28 08:10:21, fpj wrote: > > bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/GcLedgersTest.java, > > line 124 > > <https://reviews.apache.org/r/4481/diff/1/?file=95778#file95778line124> > > > > Can we perhaps use another latch here instead of time? Relying on time > > doesn't always work and in many cases it will induce unnecessary waiting > > time.
agreed. it could use another latch. will fix it in new patch. > On 2012-03-28 08:10:21, fpj wrote: > > bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/SnapshotMap.java, > > line 37 > > <https://reviews.apache.org/r/4481/diff/1/?file=95777#file95777line37> > > > > Possibly not a big deal, but I wonder if it is really necessary to use > > this lock. In the way I read the code, I think it would work without it, > > but perhaps I'm missing something important. the lock is used to avoid modifying two variables updates & updatesToMerge map, which is used to avoid inserting to updatesToMerge map during snapshot. suppose two thread, one is doing insertion, the other one is done snapshot. 1) insertion thread: get the reference of updates map, tried to call #put. 2) snapshot thread: swap updates map to updatesToMerge map. 3) snapshot thread: iterates over the updatesToMerge map to merge them to snapshot map. (it may take times) 4) insertion thread: execute put. since updates map object has been assigned to updatesToMerge, so the put actually is applied in updatesToMerge, if the insertion position is large than the current position of iteration, it is OK. otherwise, the insertion will be lost. the readwrite lock only blocks when modifying the updates & updatesToMerge references, I think it would not be expensive. - Sijie ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4481/#review6481 ----------------------------------------------------------- On 2012-03-26 14:45:30, Sijie Guo wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4481/ > ----------------------------------------------------------- > > (Updated 2012-03-26 14:45:30) > > > Review request for bookkeeper. > > > Summary > ------- > > create a snapshot of bookie active ledgers, and then fetch the list of > zookeeper metadata, then it is safe to do garbage collection. > > > This addresses bug BOOKKEEPER-193. > https://issues.apache.org/jira/browse/BOOKKEEPER-193 > > > Diffs > ----- > > > bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/LedgerManagerTestCase.java > PRE-CREATION > > bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/GcLedgersTest.java > PRE-CREATION > bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/SnapshotMap.java > PRE-CREATION > > bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManager.java > 9d9bf22 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/FlatLedgerManager.java > 100cdad > > Diff: https://reviews.apache.org/r/4481/diff > > > Testing > ------- > > > Thanks, > > Sijie > >
