Re: Question regarding force compaction/GC logic in GarbageCollectorThread.java

[Sijie Guo]

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

[Sijie Guo]

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
>>>
>>
>>