Skip to content

Hbase 14921 v3 - please review the recent addition of the CellFlatMap to CompactingMemStore#6

Open
sanastas wants to merge 9 commits intoHBASE-14920from
HBASE-14921-V3
Open

Hbase 14921 v3 - please review the recent addition of the CellFlatMap to CompactingMemStore#6
sanastas wants to merge 9 commits intoHBASE-14920from
HBASE-14921-V3

Conversation

@sanastas
Copy link
Collaborator

@sanastas sanastas commented Apr 6, 2016

@Eshcar @ebortnik

As you know the code is not yet finished, but I would like to here your comments on the current state...

}

@Override
protected Cell getCellFromIndex(int i) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't we just want to call it getCell()?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe getCellByIndex()?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed to getCell()

private AtomicLong reusedChunkCount = new AtomicLong();
private AtomicInteger chunkIDs = new AtomicInteger(1); // 14921

// 14921: IDs Mapping of all chunks (key 0 is forbidden)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add comments and remove the jira number

this.pipeline = new CompactionPipeline(getRegionServices());
this.compactor = new MemStoreCompactor();
initFlushSizeLowerBound(conf);
int t = conf.getInt(COMPACTING_MEMSTORE_TYPE_KEY, COMPACTING_MEMSTORE_TYPE_DEFAULT);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not a meaningful text?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you suggest something more meaningful then COMPACTING_MEMSTORE_TYPE ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants