Skip to content
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

HBASE-22843 [HBCK2] Fix HBCK2 after HBASE-22777 & HBASE-22758 #14

Merged
merged 2 commits into from Aug 15, 2019

Conversation

@jatsakthi
Copy link
Member

commented Aug 14, 2019

No description provided.

@jatsakthi jatsakthi force-pushed the jatsakthi:HBASE-22843 branch from f73d227 to e8c901a Aug 14, 2019

@jatsakthi jatsakthi requested a review from saintstack Aug 14, 2019

@asf-ci

This comment has been minimized.

Copy link

commented Aug 14, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-HBASE-OPERATOR-TOOLS-Build/55/

@asf-ci

This comment has been minimized.

Copy link

commented Aug 14, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-HBASE-OPERATOR-TOOLS-Build/56/

@jatsakthi

This comment has been minimized.

Copy link
Member Author

commented Aug 14, 2019

I think it's because of the 2.1.6-SNAPSHOT in pom.xml, that the mvninstall failed in precommit.

@saintstack
Copy link
Contributor

left a comment

Comments are a bit messy.... I started at top then jumped down and back...

pom.xml Outdated
@@ -123,7 +123,7 @@
<compileSource>1.8</compileSource>
<java.min.version>${compileSource}</java.min.version>
<maven.min.version>3.3.3</maven.min.version>
<hbase.version>2.1.2</hbase.version>
<hbase.version>2.1.6-SNAPSHOT</hbase.version>

This comment has been minimized.

Copy link
@saintstack

saintstack Aug 15, 2019

Contributor

Why this? Makes me worried we'll end up w/ a dependency on new facility when we are trying to keep HBCK 'universal'.

This comment has been minimized.

Copy link
@jatsakthi

jatsakthi Aug 15, 2019

Author Member

I guess, we would have to eventually take it up to 2.1.6 considering the fact that we want to call hbck.fixMeta() (which was added server side) which was added in 2.1.6?

This comment has been minimized.

Copy link
@saintstack

saintstack Aug 15, 2019

Contributor

True. Lets do that in another issue when we add the fixMeta call?

@asf-ci

This comment has been minimized.

Copy link

commented Aug 15, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-HBASE-OPERATOR-TOOLS-Build/57/

@asf-ci

This comment has been minimized.

Copy link

commented Aug 15, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-HBASE-OPERATOR-TOOLS-Build/58/

@jatsakthi
Copy link
Member Author

left a comment

Thanks for your commens @saintstack . Updated the PR. :)

@saintstack
Copy link
Contributor

left a comment

Looking good here.

@@ -39,12 +39,13 @@
* hbck's local version of the MetaTableAccessor from the hbase repo
*/
@InterfaceAudience.Private
class HBCKMetaTableAccessor {
public class HBCKMetaTableAccessor {

This comment has been minimized.

Copy link
@saintstack

saintstack Aug 15, 2019

Contributor

It can stay package private? Or is it that class in hbck1 refers to it? If so, I suppose you have to make this change yeah.

This comment has been minimized.

Copy link
@jatsakthi

jatsakthi Aug 15, 2019

Author Member

Yes, Stack. It's being used by hbck1.HbaseFsck & hbck1.HbaseFsckRepair.

@saintstack
Copy link
Contributor

left a comment

Just one question in below. Looks good otherwise.

@jatsakthi

This comment has been minimized.

Copy link
Member Author

commented Aug 15, 2019

So, now there are 2 ways to access the Meta table from hbck2. MTA & HBCKMTA. There are lots of instances of MTA usage. Are we planning to slowly move out of it to have just HBCKMTA and refractor the MTA usages with HBCKMTA? @saintstack

@jatsakthi jatsakthi force-pushed the jatsakthi:HBASE-22843 branch from c5524eb to cb0b327 Aug 15, 2019

@asf-ci

This comment has been minimized.

Copy link

commented Aug 15, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-HBASE-OPERATOR-TOOLS-Build/60/

@saintstack

This comment has been minimized.

Copy link
Contributor

commented Aug 15, 2019

I think we move over to new MTA when issue using the old. At least for now.

@saintstack

This comment has been minimized.

Copy link
Contributor

commented Aug 15, 2019

Why the failure?

@jatsakthi

This comment has been minimized.

Copy link
Member Author

commented Aug 15, 2019

One of the checkstyle issues apparently. let me fix it.

Fix checkstyle issues
- Changed import order
- Java doc for IOException for deletefromMetaTable
- Introduce a private constructor for the utility class HBCKMTA
@asf-ci

This comment has been minimized.

Copy link

commented Aug 15, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-HBASE-OPERATOR-TOOLS-Build/61/

@jatsakthi

This comment has been minimized.

Copy link
Member Author

commented Aug 15, 2019

All tests passed. Let me merge this one. Thanks @saintstack

@jatsakthi jatsakthi merged commit 379b8cc into apache:master Aug 15, 2019

1 check passed

default SUCCESS 23 tests run, 0 skipped, 0 failed.
Details

@jatsakthi jatsakthi deleted the jatsakthi:HBASE-22843 branch Aug 15, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.