Re: Question regarding force compaction/GC logic in GarbageCollectorThread.java
On Tue, Feb 7, 2017 at 10:56 PM, Charan Reddy G wrote: > Thanks Sijie, for the clarification. So going by Twitter's code, you intend > to run force Major/Minor compaction only once when enableForceGC is called. > Can you please confirm that (which is different from current community code > behavior. I don't think it only force compaction once. When force compaction is enabled, it will go through major compaction. It will then set the forceGarabageCollection back to false. If after major compaction, the disk space is freed, we don't need to do force compaction again. If the disks are still filled up, the disk monitor will set the flag back to true again. > Not sure if it was bug in the current community code or it was > intended to be like that.) Also can you please confirm why it is having > different behavior in current community code? > I am not sure about that. I need to go through the details when I am porting the change back to the community. > > Twitter code has diverged quite a bit from community code, any plans of > merging changes to the community? > I am actively working on that. It has been diverged a lot. so It will take quite a bit effort for me to port them back. > > Thanks, > Charan > > On Tue, Feb 7, 2017 at 10:27 PM, Sijie Guo wrote: > > > The forceGarbageCollection was contributed by Twitter before. I > remembered > > we wrapped that into a try-finally block. I just checked Twitter's > branch. > > We did wrap that in a try-finally block. https://github.com/ > > twitter/bookkeeper/blob/master/bookkeeper-server/src/ > main/java/org/apache/ > > bookkeeper/bookie/GarbageCollectorThread.java#L563 > > > > I need to merge that back. > > > > - Sijie > > > > On Tue, Feb 7, 2017 at 10:22 PM, Charan Reddy G > > > wrote: > > > >> Hey Sijie, > >> > >> Thanks for the response, but didn't get which finally block you are > >> referring to. Are you saying that before executing 'continue' statement > in > >> major compaction 'if' block, forceGarbageCollection should be set to > >> false, so that force Major/Minor compaction would be run only once when > >> enableForceGC is called? > >> > >> Thanks, > >> Charan > >> > >> On Feb 7, 2017 10:08 PM, "Sijie Guo" wrote: > >> > >>> I think forceGarabageCollection should be set in a final block. Can't > >>> remember why it wasn't in current master branch. > >>> > >>> - Sijie > >>> > >>> On Tue, Feb 7, 2017 at 7:01 PM, Charan Reddy G < > reddychara...@gmail.com> > >>> wrote: > >>> > Hi, > > I'm trying to understand the reason behind "continue;" statement in > line 352 of GarbageCollectorThread.java ( > https://github.com/apache/boo > kkeeper/blob/master/bookkeeper-server/src/main/java/org/apac > he/bookkeeper/bookie/GarbageCollectorThread.java). It is > understandable that if we have done majorcompaction then minor > compaction > is not required and it can be skipped, but aren't we missing " > forceGarbageCollection.set(false);" in line 362? Or is it supposed to > be like that? > > Basically I'm trying to findout when enableForceGC() is called, is it > supposed to trigger and do force GC/Compaction just once or untill it > gets > disabled by calling disableForceGC()? > > Thanks, > Charan > > >>> > >>> > > >
Re: Question regarding force compaction/GC logic in GarbageCollectorThread.java
Thanks Sijie, for the clarification. So going by Twitter's code, you intend to run force Major/Minor compaction only once when enableForceGC is called. Can you please confirm that (which is different from current community code behavior. Not sure if it was bug in the current community code or it was intended to be like that.) Also can you please confirm why it is having different behavior in current community code? Twitter code has diverged quite a bit from community code, any plans of merging changes to the community? Thanks, Charan On Tue, Feb 7, 2017 at 10:27 PM, Sijie Guo wrote: > The forceGarbageCollection was contributed by Twitter before. I remembered > we wrapped that into a try-finally block. I just checked Twitter's branch. > We did wrap that in a try-finally block. https://github.com/ > twitter/bookkeeper/blob/master/bookkeeper-server/src/main/java/org/apache/ > bookkeeper/bookie/GarbageCollectorThread.java#L563 > > I need to merge that back. > > - Sijie > > On Tue, Feb 7, 2017 at 10:22 PM, Charan Reddy G > wrote: > >> Hey Sijie, >> >> Thanks for the response, but didn't get which finally block you are >> referring to. Are you saying that before executing 'continue' statement in >> major compaction 'if' block, forceGarbageCollection should be set to >> false, so that force Major/Minor compaction would be run only once when >> enableForceGC is called? >> >> Thanks, >> Charan >> >> On Feb 7, 2017 10:08 PM, "Sijie Guo" wrote: >> >>> I think forceGarabageCollection should be set in a final block. Can't >>> remember why it wasn't in current master branch. >>> >>> - Sijie >>> >>> On Tue, Feb 7, 2017 at 7:01 PM, Charan Reddy G >>> wrote: >>> Hi, I'm trying to understand the reason behind "continue;" statement in line 352 of GarbageCollectorThread.java (https://github.com/apache/boo kkeeper/blob/master/bookkeeper-server/src/main/java/org/apac he/bookkeeper/bookie/GarbageCollectorThread.java). It is understandable that if we have done majorcompaction then minor compaction is not required and it can be skipped, but aren't we missing " forceGarbageCollection.set(false);" in line 362? Or is it supposed to be like that? Basically I'm trying to findout when enableForceGC() is called, is it supposed to trigger and do force GC/Compaction just once or untill it gets disabled by calling disableForceGC()? Thanks, Charan >>> >>> >
Re: Question regarding force compaction/GC logic in GarbageCollectorThread.java
The forceGarbageCollection was contributed by Twitter before. I remembered we wrapped that into a try-finally block. I just checked Twitter's branch. We did wrap that in a try-finally block. https://github.com/twitter/bookkeeper/blob/master/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java#L563 I need to merge that back. - Sijie On Tue, Feb 7, 2017 at 10:22 PM, Charan Reddy G wrote: > Hey Sijie, > > Thanks for the response, but didn't get which finally block you are > referring to. Are you saying that before executing 'continue' statement in > major compaction 'if' block, forceGarbageCollection should be set to > false, so that force Major/Minor compaction would be run only once when > enableForceGC is called? > > Thanks, > Charan > > On Feb 7, 2017 10:08 PM, "Sijie Guo" wrote: > >> I think forceGarabageCollection should be set in a final block. Can't >> remember why it wasn't in current master branch. >> >> - Sijie >> >> On Tue, Feb 7, 2017 at 7:01 PM, Charan Reddy G >> wrote: >> >>> Hi, >>> >>> I'm trying to understand the reason behind "continue;" statement in line >>> 352 of GarbageCollectorThread.java (https://github.com/apache/boo >>> kkeeper/blob/master/bookkeeper-server/src/main/java/org/apac >>> he/bookkeeper/bookie/GarbageCollectorThread.java). It is understandable >>> that if we have done majorcompaction then minor compaction is not required >>> and it can be skipped, but aren't we missing "forceGarbageCollection. >>> set(false);" in line 362? Or is it supposed to be like that? >>> >>> Basically I'm trying to findout when enableForceGC() is called, is it >>> supposed to trigger and do force GC/Compaction just once or untill it gets >>> disabled by calling disableForceGC()? >>> >>> Thanks, >>> Charan >>> >> >>
Re: Question regarding force compaction/GC logic in GarbageCollectorThread.java
Hey Sijie, Thanks for the response, but didn't get which finally block you are referring to. Are you saying that before executing 'continue' statement in major compaction 'if' block, forceGarbageCollection should be set to false, so that force Major/Minor compaction would be run only once when enableForceGC is called? Thanks, Charan On Feb 7, 2017 10:08 PM, "Sijie Guo" wrote: > I think forceGarabageCollection should be set in a final block. Can't > remember why it wasn't in current master branch. > > - Sijie > > On Tue, Feb 7, 2017 at 7:01 PM, Charan Reddy G > wrote: > >> Hi, >> >> I'm trying to understand the reason behind "continue;" statement in line >> 352 of GarbageCollectorThread.java (https://github.com/apache/boo >> kkeeper/blob/master/bookkeeper-server/src/main/java/org/ >> apache/bookkeeper/bookie/GarbageCollectorThread.java). It is >> understandable that if we have done majorcompaction then minor compaction >> is not required and it can be skipped, but aren't we missing " >> forceGarbageCollection.set(false);" in line 362? Or is it supposed to be >> like that? >> >> Basically I'm trying to findout when enableForceGC() is called, is it >> supposed to trigger and do force GC/Compaction just once or untill it gets >> disabled by calling disableForceGC()? >> >> Thanks, >> Charan >> > >
Re: Question regarding force compaction/GC logic in GarbageCollectorThread.java
I think forceGarabageCollection should be set in a final block. Can't remember why it wasn't in current master branch. - Sijie On Tue, Feb 7, 2017 at 7:01 PM, Charan Reddy G wrote: > Hi, > > I'm trying to understand the reason behind "continue;" statement in line > 352 of GarbageCollectorThread.java (https://github.com/apache/ > bookkeeper/blob/master/bookkeeper-server/src/main/ > java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java). It is > understandable that if we have done majorcompaction then minor compaction > is not required and it can be skipped, but aren't we missing " > forceGarbageCollection.set(false);" in line 362? Or is it supposed to be > like that? > > Basically I'm trying to findout when enableForceGC() is called, is it > supposed to trigger and do force GC/Compaction just once or untill it gets > disabled by calling disableForceGC()? > > Thanks, > Charan >
Question regarding force compaction/GC logic in GarbageCollectorThread.java
Hi, I'm trying to understand the reason behind "continue;" statement in line 352 of GarbageCollectorThread.java ( https://github.com/apache/bookkeeper/blob/master/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java). It is understandable that if we have done majorcompaction then minor compaction is not required and it can be skipped, but aren't we missing " forceGarbageCollection.set(false);" in line 362? Or is it supposed to be like that? Basically I'm trying to findout when enableForceGC() is called, is it supposed to trigger and do force GC/Compaction just once or untill it gets disabled by calling disableForceGC()? Thanks, Charan
