PHOENIX-5373 GlobalIndexChecker should treat the rows created by the … #527
Conversation
| if (isEmptyColumn(cell)) { | ||
| // Before PHOENIX-5156, the empty column value was set to 'x'. With PHOENIX-5156, it is now | ||
| // set to VERIFIED (1) and UNVERIFIED (2). In order to skip the index rows that are inserted before PHOENIX-5156 | ||
| // we consider anything that is not UNVERIFIED means VERIFIED. IndexTool should be used to |
There was a problem hiding this comment.
These comment needs to be changed too to reflect new behavior.
There was a problem hiding this comment.
Will remove the comments
| byte[] indexTableName = region.getRegionInfo().getTable().getName(); | ||
| dataHTable = ServerUtil.getHTableForCoprocessorScan(env, | ||
| SchemaUtil.getPhysicalTableName(dataTableName, env.getConfiguration())); | ||
| dataHTable = ServerUtil.ConnectionFactory.getConnection(ServerUtil.ConnectionType.DEFAULT_SERVER_CONNECTION, env).getTable(TableName.valueOf(dataTableName)); |
There was a problem hiding this comment.
Nit: Line is too long
There was a problem hiding this comment.
Will break into two lines.
priyankporwal
left a comment
priyankporwal
left a comment
There was a problem hiding this comment.
LGTM. Only minor comments to address.
gjacoby126
left a comment
gjacoby126
left a comment
There was a problem hiding this comment.
Thanks, @kadirozde . One nit on logging, plus the small changes that @priyankporwal already requested.
| } | ||
| // Rebuild the index rows from the corresponding the rows in the the data table | ||
| byte[] dataRowKey = indexMaintainer.buildDataRowKey(new ImmutableBytesWritable(indexRowKey), viewConstants); | ||
| LOG.debug("repairIndexRows : " + Bytes.toStringBinary(dataRowKey) + " --> " + Bytes.toStringBinary(indexRowKey)); |
There was a problem hiding this comment.
Log should be at TRACE, both because it could be quite verbose, and because some operators have restrictions about logging customer data, which can be contained in row keys.
There was a problem hiding this comment.
I will remove the log. It was not really important to have it. We can have metrics instead.
…previous design as unverified
gjacoby126
left a comment
gjacoby126
left a comment
There was a problem hiding this comment.
+1, thanks @kadirozde . Looks like you'll need to rebase before merging though.
3 participants
…previous design as unverified