saintstack commented on a change in pull request #14: HBASE-22843 [HBCK2] Fix HBCK2 after HBASE-22777 & HBASE-22758 URL: https://github.com/apache/hbase-operator-tools/pull/14#discussion_r314359802
########## File path: hbase-hbck2/src/main/java/org/apache/hbase/hbck1/HBCKPrivateCellUtil.java ########## @@ -0,0 +1,32 @@ +package org.apache.hbase.hbck1; + +import org.apache.hadoop.hbase.ByteBufferExtendedCell; +import org.apache.hadoop.hbase.Cell; +import org.apache.hadoop.hbase.util.ByteBufferUtils; +import org.apache.hadoop.hbase.util.Bytes; +import org.apache.yetus.audience.InterfaceAudience; + +/** + * hbck's local version of PrivateCellUtil from the hbase repo + */ [email protected] +final class HBCKPrivateCellUtil { Review comment: PrivateCellUtil is an internal serverside thing. It can be plain CellUtil here. In fact, this facility probably best belongs in CellUtil. It looks harmless except for the bit about ByteBufferExtendedCell. That is internal to hbase. Shouldn't be used outside of core hbase -- i.e. not by HBCK2. HBCK2 version could just copy the qualifier from the Cell ... clone it and then compare against that. It does not have to be performant. So, rename this class as CellUtil I think and do not use internal hbase classes that are private like ByteBufferExtendedCell. Move class to top level since it named CellUtil (no need to put into a 'util' subpackage). You might also consider just adding this single method to your new HBCKMetaTableAccessor class rather than create a new standalone class. COuld be private internal method. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
