[
https://issues.apache.org/jira/browse/PHOENIX-6067?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17193088#comment-17193088
]
Josh Elser commented on PHOENIX-6067:
-------------------------------------
Tried to get through as much of this as I could before my eyes glazed over :).
Thanks for your hard work on this.
{code:java}
[INFO] Running org.apache.phoenix.index.VerifySingleIndexRowTest
[ERROR]
testIfOptionsArePassedToIndexTool[IndexUpgradeToolTest_mutable={1}](org.apache.phoenix.index.IndexUpgradeToolTest)
Time elapsed: 0.026 s <<< ERROR!
java.lang.IllegalStateException: Can't do incremental index verification on
this version of HBase because raw skip scan filters are not supported.
at
org.apache.phoenix.mapreduce.index.IndexTool.parseOptions(IndexTool.java:388)
at
org.apache.phoenix.index.IndexUpgradeToolTest.testIfOptionsArePassedToIndexTool(IndexUpgradeToolTest.java:124)
{code}
Should we {{Assume}} this test off (rather than fail) when running against
HBase 2.1?
{code:java}
- System.out.println(current);
+ System.out.println(current + "column= " +
{code}
nit: missing a space
{code:java}
+ if (!familyMap.containsKey(family)) {
+ return false;
+ }
+ NavigableSet<byte[]> set = familyMap.get(family);
+ if (set == null || set.isEmpty()) {
+ return true;
+ }
{code}
This is a little strange at a glance. If we don't have an entry in the map, we
return false. But if there is a mapping (but the value is null) we return true.
{code:java}
+ familyMap = scan.getFamilyMap();
+ if (familyMap.isEmpty()) {
+ familyMap = null;
+ }
{code}
This code will null out `familyMap` but `isColumnIncluded(Cell)` in the same
class isn't checking for a null `familyMap`. Looks like a bug waiting to happen?
{code:java}
+ byte[] valueBytes =
scan.getAttribute(BaseScannerRegionObserver.INDEX_REBUILD_VERIFY_TYPE);
+ if (valueBytes != null) {
+ verifyType = IndexTool.IndexVerifyType.fromValue(valueBytes);
+ if (verifyType != IndexTool.IndexVerifyType.NONE) {
+ verify = true;
+ }
+ }
{code}
What happens if the client gives a serialized value which we can't parse (e.g.
newer client with a new IndexVerifyType enum value)? Or if they just give
garbage data. I'm assuming that we'll fail gracefully back to the client (and
not hit some retry loop).
{code:java}
public void close() throws IOException {
innerScanner.close();
+ if (indexRowKeyforReadRepair != null) {
+ hTableFactory.shutdown();
+ indexHTable.close();
+ return;
+ }
{code}
Closing `hTableFactory` should be done in the parent GlobalIndexRegionScanner,
not in child IndexRebuildRegionScanner? Looks like the htableFactory is never
cleaned up in the parent, nor the `indexHTable`. Any reason we to not put a
`close()` on GlobalIndexRegionScanner and have IndexRebuildRegionScanner call
super.close()? Looks like the same could be applied to IndexerRegionScanner.
{code:java}
- keys.remove(keyRange);
+ indexKeyToMutationMap.remove(result.getRow());
+ //keys.remove(keyRange);
{code}
Remove commented code?
All in all, most of these are nit-picky or me worrying about edge-cases. What
else do you all think should be done before this lands in master? Lars, you
mentioned "your wringer" -- is that something we should run to make sure the
implementation is reasonably solid before committing?
> (5.x) Global Secondary Index Parity with 4.x
> --------------------------------------------
>
> Key: PHOENIX-6067
> URL: https://issues.apache.org/jira/browse/PHOENIX-6067
> Project: Phoenix
> Issue Type: Improvement
> Reporter: Swaroopa Kadam
> Assignee: Geoffrey Jacoby
> Priority: Blocker
> Fix For: 5.1.0
>
> Attachments: PHOENIX-6067.v1.patch, PHOENIX-6067.v2.patch
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> A large number of indexing JIRAs were done for Phoenix 4.16 but were
> originally unable to be ported to 5.x because of the lack of HBase 2.x
> support for PHOENIX-5645, max lookback age. This was eventually fixed in
> HBASE-24321 and PHOENIX-5881. Because some JIRAs later than the missing ones
> _were_ ported to 5.x, applying them all one-by-one and testing each
> intermediate version would be impractical.
> This JIRA will import the 4.16 global index implementation into 5.1.0, then
> fix HBase API usage to conform to HBase 2.x standards and Phoenix's HBase 2.x
> compatibility shim between minor versions. (For example, max lookback age
> features aren't supported in 2.1 and 2.2 because they lack HBASE-24321, and
> incremental index validation will be deactivated when running against HBase
> 2.1, because of the lack of HBASE-22710 to use raw Filters.)
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
