Conversation
|
Please review the merge of the segments in the pipeline process. I think it can be committed as a separate issue before the policy. Pay attention that I didn't want to change the HeapMemStoreLAB-Chunk-MemStoreLAB awkward relationships. So this is all a bit awkward by now in that area. |
| /** | ||
| * To be used for merging multiple MSLABs | ||
| */ | ||
| public void addPooledChunkQueue(BlockingQueue<PooledChunk> targetQueue) { |
There was a problem hiding this comment.
What is the complexity here O(#items in queue)? How many items in queue, roughly? Could this become a bottleneck in merging/compaction?
Not important for this patch but maybe add a todo comment to change this to a set or list instead of queue
There was a problem hiding this comment.
The complexity is O(#items in targetQueue). Can not say how many items are in the queue as it depends on how many chunks we have per segment and this can vary. But I don't believe there are so many chunks that this would become a bottleneck, however this is for us to see later with stress benchmarking...
Added the comment
|
Please review the merge of the segments in the pipeline process. I think it can be committed as a separate issue before the policy. Pay attention that I didn't want to change the HeapMemStoreLAB-Chunk-MemStoreLAB awkward relationships. So this is all a bit awkward by now in that area. |
| // Create CellSet based on CellArrayMap from compacting iterator | ||
| private CellSet createCellArrayMapSet(int numOfCells, MemStoreCompactorIterator iterator) { | ||
| private CellSet createCellArrayMapSet(int numOfCells, MemStoreCompactorIterator iterator, | ||
| boolean merge) { |
|
Done reviewing the recent PR |
| version++; | ||
| for(Segment itemInSuffix : suffix) { | ||
| itemInSuffix.close(); | ||
| if (!merge) { |
There was a problem hiding this comment.
having the merge flag affecting the swap suffix seems wrong
applying the close in the outer scope seems to be better.
this change could be deferred to the policy jira, but if there's no policy jira, should do it here.
sanastas
force-pushed
the
master
branch
2 times, most recently
from
5d38c99 to
1d70e81
Compare
Eshcar
force-pushed
the
master
branch
2 times, most recently
from
677b95e to
a2e7557
Compare
Eshcar
force-pushed
the
master
branch
3 times, most recently
from
fb8b96a to
649fa32
Compare
sanastas
force-pushed
the
master
branch
2 times, most recently
from
47d1b90 to
f40c386
Compare
galisheffi
force-pushed
the
master
branch
5 times, most recently
from
e050aeb to
c043660
Compare
sanastas
force-pushed
the
master
branch
3 times, most recently
from
77c2c1a to
c09b847
Compare
sanastas
force-pushed
the
master
branch
3 times, most recently
from
3603e12 to
0d94ba7
Compare
3 participants
@Eshcar please review after Eddie's comments were applied