[
https://issues.apache.org/jira/browse/HBASE-16440?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15432433#comment-15432433
]
ramkrishna.s.vasudevan commented on HBASE-16440:
------------------------------------------------
Went through the patch.
In the default memstore case - I think we will clear the snapshot and call
close and it is always done by one thread only? But once we go with Compacting
memstore we will have the multi threaded case I think.
The patch now creates a Atomic Integer and that decides if we have reached the
maxCount. And putBackChunks() uses the add() method only.
Even if multi threads calls the putBackChunks - it will still be able to drain
what ever is there in the Chunkqueue right? Just asking. Your change is fine
anyway because it is making things much better so we are sure we don't cross
maxCount.
Regarding removing the thread pool creation if log is not in debug mode - I
think it is better not to change it. In UI there is a provision to change LOG
level anytime. So suppose for debugging purpose if some one changes the log
level then since the thread pool itself is not started we wont be able to log
any. I think better not to change it. May be default we can allow only one
thread to be active here?
One more suggestion - may be better to do it here -
The reclaimedChunks blockingqueue can be initialized with max Size == maxcount.
Currently it is with no max Size so it will go upto INTEGER.MAX_VALUE.
> MemstoreChunkPool might cross its maxCount of chunks to pool
> ------------------------------------------------------------
>
> Key: HBASE-16440
> URL: https://issues.apache.org/jira/browse/HBASE-16440
> Project: HBase
> Issue Type: Sub-task
> Reporter: Anoop Sam John
> Assignee: Anoop Sam John
> Fix For: 2.0.0
>
> Attachments: HBASE-16440.patch, HBASE-16440_V2.patch,
> HBASE-16440_V3.patch
>
>
> {code}
> void putbackChunks(BlockingQueue<Chunk> chunks) {
> int maxNumToPutback = this.maxCount - reclaimedChunks.size();
> if (maxNumToPutback <= 0) {
> return;
> }
> chunks.drainTo(reclaimedChunks, maxNumToPutback);
> // clear reference of any non-reclaimable chunks
> if (chunks.size() > 0) {
> if (LOG.isTraceEnabled()) {
> LOG.trace("Left " + chunks.size() + " unreclaimable chunks, removing
> them from queue");
> }
> chunks.clear();
> }
> }
> {code}
> There is no synchroization. 2 threads might be calling this API as part of a
> MSLAB close. (Once the memstore is flushed). It pass all the chunks used by
> it. (Those might not have been come from pool also). We try to put back
> chunks such that it is not crossing maxCount. Suppose maxCount is 10 and
> currently no chunks in 'reclaimedChunks'. Say both threads at line one. Both
> see 'maxNumToPutback ' as 10 and that will make 20 chunks being pooled.
> Similar issue is in putbackChunk(Chunk chunk) API also.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
