Skip to content

[BEAM-4019] Refactor HBaseIO splitting to produce ByteKeyRange objects#5081

Merged
iemejia merged 1 commit intoapache:masterfrom
iemejia:BEAM-4019-hbaseio-bytekeyrange-refactor
Apr 18, 2018
Merged

[BEAM-4019] Refactor HBaseIO splitting to produce ByteKeyRange objects#5081
iemejia merged 1 commit intoapache:masterfrom
iemejia:BEAM-4019-hbaseio-bytekeyrange-refactor

Conversation

@iemejia
Copy link
Member

@iemejia iemejia commented Apr 10, 2018

For some context this is an internal refactor towards a SDF based translation of HBase read, it is just internals. I extracted most (if not all) HBase region/split related methods into an individual class HBaseUtils too. I did not create an additional unit test for that one since it is well covered by HBaseIOTest and I did not want to add additional extra time to the build..

@iemejia iemejia requested a review from tweise April 10, 2018 13:57
@iemejia
Copy link
Member Author

iemejia commented Apr 10, 2018

R: @aromanenko-dev @tweise

Copy link
Contributor

@aromanenko-dev aromanenko-dev left a comment

Choose a reason for hiding this comment

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

LGTM, just a minor comment


/** Utils to interact with an HBase cluster and get information on tables/regions. */
class HBaseUtils {

Copy link
Contributor

Choose a reason for hiding this comment

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

Please, add JavaDoc for this method that explains how it works

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, added docs in all methods of utils.

@iemejia iemejia force-pushed the BEAM-4019-hbaseio-bytekeyrange-refactor branch 2 times, most recently from 1d8ee39 to fd79ee9 Compare April 10, 2018 22:01
regionLocations, read.tableId, read.serializableScan.get());
final int numSources = ranges.size();
LOG.debug("Spliting into {} source(s)", numSources);
if (numSources >= 1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

if numSources > 0 ?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes sir !

@iemejia iemejia force-pushed the BEAM-4019-hbaseio-bytekeyrange-refactor branch from fd79ee9 to 5241c07 Compare April 18, 2018 16:17
@iemejia
Copy link
Member Author

iemejia commented Apr 18, 2018

Ok rebased and merged, this is good to go now. I will wait for Jenkins to be happy and then merge, thanks @aromanenko-dev and @tweise for the review.

@iemejia iemejia force-pushed the BEAM-4019-hbaseio-bytekeyrange-refactor branch from 5241c07 to 80982b0 Compare April 18, 2018 16:20
@iemejia
Copy link
Member Author

iemejia commented Apr 18, 2018

Jenkins break is unrelated so I am merging it.

@iemejia iemejia merged commit ee6e590 into apache:master Apr 18, 2018
@iemejia iemejia deleted the BEAM-4019-hbaseio-bytekeyrange-refactor branch April 18, 2018 18:39
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.

3 participants