You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hbase.apache.org by "zhou_shuaifeng@sina.com" <zh...@sina.com> on 2015/04/21 15:10:35 UTC
A compact bug?
Hi all,
In compactsplitthread.java, the requestCompactionInternal method is like this:
private synchronized CompactionRequest requestCompactionInternal(final HRegion r, final Store s,
...
ThreadPoolExecutor pool = (!selectNow && s.throttleCompaction(size))
? largeCompactions : smallCompactions;
...
this will cause all compactions with selectNow =true go to the small compaction queue, even the size is large enough, is it reasonable?
I checked the commit history, this comes from HBASE-8665, is there any reason or a bug?
thanks.
zhou_shuaifeng@sina.com
Re: Re: A compact bug?
Posted by "zhou_shuaifeng@sina.com" <zh...@sina.com>.
Yes, if this.compaction != null, it will also go to small compactions even if the size is large enough,
So, I think it's still a bug.
I can open a issue and fix it.
Thanks.
zhou_shuaifeng@sina.com
From: 张铎
Date: 2015-04-22 07:12
To: Ted Yu
CC: dev@hbase.apache.org; sershe
Subject: Re: A compact bug?
These code is wrapped by a 'if (this.compaction == null)', so the code will
only be executed when selectNow == false.
2015-04-21 22:49 GMT+08:00 Ted Yu <yu...@gmail.com>:
> In CompactionRunner#run(), we have:
>
> // Now see if we are in correct pool for the size; if not, go to
> the correct one.
>
> // We might end up waiting for a while, so cancel the selection.
>
> assert this.compaction.hasSelection();
>
> ThreadPoolExecutor pool = store.throttleCompaction(
>
> compaction.getRequest().getSize()) ? longCompactions :
> shortCompactions;
>
> if (this.parent != pool) {
>
> this.store.cancelRequestedCompaction(this.compaction);
>
> The above code would adjust to the correct pool, right ?
>
>
> Cheers
>
> On Tue, Apr 21, 2015 at 7:10 AM, 张铎 <pa...@gmail.com> wrote:
>
>> I think it is a bug. According to the comment above, they just want to
>> put system compactions(selectNow == false) to the small pool, so the code
>> should like this
>>
>> ThreadPoolExecutor pool;
>> if (selectNow) {
>> pool = s.throttleCompaction(compaction.getRequest().getSize())
>> ? longCompactions : shortCompactions;
>> } else {
>> pool = shortCompactions;
>> }
>>
>> My code is on master so ignore the variable name differences...
>>
>> Would you mind open a issue on jira?
>>
>> Thanks.
>>
>> 2015-04-21 21:10 GMT+08:00 zhou_shuaifeng@sina.com <
>> zhou_shuaifeng@sina.com>:
>>
>>> Hi all,
>>>
>>> In compactsplitthread.java, the requestCompactionInternal method is like
>>> this:
>>>
>>> private synchronized CompactionRequest requestCompactionInternal(final
>>> HRegion r, final Store s,
>>> ...
>>>
>>> ThreadPoolExecutor pool = (!selectNow && s.throttleCompaction(size))
>>> ? largeCompactions : smallCompactions;
>>>
>>> ...
>>>
>>> this will cause all compactions with selectNow =true go to the small
>>> compaction queue, even the size is large enough, is it reasonable?
>>>
>>> I checked the commit history, this comes from HBASE-8665, is there any
>>> reason or a bug?
>>>
>>> thanks.
>>>
>>>
>>> zhou_shuaifeng@sina.com
>>>
>>
>>
>
Re: A compact bug?
Posted by 张铎 <pa...@gmail.com>.
These code is wrapped by a 'if (this.compaction == null)', so the code will
only be executed when selectNow == false.
2015-04-21 22:49 GMT+08:00 Ted Yu <yu...@gmail.com>:
> In CompactionRunner#run(), we have:
>
> // Now see if we are in correct pool for the size; if not, go to
> the correct one.
>
> // We might end up waiting for a while, so cancel the selection.
>
> assert this.compaction.hasSelection();
>
> ThreadPoolExecutor pool = store.throttleCompaction(
>
> compaction.getRequest().getSize()) ? longCompactions :
> shortCompactions;
>
> if (this.parent != pool) {
>
> this.store.cancelRequestedCompaction(this.compaction);
>
> The above code would adjust to the correct pool, right ?
>
>
> Cheers
>
> On Tue, Apr 21, 2015 at 7:10 AM, 张铎 <pa...@gmail.com> wrote:
>
>> I think it is a bug. According to the comment above, they just want to
>> put system compactions(selectNow == false) to the small pool, so the code
>> should like this
>>
>> ThreadPoolExecutor pool;
>> if (selectNow) {
>> pool = s.throttleCompaction(compaction.getRequest().getSize())
>> ? longCompactions : shortCompactions;
>> } else {
>> pool = shortCompactions;
>> }
>>
>> My code is on master so ignore the variable name differences...
>>
>> Would you mind open a issue on jira?
>>
>> Thanks.
>>
>> 2015-04-21 21:10 GMT+08:00 zhou_shuaifeng@sina.com <
>> zhou_shuaifeng@sina.com>:
>>
>>> Hi all,
>>>
>>> In compactsplitthread.java, the requestCompactionInternal method is like
>>> this:
>>>
>>> private synchronized CompactionRequest requestCompactionInternal(final
>>> HRegion r, final Store s,
>>> ...
>>>
>>> ThreadPoolExecutor pool = (!selectNow && s.throttleCompaction(size))
>>> ? largeCompactions : smallCompactions;
>>>
>>> ...
>>>
>>> this will cause all compactions with selectNow =true go to the small
>>> compaction queue, even the size is large enough, is it reasonable?
>>>
>>> I checked the commit history, this comes from HBASE-8665, is there any
>>> reason or a bug?
>>>
>>> thanks.
>>>
>>>
>>> zhou_shuaifeng@sina.com
>>>
>>
>>
>
Re: A compact bug?
Posted by Ted Yu <yu...@gmail.com>.
In CompactionRunner#run(), we have:
// Now see if we are in correct pool for the size; if not, go to
the correct one.
// We might end up waiting for a while, so cancel the selection.
assert this.compaction.hasSelection();
ThreadPoolExecutor pool = store.throttleCompaction(
compaction.getRequest().getSize()) ? longCompactions :
shortCompactions;
if (this.parent != pool) {
this.store.cancelRequestedCompaction(this.compaction);
The above code would adjust to the correct pool, right ?
Cheers
On Tue, Apr 21, 2015 at 7:10 AM, 张铎 <pa...@gmail.com> wrote:
> I think it is a bug. According to the comment above, they just want to put
> system compactions(selectNow == false) to the small pool, so the code
> should like this
>
> ThreadPoolExecutor pool;
> if (selectNow) {
> pool = s.throttleCompaction(compaction.getRequest().getSize())
> ? longCompactions : shortCompactions;
> } else {
> pool = shortCompactions;
> }
>
> My code is on master so ignore the variable name differences...
>
> Would you mind open a issue on jira?
>
> Thanks.
>
> 2015-04-21 21:10 GMT+08:00 zhou_shuaifeng@sina.com <
> zhou_shuaifeng@sina.com>:
>
>> Hi all,
>>
>> In compactsplitthread.java, the requestCompactionInternal method is like
>> this:
>>
>> private synchronized CompactionRequest requestCompactionInternal(final
>> HRegion r, final Store s,
>> ...
>>
>> ThreadPoolExecutor pool = (!selectNow && s.throttleCompaction(size))
>> ? largeCompactions : smallCompactions;
>>
>> ...
>>
>> this will cause all compactions with selectNow =true go to the small
>> compaction queue, even the size is large enough, is it reasonable?
>>
>> I checked the commit history, this comes from HBASE-8665, is there any
>> reason or a bug?
>>
>> thanks.
>>
>>
>> zhou_shuaifeng@sina.com
>>
>
>
Re: A compact bug?
Posted by 张铎 <pa...@gmail.com>.
I think it is a bug. According to the comment above, they just want to put
system compactions(selectNow == false) to the small pool, so the code
should like this
ThreadPoolExecutor pool;
if (selectNow) {
pool = s.throttleCompaction(compaction.getRequest().getSize())
? longCompactions : shortCompactions;
} else {
pool = shortCompactions;
}
My code is on master so ignore the variable name differences...
Would you mind open a issue on jira?
Thanks.
2015-04-21 21:10 GMT+08:00 zhou_shuaifeng@sina.com <zh...@sina.com>
:
> Hi all,
>
> In compactsplitthread.java, the requestCompactionInternal method is like
> this:
>
> private synchronized CompactionRequest requestCompactionInternal(final
> HRegion r, final Store s,
> ...
>
> ThreadPoolExecutor pool = (!selectNow && s.throttleCompaction(size))
> ? largeCompactions : smallCompactions;
>
> ...
>
> this will cause all compactions with selectNow =true go to the small
> compaction queue, even the size is large enough, is it reasonable?
>
> I checked the commit history, this comes from HBASE-8665, is there any
> reason or a bug?
>
> thanks.
>
>
> zhou_shuaifeng@sina.com
>