[
https://issues.apache.org/jira/browse/PHOENIX-1059?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14270319#comment-14270319
]
Jeffrey Zhong commented on PHOENIX-1059:
----------------------------------------
I checked the patch. Below are my comments and the rest looks good to me.
1)
{noformat}
+ if (SchemaUtil.isMetaTable(tableDesc.getName())
+ || SchemaUtil.isSequenceTable(tableDesc.getName())
+ || SchemaUtil.isStatsTable(tableDesc.getName())) {
+ return;
+ }
{noformat}
Should you check system schema? Currently there is one more system trace table
and in future there maybe more.
2)
{noformat}
+ if (rmt != null) {
+ LOG.error("Error while rolling back the merge failure for
index regions", e);
+ }
{noformat}
You don't have to check "rmt!=null"
3)
{noformat}
+ if
(Bytes.compareTo(mergedRegions.getFirst().getStartKey(), splitRow) == 0) {
+ return reader;
+ } else {
+ childRegion = mergedRegions.getSecond();
+ parentRegion = childRegion;
+ }
{noformat}
Should the condition be
"Bytes.compareTo(mergedRegions.getFirst().getStartKey(), splitRow) != 0"? In
else block, parentRegion & childRegion will have some value? This part you may
can explain a little bit more.(adding comments?)
4) getIndexRegion & getDataRegion will return null. I scanned the code and seem
not all referencing places are checking null. In addition, I see very similar
code in IndexUtil already.
> Support index regions merge on their corresponding data regions merge
> ---------------------------------------------------------------------
>
> Key: PHOENIX-1059
> URL: https://issues.apache.org/jira/browse/PHOENIX-1059
> Project: Phoenix
> Issue Type: Sub-task
> Reporter: rajeshbabu
> Assignee: Rajeshbabu Chintaguntla
> Attachments: PHOENIX-1059.patch
>
>
> When data regions merge corresponding index regions with the same start keys
> also should merge. Check how we can replace the start key in the second
> region data with first region start key.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
