-
Notifications
You must be signed in to change notification settings - Fork 1k
PHOENIX-7731: Add phoenix-hbase-compat module for HBase 2.6.4 #2291
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
… memstore and block count read from FS
|
@sanjeet006py at high level, the changes look good but I think once we have hbase 2.6 release, we should create separate PR for the new module without including phoenix metrics changes. |
@sanjeet006py this build will still use 2.5.4 by default right? you can make temporary commit with applying The high level changes look good so far. |
pom.xml
Outdated
| </activation> | ||
| <properties> | ||
| <hbase.profile>2.6.3</hbase.profile> | ||
| <hadoop.version>3.4.1</hadoop.version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you add this ?
While 2.6.3 is built with 3.4.1, in my tests it ran fine with 3.4.2.
We generally want the latest Hadoop that the Hbase version works with (and can run the tests)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I initially started on PR then Phoenix by default was using hadoop 3.4.1 so, I also used same for pinning hadoop version for HBase 2.6.3. But I see now HBase & Phoenix are using 3.4.2 so will update the same. Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should just remove the line and fall back to the default 3.4.2
stoty
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 LGTM
Don't forget to revert the default profile before committing.
|
@sanjeet006py we still don't have full build results with 2.6 profile |
Yes, one of my changes in the PR broke regions splits for local indexes. And, Phoenix version has been updated to 5.4.0 so, that also broke PR's build as I didn't sync latest from master. |
|
My bad, i thought 2.6 default profile was changed to 2.5. I misunderstood with 5.4 change |
…or, checkstyle error is false -ve
pom.xml
Outdated
| <module>phoenix-client-parent</module> | ||
| <module>phoenix-client-parent/phoenix-client-embedded</module> | ||
| <module>phoenix-client-parent/phoenix-client-lite</module> | ||
| <module>phoenix-server</module> | ||
| <module>phoenix-mapreduce-byo-shaded-hbase</module> | ||
| <module>phoenix-assembly</module> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious, we need this change for checkstyle? It seems we should have had these modules defined here right? @stoty wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These shouldn't be listed here.
Adding these here will disable the build optimizations that enable quicker builds by skipping the shading steps.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the comment above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I am seeing Javadoc failure after patch is applied saying above extra modules listed under shade-and-assembly profile for hbase-2.6 are not found. I thought adding them here might help me get past that but doesn't seem to be case. So, reverting this change. I will be posting a summary of unexpected failures I am seeing and would need help on that. Thanks
|
@stoty @virajjasani I am seeing following issue with HBase 2.6 profile (using HBase 2.6.4): Error related to Javadoc status check failure: The exact maven command is working fine locally for me. Any suggestions on how to fix this? |
|
Ignore yetus errors if it works locally. Yetus is known to have problems when the projexct structure changes |
|
All tests have succeeded with HBase 2.6.4 so, switching back to HBase 2.5 as default profile. |
haridsv
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
phoenix-hbase-compatmodule for HBase 2.6.4. Link to the Phoenix dev mailing list thread with alignment on creating newphoenix-hbase-compatmodule.phoenix-hbase-compat-2.6.4module is same asphoenix-hbase-compat-2.6.0except for class:CompatIndexHalfStoreFileReaderwhere new API in 2.6.4 is being used:StoreFileInfo.createStoreFileInfoForHFile().