Skip to content
This repository was archived by the owner on Aug 20, 2025. It is now read-only.

Conversation

@nickwallen
Copy link
Contributor

@nickwallen nickwallen commented Jul 3, 2019

This PR is for the feature/METRON-2088-support-HDP-3.1 feature branch.

Description

The major change in migration to HBase 2.0.2 is that connection management must now be managed by the client (us). Previously this was managed internally within the HBase client.

More specifically, previously you could just instantiate a Table whenever you like and not worry about closing it. Now you must first establish a Connection and from that Connection, you can create a Table. Both of those need to be closed after use. A Connection can be shared by multiple threads, but a Table cannot be shared.

Most of our existing abstractions were not designed to handle connection management, specifically closing the Connection and Table resources after use. This is why the HBase upgrade is quite involved.

This Jira will introduce the new abstractions that allow Metron to interact with HBase 2.0.2, but not remove the existing ones. I will then introduce additional follow-on PRs to start using these new abstractions in various parts of the Metron code base. Finally, the existing abstractions that do not work with HBase 2.0.2 will be removed as a final step.

Changes

  1. Moved the legacy HBase client from HBaseClient to LegacyHBaseClient. This allows the code to continue to function while the new abstractions are introduced. The LegacyHBaseClient will be removed before the feature branch for HDP-3.1 is merged into master.

  2. Introduced two new profiles HDP-2.6 and HDP-3.1. The HDP-2.6 profile is currently the default and will continue to compile Metron with all the same versions that we do today. After the HBase upgrade work is complete, we will change the default profile to HDP-3.1.

  3. Created the HBaseClient interface that expands on the methods provided by the LegacyHBaseClient. These additional methods will allow it to be used more broadly across the code base in future PRs.

    • Created the HBaseTableClient which uses HBase's Table to interact with HBase.
  4. Created the HBaseClientFactory that allows an HBaseClient to be injected as needed.

  5. Created a HBaseConnectionFactory that is responsible for creating connections to HBase.

Acceptance Testing

This should not impact the deployment and functioning of the system. As a sanity check, spin-up Full Dev and ensure alerts are visible.

Pull Request Checklist

  • Is there a JIRA ticket associated with this PR? If not one needs to be created at Metron Jira.
  • Does your PR title start with METRON-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
  • Has your PR been rebased against the latest commit within the target branch (typically master)?
  • Have you included steps to reproduce the behavior or problem that is being changed or addressed?
  • Have you included steps or a guide to how the change may be verified and tested manually?
  • Have you ensured that the full suite of tests and checks have been executed in the root metron folder via:
  • Have you written or updated unit tests and or integration tests to verify your changes?
  • If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
  • Have you verified the basic functionality of the build by building and running locally with Vagrant full-dev environment or the equivalent?

* <p>Maintains a static, in-memory set of records to mimic the behavior of
* an {@link HBaseClient} that interacts with HBase.
*/
public class FakeHBaseClient implements HBaseClient, Serializable {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ends up being very useful as an abstraction to test things that need to interact with HBase. The implementation ended up being a little more complex than I had hoped. That is why I also added a test for this class in FakeHBaseClientTest.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is my understanding correct that FakeHBaseClient and FakeHBaseClientFactory are not directly used in this PR? I agree that FakeHBaseClient looks fairly complex for a testing class. Is this used in PRs that follow or am I just missing where it's included?

My experience with MockHTable has been that it's very complex and difficult to maintain. I would prefer to move away from depending on something like this for unit testing (integration tests use an in-memory server correct?).

I trust that you created this for a reason but it's hard for me to wrap my head around it without some context.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, good point. Let me see if I can drop those from this PR and add them to one of the follow-on PRs where they are actually used.

Maybe there is a way that we can think of to avoid some complexity here. But obviously you won't be able to help unless you see these classes in use.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the Fake* classes and will introduce them in later PRs.

@mmiklavc
Copy link
Contributor

mmiklavc commented Jul 3, 2019

Just curious, if the new API existed in the earlier version, would it make sense to perform this change against master? I'm not sure there are any tangible benefits aside from it de-risking merges and other code changes.

@nickwallen
Copy link
Contributor Author

I think it would be better to just continue on in the feature branch. After all the changes required for the HBase 2.0.2 get merged I'd like to run it through a good set of acceptance tests. It will be a bit safer to progress through this in the feature branch.

@mmiklavc
Copy link
Contributor

mmiklavc commented Jul 3, 2019

@nickwallen - ok, sounds good to me.

@merrimanr
Copy link
Contributor

Looks like a good start to me. +1

asfgit pushed a commit that referenced this pull request Jul 18, 2019
@nickwallen
Copy link
Contributor Author

This has been merged into feature/METRON-2088-support-hdp-3.1. Thanks for the review

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants