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] Changes to support secured HBase cluster #591

Merged
merged 4 commits into from
Jan 30, 2016

Conversation

bijugs
Copy link
Contributor

@bijugs bijugs commented Jan 20, 2016

Changes in this pull request include

For hbase098

  • Use HBase connection object to create table object
  • Changes to make YCSB hbase tests run against a kerberized hbase cluster
  • Update to README.txt to document the new parameters

For hbase10

  • Changes to make YCSB hbase tests run against a kerberized hbase cluster

If users like to use kerberos keytab to make the connection to secure hbase cluster instead of using the user-id of the user running the test they can be passed using principal=keytab-principal-name and keytab=keytab=file properties.

Update: After the conversation in issue #322 , changes were made to hbase098 and hbase10 to use single HBase connection across all the threads when running a multi-thread test.

@busbey
Copy link
Collaborator

busbey commented Jan 20, 2016

please start commit messages with just then name of the module impacted (i.e. "[hbase098]" rather than "changes to hbase098"

@busbey
Copy link
Collaborator

busbey commented Jan 20, 2016

For hbase098

  • Use a single HBase connection object across all the threads which is recommended

Do you have some perf numbers to show the actual impact?

Why only do this change in the 0.98 driver?

@busbey
Copy link
Collaborator

busbey commented Jan 20, 2016

If you're going to add properties for the principal and keytab, please document them

  • In the relevant binding README
  • In the workload template file

Are these changes needed to run on a kerberos cluster? I was under the impression the one merely needed to kinit before running YCSB and the hbase driver would correctly use the cached tickets?

}
}
try {
_hConn = HConnectionManager.createConnection(config);
Copy link
Collaborator

Choose a reason for hiding this comment

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

where does this get closed? should either ref count or register a shutdown hook

@busbey
Copy link
Collaborator

busbey commented Jan 20, 2016

Note also that #350 has been waiting for verification of essentially the same keytab functionality.

@bijugs
Copy link
Contributor Author

bijugs commented Jan 22, 2016

Thanks @busbey for the review and feedback.

  • please start commit messages with just then name of the module impacted (i.e. "[hbase098]" rather than "changes to hbase098"
    • Will change the commit message to start with module name.
  • Why only do this change in the 0.98 driver?
    • hbase10 is already using connection object to create table objects and I don't have a 0.94 environment to test the changes.
  • If you're going to add properties for the principal and keytab, please document them
    • Will document them in the module specific readme
  • Are these changes needed to run on a kerberos cluster?
    • We are testing against a Kerberized cluster from a node outside the cluster. These changes should not impact the current behavior
  • Do you have some perf numbers to show the actual impact? & where does this get closed?
    • Since core/Client.java calls _db.cleanup(), at this time single connection object will not work. To complete this PR will close the connection on the cleanup() method. Will raise another request to update the core/Client.java so that DBs which can work with single connection object shared among multiple threads will be able to leverage it.

@risdenk
Copy link
Collaborator

risdenk commented Jan 22, 2016

@busbey and @bijugs There is a related issue #322 for HBase single connection object. Just wanted to cross reference it here.

@bijugs
Copy link
Contributor Author

bijugs commented Jan 22, 2016

@busbey Changes were made for the review comments.

@risdenk Thanks for pointing out the existing issue. Will update with my comments there so that we can have a conversation on the proposed change.

@cmatser
Copy link
Collaborator

cmatser commented Jan 27, 2016

👍 from me

@bijugs bijugs changed the title Changes to support secured HBase cluster [hbase] Changes to support secured HBase cluster Jan 28, 2016
busbey added a commit that referenced this pull request Jan 30, 2016
[hbase] Changes to support secured HBase cluster
@busbey busbey merged commit 779a703 into brianfrankcooper:master Jan 30, 2016
@busbey
Copy link
Collaborator

busbey commented Jan 30, 2016

Thanks for the contribution!

@risdenk risdenk mentioned this pull request Feb 15, 2016
jaricftw pushed a commit to jaricftw/YCSB that referenced this pull request Jul 19, 2016
[hbase] Changes to support secured HBase cluster
jaricftw pushed a commit to jaricftw/YCSB that referenced this pull request Jul 19, 2016
[hbase] Changes to support secured HBase cluster
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants